Opened 8 years ago
Closed 8 years ago
#16051 closed defect (fixed)
fast_callable can return ipow with exponents in the base ring
Reported by: | bhutz | Owned by: | bhutz |
---|---|---|---|
Priority: | minor | Milestone: | sage-6.3 |
Component: | misc | Keywords: | |
Cc: | Merged in: | ||
Authors: | Ben Hutz | Reviewers: | Dillon Rose |
Report Upstream: | N/A | Work issues: | |
Branch: | 9eb0789 (Commits, GitHub, GitLab) | Commit: | 9eb0789bf99a2d976cb57f8104f33bc6e7c5deea |
Dependencies: | Stopgaps: |
Description
fast_callable returns a string of operations. However, due to how caching of constants is done, the integer power function (ipow) can have constants in the base ring. This is because it checks the list of constants by comparing with == without taking into consideration the parent/type.
Change History (11)
comment:1 Changed 8 years ago by
- Branch set to u/bhutz/ticket/16051
- Created changed from 04/03/14 11:45:12 to 04/03/14 11:45:12
- Modified changed from 04/03/14 11:45:12 to 04/03/14 11:45:12
comment:2 Changed 8 years ago by
- Commit set to 27663feffb8cf5325b6ea4c8c2f2dd793775ce23
- Owner changed from (none) to bhutz
comment:3 Changed 8 years ago by
- Status changed from new to needs_review
comment:4 Changed 8 years ago by
I think parent, when it is available, is a more reliable attribute to determine the mathematical identity of an element (and hence with what constants it can be folded). I think sage.structure.parent
is the right routine to use, because it will fall back on type
if parent
is not available:
sage: parent(1) Integer Ring sage: parent(int(1)) <type 'int'> sage: parent(ZZ) <type 'sage.rings.integer_ring.IntegerRing_class'>
One example where going by parent
is ostensibly better than going by type
is:
sage: a=RealField(1000)(1) sage: b=RealField(200)(1) sage: a==b True sage: (type(a),a)==(type(b),b) True sage: (parent(a),a)==(parent(b),b) False
We shouldn't be folding constants that have different precision.
I don't have examples available, but it's certainly not ruled out that elements of different types could have the same parent (to allow for different representation methods in the same parent, for instance). I don't know if we should be folding constants in those cases.
In any of the cases where this matters other than to distinguish integer powers from from other constants one is really stretching the application domain of fast_callable
anyway.
comment:5 Changed 8 years ago by
- Commit changed from 27663feffb8cf5325b6ea4c8c2f2dd793775ce23 to 9eb0789bf99a2d976cb57f8104f33bc6e7c5deea
Branch pushed to git repo; I updated commit sha1. New commits:
9eb0789 | Trac #16051: changed type to parent for constant comparison
|
comment:6 Changed 8 years ago by
I wasn't aware of the instance of parent. That is definitely better, so I've changed it.
The instances I'm looking at is evaluating polynomials where the base ring can be something interesting like a powerseries ring, i.e. morphisms of projective space where base of projective space is something interesting. Mostly, when we implement fast evaluation for morphisms of projective space in #15920, we just don't want to break anything that currently works.
comment:7 Changed 8 years ago by
opps. I meant #15780.
comment:8 Changed 8 years ago by
I have run the complete doctest against this ticket. Everything is good.
comment:9 Changed 8 years ago by
- Reviewers set to Dillon Rose
- Status changed from needs_review to positive_review
comment:10 Changed 8 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:11 Changed 8 years ago by
- Branch changed from u/bhutz/ticket/16051 to 9eb0789bf99a2d976cb57f8104f33bc6e7c5deea
- Resolution set to fixed
- Status changed from positive_review to closed
Made the comparison to be by type and value instead of just value
New commits:
Trac 16051: comparing fast_callable constants by value and type