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: 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:

Status badges


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 Ben Hutz

Branch: u/bhutz/ticket/16051
Created: Apr 3, 2014, 11:45:12 AMApr 3, 2014, 11:45:12 AM
Modified: Apr 3, 2014, 11:45:12 AMApr 3, 2014, 11:45:12 AM

comment:2 Changed 9 years ago by Ben Hutz

Authors: Ben Hutz
Commit: 27663feffb8cf5325b6ea4c8c2f2dd793775ce23
Owner: set to Ben Hutz

Made the comparison to be by type and value instead of just value

New commits:

27663feTrac 16051: comparing fast_callable constants by value and type

comment:3 Changed 9 years ago by Ben Hutz

Status: newneeds_review

comment:4 Changed 9 years ago by Nils Bruin

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
sage: (type(a),a)==(type(b),b)
sage: (parent(a),a)==(parent(b),b)

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 git

Commit: 27663feffb8cf5325b6ea4c8c2f2dd793775ce239eb0789bf99a2d976cb57f8104f33bc6e7c5deea

Branch pushed to git repo; I updated commit sha1. New commits:

9eb0789Trac #16051: changed type to parent for constant comparison

comment:6 Changed 9 years ago by Ben Hutz

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 9 years ago by Ben Hutz

opps. I meant #15780.

comment:8 Changed 9 years ago by Dillon Rose

I have run the complete doctest against this ticket.  Everything is good.

comment:9 Changed 9 years ago by Dillon Rose

Reviewers: Dillon Rose
Status: needs_reviewpositive_review

comment:10 Changed 9 years ago by For batch modifications

Milestone: sage-6.2sage-6.3

comment:11 Changed 9 years ago by Volker Braun

Branch: u/bhutz/ticket/160519eb0789bf99a2d976cb57f8104f33bc6e7c5deea
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.