#16276: fixes PolynomialElement.mod()
-------------------------------------+-------------------------------------
Reporter: Bouillaguet | Owner:
Type: defect | Status: needs_review
Priority: major | Milestone: sage-6.3
Component: basic arithmetic | Resolution:
Keywords: | Merged in:
Authors: Charles | Reviewers:
Bouillaguet | Work issues:
Report Upstream: N/A | Commit:
Branch: | 0061d17ca5e04465c1189d62aa032f98a0c120cf
u/Bouillaguet/ticket/16276 | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Comment (by pbruin):
Replying to [comment:20 Bouillaguet]:
> I made a case for {{{quo_rem()}}} implementing **euclidean division** in
https://groups.google.com/d/msg/sage-devel/UVrx5CG4qA0/WUz2RItJtF8J Nobody
complained (and John approved).
Sorry, I did not closely follow the sage-devel discussion and only
realised I didn't like this approach after seeing your patch.
Actually I interpreted John's comment:7 as saying that he was against
raising an error given that there is also a reasonable answer that we can
return without raising an error. Maybe he can clarify his opinion?
> What you ask for can be obtained (as you suggested) by other means.
What I'm most concerned about is that examples which used to work now
raise an error. For example, I think the following examples (of the FLINT
implementation) are mathematically perfectly valid:
{{{
sage: R.<x> = ZZ[]
sage: (x^100).quo_rem(2*x+1)
(0, x^100)
sage: (5*x+7).quo_rem(3)
(x + 2, 2*x + 1)
}}}
Is there a simple way to obtain these results after your patch?
> Note that depending on the ring, and on the implementation, quo_rem does
not do the same thing. For instance, the NTL implementation of ZZ[X] does
what I ask for, but the FLINT implementation of ZZ[X] does what you
requested. This situation calls for clarification.
Yes, I agree that we should come up with a consistent approach. I can see
that '''Z'''[''x''] (in the FLINT implementation) and ''K''[''t''][''x'']
(for a field ''K'') behave inconsistently and that this should be fixed.
However, I also think one should think twice before choosing a solution
that fails in more cases, rather than a solution that returns a result in
more cases.
> Note that {{{src/sage/structure/element.pyx}}} asks that {{{quo_rem()}}}
and {{{mod()}}} implement euclidean division, or fail if the result is not
mathematically defined... I think that this is fine (if anything, it may
help students realize that the result is not defined !).
I don't agree that the result is necessarily undefined; that depends on
what definition you use. FLINT in fact uses a definition of `quo_rem(f,
g)` which is valid for all `f` and `g != 0` (the one I described), and
with your patch we would simply refuse to return this answer in many
cases. (Note that the current version of the patch does not seem to
conform to the documentation either: you check for the leading coefficient
being 1 instead of any unit, and you don't raise an error if the remainder
is 0.)
I think that we should interpret the documentation as what it is, namely
''documentation'' (descriptive), not a ''specification'' (prescriptive).
In particular, it may be clarified/amended if necessary, and it should not
force methods in derived classes to raise errors in all cases where the
original method would raise an error. In this particular case it should
allow (not necessarily force) `quo_rem()` to use a more general algorithm
if the base ring is a Euclidean domain that is not a field, e.g. if the
polynomial ring is '''Z'''[''x''] (base ring '''Z''') or
''K''[''t''][''x''] (base ring ''K''[''t'']).
--
Ticket URL: <http://trac.sagemath.org/ticket/16276#comment:21>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
