Define the zero polynomial to have degree -Infinity for LaurentPolynomialRing

[GiacomoPope]

At the moment LaurentPolynomialRing computes the degree of the zero polynomial element to be -1, which is ambiguous as R(1/x) also has degree -1.

This PR changes the behaviour for both univariate and multivariate LaurentPolynomialRings so the degree of the zero polynomial is -Infinity. This is a common definition for the zero polynomial.

This PR has been made following the discussion on sage-devel: https://groups.google.com/g/sage-devel/c/XVz-4kqWuuo about exactly when the degree of the zero polynomial should be -1, -Infinity or undefined.

sage: R.<x> = LaurentPolynomialRing(ZZ)
sage: R(1/x).degree()
-1
sage: R.zero().degree()
-Infinity
sage: 
sage: R.<x, y> = LaurentPolynomialRing(ZZ)
sage: R(1/x).degree()
-1
sage: R.zero().degree()
-Infinity

Fixes #37491

[mantepse]

I really think we should switch to total degree, but that needs discussion on sage-devel - on the same thread as the infinity change.

[GiacomoPope]

I'm sorry it was always returning the total degree. What I meant before is that it returns max(sum(e) for e in self.exponents() -- which I saw as returning the degree of the term in the polynomial with the highest degree. From conversations elsewhere I now see this is exactly the total degree.

Sorry for my lack of language / understanding in this area. I have adjusted the docstring for degree.

[GiacomoPope]

@mantepse do you think this needs further review? I just want to organise the labels

[tscrim]

Edit: Looking at @mantepse's comment, I actually disagree with it. I would say the doc for multi_polynomial_libsingular.pyx should be changed (although this doesn't have to be done here, but I agree with his general principal that they should match).

[tscrim]

@mantepse I don't think we need to have a sage-devel discussion to change the doc to total degree. I am going to cc @nbruin to see if he has any thoughts on the doc change to using total degree in both places.

[GiacomoPope]

So we change the OUTPUT of the degree() function in multi_polynomial_libsingular.pyx in the following way?

If ``x`` is not given, return the maximum degree of the monomials of        
the polynomial. Note that the degree of a monomial is affected by the
gradings given to the generators of the parent ring. If ``x`` is given,
it is (or coercible to) a generator of the parent ring and the output
is the maximum degree in ``x``. This is not affected by the gradings of
ithe generators.

If ``x`` is ``None``, return the total degree of ``self``. Note that this
result is affected by the gradings given to the generators of the parent
ring. Otherwise, if ``x`` is (or is coercible to) a generator of the parent
ring, the output is the maximum degree of ``x`` in ``self``. This is not
affected by the gradings of the generators.

[mantepse]

@mantepse I don't think we need to have a sage-devel discussion to change the doc to total degree.

No, we certainly don't - I thought, by mistake, that it wouldn't compute the total degree but rather the maximum of the degrees of the generators.

[nbruin]

I find the description on multi_polynomial_singular confusing. If the degree is dependent on the term ordering, does that mean it returns some kind of degree for the leading term? I don't think it does:

sage: R.<x,y>=QQ[] # I think this has lex order
sage: (x^2*y+x*y^3).degree()
4
sage: (x^3*y+x*y^2).degree()
4
sage: (x^3*y).degree()
4
sage: (x*y^2).degree()
3

