Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No order for univariate polynomial rings #28420

Open
bgrenet opened this issue Aug 29, 2019 · 8 comments
Open

No order for univariate polynomial rings #28420

bgrenet opened this issue Aug 29, 2019 · 8 comments

Comments

@bgrenet
Copy link

bgrenet commented Aug 29, 2019

This ticket add an error message for the constructor of polynomial ring when trying to build a univariate polynomial ring with a term order. Currently, order = ... is silently ignored for univariate polynomial rings and this leads to confusions for some users, cf this Ask question¹.

Before:

sage: PolynomialRing(QQ, 'x', order = TermOrder('wdegrevlex', (2,)))
Univariate Polynomial Ring in x over Rational Field

After:

sage: PolynomialRing(QQ, 'x', order = TermOrder('wdegrevlex', (2,)))
Traceback (most recent call last):
...
TypeError: univariate polynomial rings do not accept an order. Use a 1-variable multivariate polynomial ring instead. 

¹ The Ask question uncovers two distinct problems, the second one is fixed in #28421.

CC: @sagetrac-tmonteil

Component: algebra

Keywords: polynomial, order

Author: Bruno Grenet

Branch: u/bruno/univariate_polyring_order

Issue created by migration from https://trac.sagemath.org/ticket/28420

@bgrenet bgrenet added this to the sage-8.9 milestone Aug 29, 2019
@bgrenet
Copy link
Author

bgrenet commented Aug 29, 2019

Branch: u/bruno/univariate_polyring_order

@bgrenet

This comment has been minimized.

@bgrenet
Copy link
Author

bgrenet commented Aug 29, 2019

Changed keywords from polynomial to polynomial, order

@bgrenet bgrenet changed the title Error messages for polynomial ring constructor No order for univariate polynomial rings Aug 29, 2019
@bgrenet

This comment has been minimized.

@bgrenet
Copy link
Author

bgrenet commented Aug 30, 2019

comment:3

Actually, I've trouble with my proposed solution. I actually tried two possibilities, but with difficulties in both cases:

  1. The solution currently in description: Constructing a univariate polynomial ring with an order in paramater throws an Exception;
  2. Do not throw an Exception, rather make PolynomialRing(R, 'x', order = ...) behave like PolynomialRing(R, 1, 'x', order = ...), that is return a 1-variable multivariate polynomial ring.

In the first case I have a lot of doctests failing, I've not investigated all of them yet. The second solution has much less failed doctests. Though I have mainly one, that actually also occurs with the first solution.

The problem is as follows: To discover that there is a coercion from R['x,y'] to Frac(R['x'])['y'], the algorithm checks whether there is a coercion from R['x,y'].remove_var('y') to Frac(R['x']). A problem occurs in the two solutions, because R['x,y'].remove_var('y') attempts to build the polynomial ring R['x'] but passes the order parameter.

  1. With the first solution, an Exception is raised.
  2. With the second solution, a 1-variable multivariate ring is built, but it happens that there is no coercion from the multivariate R['x'] to the fraction field of the univariate R['x'].

The simplest(?) workaround I've in mind is to keep with the second solution and to add the missing coercion (note: I don't know how to do that!). But I am not sure that this is the right thing to do. In particular, this implies that remove_var now returns a 1-variable multivariate polynomial ring instead of a univariate polynomial ring.
i. The pro: It makes sense for weighted term orders since the 1-variable multivariate ring keeps the weight of the remaining variable;
ii. The con: Users may be surprised by R['x,y'].remove_var('y') not to return a (truly) univariate ring.

@mezzarobba
Copy link
Member

comment:4

Would it be hard to check if the term order is weighted, and return a multivariate ring in a single variable or a univariate ring depending on that?

@embray
Copy link
Contributor

embray commented Dec 30, 2019

comment:5

Ticket retargeted after milestone closed

@embray embray modified the milestones: sage-8.9, sage-9.1 Dec 30, 2019
@mkoeppe
Copy link
Member

mkoeppe commented May 1, 2020

comment:6

Moving tickets to milestone sage-9.2 based on a review of last modification date, branch status, and severity.

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 May 1, 2020
@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Oct 24, 2020
@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Apr 15, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Aug 22, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 May 3, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
@mkoeppe mkoeppe added this to the sage-9.9 milestone Jan 29, 2023
@mkoeppe mkoeppe removed this from the sage-10.0 milestone Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants