BooleanPolynomialRing.remove_var(...) does not return a BooleanPolynomialRing

[sagetrac-Bouillaguet]

sage: P.<x,y,z,w> = BooleanPolynomialRing()
sage: P
Boolean PolynomialRing in x, y, z, w

and :

sage: P.remove_var('x')
Multivariate Polynomial Ring in y, z, w over Finite Field of size 2

CC: @malb @alexanderdreyer @sagetrac-PolyBoRi

Component: commutative algebra

Author: Charles Bouillaguet

Reviewer: Alexander Dreyer

Merged: sage-5.7.beta1

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

[sagetrac-Bouillaguet]

Author: Charles Bouillaguet

[alexanderdreyer]

comment:2

The patch looks good, but what about the ordering?

[sagetrac-Bouillaguet]

comment:3

I mimicked what existed in multi_polynomial_ring_generic.pyx, and it ignored the order (my intuition would be that sometimes it must not be possible to carry on the order, when it is a block order for instance).

I think that we could let the user optionally provide an order for the "reduced" ring, and not try to do anything automatically... Do you agree ?

[alexanderdreyer]

comment:4

Replying to @sagetrac-Bouillaguet:

I mimicked what existed in multi_polynomial_ring_generic.pyx, and it ignored the order (my intuition would be that sometimes it must not be possible to carry on the order, when it is a block order for instance).

For block orders you just have to make the blocks smaller. That' not a oneliner, but something like

blks=R.term_order().blocks()
max_idx = len(blks[0])
if idx < max_idx:
  if nlen > 0:
    new_order = TermOrder(bl, len(bl)-1)
else:
  new_order = bl

for bl in blks[1:]:
  nlen = len(bl)
  max_idx += nlen
  if idx < max_idx:
     if nlen > 0:
       new_order += TermOrder(bl, nlen-1)
  else:
     new_order += bl
 

should do.

I think that we could let the user optionally provide an order for the "reduced" ring, and not try to do anything automatically... Do you agree ?

It the line above do not work out for some case, it would be an acceptable alternative.

[sagetrac-Bouillaguet]

comment:5

Attachment: 13965_boolean_remove_var.patch.gz

I implemented a compromise solution, and for good measure I also adapted it to generic polynomial rings.

AlexanderDreyer, do you like it?

[alexanderdreyer]

comment:6

Replying to @sagetrac-Bouillaguet:

I implemented a compromise solution, and for good measure I also adapted it to generic polynomial rings.

AlexanderDreyer, do you like it?

Yeah, that indeed a good comprise: it works out of the box in typical use cases and is not to hard in the remaining cases (where people probably know their way thru). So this is a positive review, if patchbot agrees.

[sagetrac-Bouillaguet]

Reviewer: Alexander Dreyer

[sagetrac-Bouillaguet]

comment:7

Patchbot agrees :)

[jdemeyer]

Merged: sage-5.7.beta1