Or does it mean that variables may have a weight and that the weighted total degree (shortened to weighted degree if the weighting is not the standard all 1's) ? Degrees generally arise from putting some grading on your ring (not necessarily into finite-dimensional pieces!) and then the degree of a polynomial is the highest graded piece your polynomial has a component in. From what I see above, I don't think degree without a parameter uses a degree that is fully derived from the term ordering.

So I think it's worthwhile looking at what degree actually does there; use the term "total degree" for what it actually means and explain "weighted degree" when necessary. "maximum degree" derived from the term ordering seems to me "(weighted) degree of leading monomial", which probably isn't what we use in the case of lex ordering.

[GiacomoPope]

I don't really understand how grading is defined here, but for all standard orderings the degree is unchanged (as one would expect from the total degree) but for weighting this indeed has a different result:

sage: R.<x, y> = PolynomialRing(QQ)
sage: (x^2*y + x*y^3).degree()
4
sage: (x^3*y + x*y^2).degree()
4
sage: (x^3*y).degree()
4
sage: (x*y^2).degree()
3

sage: R.<x, y> = PolynomialRing(QQ, order=TermOrder("wdegrevlex", [1, 3]))
sage: (x^2*y + x*y^3).degree()
10
sage: (x^3*y + x*y^2).degree()
7
sage: (x^3*y).degree()
6
sage: (x*y^2).degree()
7
sage: 
sage: (x^2*y + x*y^3).degree()
10
sage: (x^2*y + x*y^3).degree(x)
2
sage: (x^2*y + x*y^3).degree(y)
3

So maybe a better description is:

If ``x`` is ``None``, return the total degree of ``self``. Note that this
result is affected by the weighting given to the generators of the parent
ring. Otherwise, if ``x`` is (or is coercible to) a generator of the parent
ring, the output is the maximum degree of ``x`` in ``self``. This is not
affected by the weighting of the generators.

[nbruin]

@GiacomoPope +1. It's cleaner if this documentation changes to multi_polynomials goes on a separate ticket, though.

[GiacomoPope]

OK I have made a new PR for this change. Does anything else need work / review on this PR?

[tscrim]

I think I am okay. @mantepse any other comments?

[mantepse]

fine with me!

[tscrim]

Then let's get this in. We will need to have the degree() function for Laurent polynomials work with the grading first before changing the doc here to match the regular polynomials. That should be on its own PR as well.

[GiacomoPope]

Yeah I think there needs to be a PR for a check of weighted polynomials and this class. Maybe loads is broken.

[tscrim]

Laurent polynomials have not received as much care as regular polynomials (this is unsurprising to me though). There are likely other similar inconsistencies. I probably would be more reserved with suggesting there might be large amounts of broken code, but that's me.

[tscrim]

To require less reading between the lines: I very much appreciate the work you're doing in finding and fixing these issues with Laurent polynomials.

[GiacomoPope]

Oh! I didn't mean so much that the Laurent code is broken but rather the use of weighted polynomials seems to be permitted in many places but not expected often.

I wouldn't be surprised if many functions simply didn't anticipate weighted polynomials and so will just behave weirdly.

As far as I can tell the weighted polynomials and their uses in sage aren't fully mature. An example I came across recently is really weighted polynomials are natural for hyperelliptic curves but currently aren't used and moving to them from normal ones reveals some bugs.

[tscrim]

It's no problem. I know what you meant and it wasn't meant as an admonishment in any way. You had good qualifiers in your statement too.

I just try to be careful with these types of statements with potential misinterpretation. I spend some time going around trying to convince people to use and develop for Sage (via SageDays (Please consider coming to one or more! Typically, they are only announced on that page.), research visits, math talks), and having some precise statements about where there are problems helps. There have been a few cases recently where people go "everything in area X is horribly broken" (paraphrased here) because of some isolated bugs, which might have made me too sensitive on this too...

[GiacomoPope]

Yeah my recent sage burst of sage energy came off of sage days 123. In fact aside from a few rabbit holes most of these PR I've been looking at have been to try and support (hyper)elliptic and isogeny code.

Nice to do clean up PR like these ones too. I owe a lot of my maths understanding to sage and I really want it to be more robust

[tscrim]

Yeah my recent sage burst of sage energy came off of sage days 123. In fact aside from a few rabbit holes most of these PR I've been looking at have been to try and support (hyper)elliptic and isogeny code.

That's great. Sorry I couldn't be there for that SageDays (I was still teaching and am in Europe for all of this month of March).

Nice to do clean up PR like these ones too. I owe a lot of my maths understanding to sage and I really want it to be more robust

That is exactly how I learn a number of (or verify my understanding of) math things. Programming doesn't allow for shortcuts. Although I also take this as part of my (broader community) service work too...

[nbruin]

Then let's get this in. We will need to have the degree() function for Laurent polynomials work with the grading first before changing the doc here to match the regular polynomials. That should be on its own PR as well.

If we have different weights on the generators, I think this might affect what one would consider the "valuation" as well. I suspect there is some toric geometry that can make sense of such a weighting. The valuation would not correspond to a straight blow-up of the origin, but I suspect there is some interpretation that makes sense.

[tscrim]

If we have different weights on the generators, I think this might affect what one would consider the "valuation" as well. I suspect there is some toric geometry that can make sense of such a weighting. The valuation would not correspond to a straight blow-up of the origin, but I suspect there is some interpretation that makes sense.

I don't know enough (algebraic) geometry to try and construct this, or even to understand the details your previous description. I imagine someone has already thought of this before since it is such a natural (or at least naive) construction considering it algebraically.

[vbraun]

merge conflict

[GiacomoPope]

@vbraun i merged develop into this branch. There were no merge conflicts but I suppose after the CI runs we can make sure everything is ok

[GiacomoPope]

This should now be all good @vbraun -- thank you for your help with this.

[GiacomoPope]

@vbraun I think I have handled all the failures and conflicts. There is a package failure in the CI but I think this is unrelated? Pinging because of the needs work tab

[github-actions]

Documentation preview for this PR (built with commit abd4188; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[GiacomoPope]

Sorry for the ping, @vbraun but wanted to follow up on the needs work tag.

[tscrim]

@GiacomoPope Same comment as the other ticket (@dimpase has already removed the tag here).