Opened 9 years ago
Closed 9 years ago
#16051 closed defect (fixed)
fast_callable can return ipow with exponents in the base ring
Reported by:  Ben Hutz  Owned by:  Ben Hutz 

Priority:  minor  Milestone:  sage6.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 9 years ago by
Branch:  → u/bhutz/ticket/16051 

Created:  Apr 3, 2014, 11:45:12 AM → Apr 3, 2014, 11:45:12 AM 
Modified:  Apr 3, 2014, 11:45:12 AM → Apr 3, 2014, 11:45:12 AM 
comment:2 Changed 9 years ago by
Authors:  → Ben Hutz 

Commit:  → 27663feffb8cf5325b6ea4c8c2f2dd793775ce23 
Owner:  set to Ben Hutz 
comment:3 Changed 9 years ago by
Status:  new → needs_review 

comment:4 Changed 9 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 9 years ago by
Commit:  27663feffb8cf5325b6ea4c8c2f2dd793775ce23 → 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 9 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:8 Changed 9 years ago by
I have run the complete doctest against this ticket. Everything is good.
comment:9 Changed 9 years ago by
Reviewers:  → Dillon Rose 

Status:  needs_review → positive_review 
comment:10 Changed 9 years ago by
Milestone:  sage6.2 → sage6.3 

comment:11 Changed 9 years ago by
Branch:  u/bhutz/ticket/16051 → 9eb0789bf99a2d976cb57f8104f33bc6e7c5deea 

Resolution:  → fixed 
Status:  positive_review → 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