Opened 5 years ago

Closed 5 years ago

#17366 closed defect (fixed)

for multivariate polynomial rings, degree method should convert its argument

Reported by: jhpalmieri Owned by:
Priority: major Milestone: sage-6.5
Component: algebra Keywords:
Cc: Merged in:
Authors: John Palmieri Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: 65e68e1 (Commits) Commit: 65e68e16e2bffc1fcfb6fe5328c6ca5c0695ddbc
Dependencies: Stopgaps:

Description

As reported at #17205:

    sage: x, y = ZZ['x','y'].gens()
    sage: GF(1091)['x','y'].random_element().degree(x) ### works
    sage: GF(3037000453)['x','y'].random_element().degree(x) ### fails

The 2nd and 3rd lines are handled by different code, one from multi_polynomial_libsingular.pyx and one from multi_polynomial_element.py. The first tries to convert the argument x into the parent ring, but the second does not.

The point here is to reconcile the approaches: always try to convert the argument into the parent ring.

Change History (10)

comment:1 Changed 5 years ago by jhpalmieri

  • Branch set to u/jhpalmieri/degree_convert_element

comment:2 Changed 5 years ago by jhpalmieri

  • Commit set to 22d541ea43b73e900b9ea72432337b2bac81a492
  • Status changed from new to needs_review

New commits:

22d541eTrac 17366: the degre method for elements of multivariate polynomials rings

comment:3 Changed 5 years ago by git

  • Commit changed from 22d541ea43b73e900b9ea72432337b2bac81a492 to 65e68e16e2bffc1fcfb6fe5328c6ca5c0695ddbc

Branch pushed to git repo; I updated commit sha1. New commits:

65e68e1Trac 17366: additional minor changes

comment:4 Changed 5 years ago by vbraun

Why not just convert (i tried to say that in my sage-devel post, was probably too terse ;-) instead of coercing. E.g. it might be convenient to specify the generator as string if you haven't injected the variables into the current session.

comment:5 Changed 5 years ago by jhpalmieri

This was the path of least resistance: I just copied the code from multi_polynomial_libsingular.pyx. It was therefore the easiest way to make the behavior consistent regardless of the size of the characteristic of the base field.

comment:6 Changed 5 years ago by jhpalmieri

By "convert", do you mean just doing self.parent()(x)?

comment:7 Changed 5 years ago by vbraun

Yes, thats what I meant.

comment:8 Changed 5 years ago by jhpalmieri

Making this change in multi_polynomial_libsingular.pyx breaks a doctest which is supposed to test exactly this:

        The following example is inspired by trac 11652::

            sage: R.<p,q,t> = ZZ[]
            sage: poly = p+q^2+t^3
            sage: poly = poly.polynomial(t)[0]
            sage: poly
            q^2 + p

        There is no canonical coercion from ``R`` to the parent of ``poly``, so
        this doesn't work::

            sage: poly.degree(q)
            Traceback (most recent call last):
            ...
            TypeError: argument must canonically coerce to parent

At #11652, there was some debate about this, and the conclusion was that coercion was appropriate, not conversion. (In the above example, q obviously converts to an element of the parent of poly, but there is no coercion.)

comment:9 Changed 5 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

sounds good

comment:10 Changed 5 years ago by vbraun

  • Branch changed from u/jhpalmieri/degree_convert_element to 65e68e16e2bffc1fcfb6fe5328c6ca5c0695ddbc
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.