Opened 11 years ago

Closed 8 years ago

#383 closed defect (fixed)

quo_rem in the polynomial rings does not use canonical coercion

Reported by: jbmohler Owned by: somebody
Priority: minor Milestone: sage-4.3.1
Component: basic arithmetic Keywords:
Cc: Merged in: sage-4.3.1.rc2
Authors: Robert Bradshaw Reviewers: William Stein
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

I'm looking at the polynomial function quo_rem and I see that it does it's own coercion manually. This feels a little wrong to me. I think it should go through the standard coercion routines. Here's a "bug" that results:

sage: x=ZZ['x'].0
sage: y=QQ['x'].0
sage: (y+1).quo_rem(1/2*x)
(2, 1)
sage: (x+1).quo_rem(1/2*y)
...
<type 'exceptions.TypeError'>: no coercion of this rational to integer

The bug is that I don't see why these two things are treated substantially differently. The reason I found this is because the simple "TypeError?" exception did not provide the usual message about parents being mis-matched -- I think this is a bug in itself

The fix for all that is to make the quo_rem stuff use canonical coercion model.

All of the quo_rem instances in sage/rings/polynomial/polynomial_element_generic.py suffer from some sort of coercion impropriety.

Attachments (3)

383-binop-decorator.patch (25.6 KB) - added by robertwb 8 years ago.
383-fixes.patch (2.4 KB) - added by robertwb 8 years ago.
383-more-fixes.patch (4.1 KB) - added by robertwb 8 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 10 years ago by mabshoff

  • Milestone set to Sage-2.10

comment:2 Changed 8 years ago by robertwb

  • Report Upstream set to N/A
  • Status changed from new to needs_review

comment:3 Changed 8 years ago by was

Typo: arithmatic --> arithmetic

Changed 8 years ago by robertwb

comment:4 Changed 8 years ago by robertwb

Oops. Thanks, fixed.

comment:5 Changed 8 years ago by was

I read the code. Looks AWESOME!

It appears to expose numerous issues:

sage -t devel/sage/sage/rings
...

The following tests failed:

        sage -t  devel/sage/sage/rings/finite_field_element.py # 3 doctests failed
        sage -t  devel/sage/sage/rings/tests.py # 1 doctests failed
        sage -t  devel/sage/sage/rings/finite_field_ext_pari.py # 1 doctests failed
        sage -t  devel/sage/sage/rings/fraction_field_element.pyx # 1 doctests failed
        sage -t  devel/sage/sage/rings/polynomial/polynomial_integer_dense_flint.pyx # 1 doctests failed
        sage -t  devel/sage/sage/rings/residue_field.pyx # 3 doctests failed
        sage -t  devel/sage/sage/rings/polynomial/polynomial_zmod_flint.pyx # 5 doctests failed
        sage -t  devel/sage/sage/rings/number_field/number_field_ideal.py # 2 doctests failed
----------------------------------------------------------------------
Total time for all tests: 192.3 seconds

[1]+  Done                    ./sage -tp 10 devel/sage/sage/rings > 383.out
wstein@boxen:~/build/sage-4.3.1.rc0$ pwd
/home/wstein/build/sage-4.3.1.rc0
wstein@boxen:~/build/sage-4.3.1.rc0$ ls
383.out   6207.out~    data   dist      install.log  local     README.txt  sage-python          spkg      tmp
6207.out  COPYING.txt  devel  examples  ipython      makefile  sage        sage-README-osx.txt  test.log
wstein@boxen:~/build/sage-4.3.1.rc0$ pwd
/home/wstein/build/sage-4.3.1.rc0
wstein@boxen:~/build/sage-4.3.1.rc0$

Changed 8 years ago by robertwb

comment:6 Changed 8 years ago by robertwb

OK, I've fixed all the above doctests issues.

comment:7 Changed 8 years ago by was

  • Status changed from needs_review to needs_work

Ut oh:

----------------------------------------------------------------------

The following tests failed:

        sage -t  devel/sage/sage/crypto/classical.py # 14 doctests failed
        sage -t  devel/sage/sage/modular/etaproducts.py # 24 doctests failed
        sage -t  devel/sage/sage/structure/element.pyx # 1 doctests failed
        sage -t  devel/sage/sage/libs/pari/gen.pyx # Segfault
        sage -t  devel/sage/sage/modular/arithgroup/arithgroup_generic.py # 4 doctests failed
        sage -t  devel/sage/sage/modular/arithgroup/congroup_gamma0.py # 2 doctests failed
        sage -t  devel/sage/sage/quadratic_forms/quadratic_form__split_local_covering.py # 2 doctests failed
        sage -t  devel/sage/sage/modular/cusps.py # 1 doctests failed
----------------------------------------------------------------------
Total time for all tests: 478.6 seconds
wstein@boxen:~/build/sage-4.3.1.rc0-boxen-x86_64-Linux$

Changed 8 years ago by robertwb

comment:8 Changed 8 years ago by robertwb

  • Status changed from needs_work to needs_review

OK, I've doctested the entire sage library this time.

comment:9 Changed 8 years ago by was

  • Status changed from needs_review to positive_review

comment:10 Changed 8 years ago by rlm

  • Authors set to Robert Bradshaw
  • Merged in set to sage-4.3.1.rc2
  • Resolution set to fixed
  • Reviewers set to William Stein
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.