Opened 5 years ago

Closed 5 years ago

#25022 closed defect (fixed)

change_ring broken on polynomials

Reported by: Vincent Delecroix Owned by:
Priority: major Milestone: sage-8.2
Component: commutative algebra Keywords:
Cc: Ralf Stephan Merged in:
Authors: Vincent Delecroix Reviewers: Julian Rüth
Report Upstream: N/A Work issues:
Branch: 319bb43 (Commits, GitHub, GitLab) Commit: 319bb43a70dd8ddfd9637cbc310230579a643fe6
Dependencies: Stopgaps:

Status badges


If the polynomial can be coerced in the new base ring, the result of change_ring is a constant

sage: p = ZZ['x']([1,2,3])
sage: p.change_ring(QQ['x']).degree()
sage: p.change_ring(SR).degree()

Change History (8)

comment:1 Changed 5 years ago by Nils Bruin

Note that change_ring is supposed to change the base ring. For instance, we have:

sage: p.change_ring(QQ['y']).degree()
sage: p.change_ring(QQ['y']).parent()
Univariate Polynomial Ring in x over Univariate Polynomial Ring in y over Rational Field

In your case, you basically end up with

sage: QQ['x']['x'](p)
3*x^2 + 2*x + 1

and as it goes with coercion, the x of p coerces as low in the tower as possible.

Perhaps you'd prefer the result

sage: QQ['x']['x'](p)
3*x^2 + 2*x + 1

There's some work to be done, though:

sage: R.<x,y>=ZZ[]
sage: f=x^2+y^2
sage: f.change_ring(QQ['x','y']).monomials()
sage: f.change_ring(QQ['x']).monomials()
[x^2, y^2]
sage: f.change_ring(QQ['y']).monomials()
[x^2, y^2]

which does match

sage: QQ['x']['x','y'](f).monomials()
[x^2, y^2]
sage: QQ['x','y']['x','y'](f).monomials()

but doesn't seem to fit the pattern of "coerce 'x' as low as possible. But then, following that rule consistently might not be such a good idea.

sage: QQ['x']['x'](QQ['x']['x'].0).degree()

comment:2 Changed 5 years ago by Vincent Delecroix

I think that change_ring on elements should uniformly be

def change_ring(self, R):
    return self.parent().change_ring(R)(list(self))

I don't plan to change anything to the _element_constructor_ which is definitely allowed to behave differently.

comment:3 Changed 5 years ago by Vincent Delecroix

Authors: Vincent Delecroix
Branch: u/vdelecroix/25022
Commit: 319bb43a70dd8ddfd9637cbc310230579a643fe6
Status: newneeds_review

New commits:

319bb43fix change_ring for polynomials

comment:4 Changed 5 years ago by Julian Rüth

Reviewers: Julian Rüth

Changes look good to me. nbruin, do you agree?

comment:5 Changed 5 years ago by Ralf Stephan

Patchbot is green, too.

comment:6 Changed 5 years ago by Vincent Delecroix


comment:7 Changed 5 years ago by Julian Rüth

Status: needs_reviewpositive_review

Since nbruin has not reacted, let's assume that he does at least not strongly disagree…

comment:8 Changed 5 years ago by Volker Braun

Branch: u/vdelecroix/25022319bb43a70dd8ddfd9637cbc310230579a643fe6
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.