Open rusydi opened 7 years ago
Branch: u/ruhm/polyseq_change_ring
Description changed:
---
+++
@@ -1 +1 @@
-
+This patch implements change_ring() in PolynomialSequence to make it easier if someone wants to change the polynomials to be defined over the new polynomial ring (e.g. polynomial ring with different monomial ordering or variable names).
Changed keywords from none to PolynomialSequence
Author: Rusydi H. Makarim
Hello,
You would better follow the dosctring convention: a short one line summary followed by an optional description paragraph.
V.
Branch pushed to git repo; I updated commit sha1. New commits:
184010d | change description to follow docstring convention |
Replying to @videlec:
Hello,
You would better follow the dosctring convention: a short one line summary followed by an optional description paragraph.
V.
Thanks Vincent, the fix has been made in the last commit.
Cheers, Rusydi
The behavior of the method does not correspond to what the documentation says. In the following example, the base ring is not isomorphic at all.
sage: P.<x, y, z> = GF(31)[]
sage: F = Sequence([x*y, z])
sage: F.change_ring(ZZ)
[x*y, z]
sage: F.change_ring(ZZ).universe()
Multivariate Polynomial Ring in x, y, z over Integer Ring
As this is the natural behavior for change_ring
, I would keep it and adapt the documentation.
Replying to @videlec:
The behavior of the method does not correspond to what the documentation says. In the following example, the base ring is not isomorphic at all.
sage: P.<x, y, z> = GF(31)[] sage: F = Sequence([x*y, z]) sage: F.change_ring(ZZ) [x*y, z] sage: F.change_ring(ZZ).universe() Multivariate Polynomial Ring in x, y, z over Integer Ring
As this is the natural behavior for
change_ring
, I would keep it and adapt the documentation.
Hi Vincent,
I see your point. I initially wrote the documentation by referring to the documentation of change_ring()
in MPolynomialRing_generic
class
sage: P.<x, y, z> = ZZ[]
sage: P.change_ring?
Docstring:
Return a new multivariate polynomial ring which isomorphic to self,
but has a different ordering given by the parameter 'order' or
names given by the parameter 'names'.
INPUT:
* "base_ring" -- a base ring
* "names" -- variable names
* "order" -- a term order
EXAMPLES:
sage: P.<x,y,z> = PolynomialRing(GF(127),3,order='lex')
sage: x > y^2
True
sage: Q.<x,y,z> = P.change_ring(order='degrevlex')
sage: x > y^2
False
I will make the change in docstring accordingly. But in that case, should the docstring of change_ring()
of MPolynomialRing_generic
class must be changed too ? Do you agree ?
Regards, Rusydi
Branch pushed to git repo; I updated commit sha1. New commits:
45b05be | adapt docstring to the behaviour of change_ring() |
You are definitely right. It would make sense to also modify the documentation of MPolynomialRing_generic
accordingly. Could you do it simultaneously in this ticket?
In both docstrings it would be good to say that each argument is optional. Your examples are well chosen to illustrate the behavior.
Note that when base_ring
is not specified, then the new polynomial ring is actually isomorphic.
Branch pushed to git repo; I updated commit sha1. New commits:
561058c | add more description, modify docstring of change_ring() in MPolynomialRing_generic |
Changed branch from u/ruhm/polyseq_change_ring to u/defeo/polyseq_change_ring
Note that the behaviour of this new change_ring
method is somewhat inconsistent with the one of ideals:
sage: A.<x,y,z> = QQ[]
sage: I = A.ideal(x^3-x*y+z^4-1, x*z*y-9)
sage: G = I.groebner_basis()
sage: G.change_ring(order='lex')
[x^4*y - x^2*y^2 - x*y + 9*z^3, x^3 - x*y + z^4 - 1, x*y*z - 9]
sage: I.change_ring(A.change_ring(order='lex'))
Ideal (x^3 - x*y + z^4 - 1, x*y*z - 9) of Multivariate Polynomial Ring in x, y, z over Rational Field
sage: I.change_ring(order='lex')
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
<ipython-input-6-7c629b59d8c9> in <module>()
----> 1 I.change_ring(order='lex')
TypeError: change_ring() got an unexpected keyword argument 'order'
In my opinion, polynomial sequences should behave like ideals.
Replying to @defeo:
Note that the behaviour of this new
change_ring
method is somewhat inconsistent with the one of ideals:sage: A.<x,y,z> = QQ[] sage: I = A.ideal(x^3-x*y+z^4-1, x*z*y-9) sage: G = I.groebner_basis() sage: G.change_ring(order='lex') [x^4*y - x^2*y^2 - x*y + 9*z^3, x^3 - x*y + z^4 - 1, x*y*z - 9] sage: I.change_ring(A.change_ring(order='lex')) Ideal (x^3 - x*y + z^4 - 1, x*y*z - 9) of Multivariate Polynomial Ring in x, y, z over Rational Field sage: I.change_ring(order='lex') --------------------------------------------------------------------------- TypeError Traceback (most recent call last) <ipython-input-6-7c629b59d8c9> in <module>() ----> 1 I.change_ring(order='lex') TypeError: change_ring() got an unexpected keyword argument 'order'
In my opinion, polynomial sequences should behave like ideals.
I feel this behaviour of change_ring()
in Ideal, which takes a ring as input, is too restrictive. The main purpose of calling change_ring()
in PolynomialSequence()
using optional arguments is to simplify how users can modify a set of polynomials. There should not be any need to explicitly construct a new ring when one only wants to change, say, the term ordering while keeping the rest (variable names and base ring).
I'm not saying that ideals do the right thing, I'm just saying that we want the interfaces to be consistent. Think of a user who already knows how to change the ordering of ideals, and discovers this new .change_ring
method on polynomial sequences. She might tempted to do
sage: A.<x,y,z> = QQ[]
sage: I = A.ideal(x^3-x*y+z^4-1, x*z*y-9)
sage: G = I.groebner_basis()
sage: H = G.change_ring(A.change_ring(order='lex'))
sage: H
[x^4*y - x^2*y^2 - x*y + 9*z^3, x^3 - x*y + z^4 - 1, x*y*z - 9]
sage: H.ring()
Multivariate Polynomial Ring in x, y, z over Multivariate Polynomial Ring in x, y, z over Rational Field
Oops!
I don't know why MPolynomialIdeal.change_ring
was designed this way. We may think of changing it, however this would require a deprecation process.
In my opinion change_ring
is a misnomer in the first place. It would have been much less error prone to have methods change_order
and change_names
, then we would have had no ambiguity on what these methods do.
It might be worth asking on sage-devel what other developers involved in multivariate polynomial rings think of this.
Yeah, I got your point about consistency issue. I have sent an email to sage-devel. Lets see what others opinion about it. Btw, thanks alot for pointing out this issue! Otherwise, it might go unnoticed :-)
Cheers. Rusydi
Changed branch from u/defeo/polyseq_change_ring to u/ruhm/polyseq_change_ring
Hello,
Ideals are not restricted to polynomial rings (e.g. order in number
fields). Hence "change_ring" in that case should be generic enough.
For me,
* it would be better to have "change_ring" consistent everywhere
(possibly using *args and/or **kwds)
* if not possible, it would be better to have consistency of
"change_ring" for Polynomial, MPolynomial and PolynomialSequence
rather than with ideal
Vincent
From Vincent's reply, I dont see any need to make this implementation to be consistent with Ideal. But I have to disagree with his second suggestion because the method change_ring()
in Polynomial
and MPolynomial
has different purpose than I originally intended for PolynomialSequence
. From the documentation, change_ring()
in Polynomial and MPolynomial only changes the coefficients. This essentially only change the base ring. So, I don't think this is a good reason to make change_ring() consistent with the one implemented for Polynomial and MPolynomial.
So since PolynomialSequence
is not as generic as Ideal
and also with the reason that I mentioned above, I would suggest to keep the patch in this ticket as it is for the moment.
Regards,
So, to summarize, there are two different concepts that are called "changing ring" in Sage:
Depending on the object, the method change_ring()
does one or the other. I know by experience that this causes a lot of confusion to students. In my opinion it would have been better to have two different and more explicit names (e.g., change_base_ring()
and change_parent_ring()
)
For univariate and multivariate polynomials change_ring()
changes the coefficient ring. Changing the ambient ring is easily done by changing parent:
sage: A.<X> = QQ[]
sage: X.change_ring(RR).parent()
Univariate Polynomial Ring in X over Real Field with 53 bits of precision
sage: B = QQ['X,Y']
sage: B(X).parent()
Multivariate Polynomial Ring in X, Y over Rational Field
For ideals of multivariate polynomials change_ring()
changes the ambient ring. This can be used to change the coefficient ring too:
sage: A.<X,Y> = QQ[]
sage: I = A.ideal(X)
sage: B = QQ['X,Y,Z']
sage: I.change_ring(B).ring()
Multivariate Polynomial Ring in X, Y, Z over Rational Field
sage: C = RR['X,Y']
sage: I.change_ring(C).ring()
Multivariate Polynomial Ring in X, Y over Real Field with 53 bits of precision
Ideals of univariate polynomials have no change_ring()
method.
Polynomial sequences are similar to ideals in the fact that their parent is not the ring:
sage: A.<X,Y> = QQ[]
sage: S = Sequence([X,X^2])
sage: I = S.ideal()
sage: S.parent()
<class 'sage.rings.polynomial.multi_polynomial_sequence.PolynomialSequence_generic'>
sage: I.parent()
Monoid of ideals of Multivariate Polynomial Ring in X, Y over Rational Field
sage: S.ring()
Multivariate Polynomial Ring in X, Y over Rational Field
sage: I.ring()
Multivariate Polynomial Ring in X, Y over Rational Field
So we cannot use coercion to change their ambient ring. However polynomial sequences are different from ideals in that they're not an algebraic structure per se; in this respect they are more similar to polynomials.
If we want to be consistent with Vincent's proposal, we should make polynomial sequences behave as much as possible like polynomials. To that extent:
parent()
of a polynomial sequence should be the ring,change_ring()
should change the coefficient ring.Note that in this case the only way to change the ambient ring would be a list comprehension, or something similar.
If we want to be consistent with my proposal, we should make polynomial sequences behave as much as possible like ideals. To that extent:
parent()
will stay not well defined,change_ring()
should change the ambient ring.The coefficient ring could then be changed via change_ring()
, like for ideals.
I say "Vincent's" and "my" proposal, but I'm actually quite undecided myself. I would like if more devs stepped up and gave strong arguments for one or the other. There might also be other objects to keep into account, and inconsistencies I haven't thought of.
In the end, I think my preferred solution would be to deprecate change_ring()
, and have two methods with more explicit names. This requires even more consensus, however.
P.S.: Sorry for delaying your ticket, @rusydi. It looks like you opened a can of worms :)
The branch doesn't merge cleanly.
Work Issues: Rebase
This patch implements change_ring() in PolynomialSequence to make it easier if someone wants to change the polynomials to be defined over the new polynomial ring (e.g. polynomial ring with different monomial ordering or variable names).
Component: commutative algebra
Keywords: PolynomialSequence
Work Issues: Rebase
Author: Rusydi H. Makarim
Branch/Commit: u/ruhm/polyseq_change_ring @
be773d6
Issue created by migration from https://trac.sagemath.org/ticket/21780