Opened 4 years ago
Closed 4 years ago
#23912 closed enhancement (fixed)
parent() is slow for non-Elements
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.1 |
Component: | coercion | Keywords: | |
Cc: | tscrim | Merged in: | |
Authors: | Jeroen Demeyer | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | b87032e (Commits, GitHub, GitLab) | Commit: | b87032e4aad9ce2064ac46ed365ff93c0c4b0e57 |
Dependencies: | Stopgaps: |
Description (last modified by )
The current implementation of the global function parent()
is:
cpdef inline parent(x): if isinstance(x, Element): return (<Element>x)._parent # Fast check for "number" types, including int and float if PyNumber_Check(x): return type(x) try: p = x.parent except AttributeError: return type(x) else: return p()
It turns out that the non-Element
case is sufficiently slow to cause an important slow-down, as observed in #23905.
There are a few classes in Sage which do not inherit from Element
but still define a parent()
method:
src/sage/algebras/cluster_algebra.py:class ClusterAlgebraSeed(SageObject) src/sage/combinat/root_system/weyl_characters.py:class WeightRing(CombinatorialFreeModule) src/sage/combinat/species/structure.py:class GenericSpeciesStructure(CombinatorialObject) src/sage/combinat/words/abstract_word.py:class Word_class(SageObject) src/sage/dynamics/flat_surfaces/strata.py:class ConnectedComponentOfAbelianStratum(SageObject) src/sage/geometry/lattice_polytope.py:class LatticePolytopeClass(SageObject, collections.Hashable) src/sage/schemes/elliptic_curves/heegner.py:class GaloisAutomorphism(SageObject) src/sage/structure/sage_object.pyx:cdef class SageObject # Returns type(self)
Change History (8)
comment:1 Changed 4 years ago by
- Description modified (diff)
comment:2 Changed 4 years ago by
- Description modified (diff)
comment:3 Changed 4 years ago by
- Branch set to u/jdemeyer/parent___is_slow_for_non_elements
comment:4 Changed 4 years ago by
- Commit set to b87032e4aad9ce2064ac46ed365ff93c0c4b0e57
- Milestone changed from sage-8.1 to sage-pending
- Status changed from new to needs_review
comment:5 Changed 4 years ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
In some of those cases, there are probably better names than parent
but were initially chosen to have compatibility of sorts with Sage's framework. Anyways, I doubt it would cause a problem in any private code.
comment:6 Changed 4 years ago by
Note: I put this ticket in the milestone sage-pending to wait a few days to see how the discussion on sage-devel turns out.
comment:7 Changed 4 years ago by
- Milestone changed from sage-pending to sage-8.1
I think we can set this back to be merged into Sage.
comment:8 Changed 4 years ago by
- Branch changed from u/jdemeyer/parent___is_slow_for_non_elements to b87032e4aad9ce2064ac46ed365ff93c0c4b0e57
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
In parent(x), do not call x.parent()