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

quo_rem in the polynomial rings does not use canonical coercion #383

Closed
sagetrac-jbmohler mannequin opened this issue Jun 1, 2007 · 10 comments
Closed

quo_rem in the polynomial rings does not use canonical coercion #383

sagetrac-jbmohler mannequin opened this issue Jun 1, 2007 · 10 comments

Comments

@sagetrac-jbmohler
Copy link
Mannequin

sagetrac-jbmohler mannequin commented Jun 1, 2007

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.

Component: basic arithmetic

Author: Robert Bradshaw

Reviewer: William Stein

Merged: sage-4.3.1.rc2

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

@sagetrac-mabshoff sagetrac-mabshoff mannequin added this to the sage-2.10 milestone Sep 11, 2007
@williamstein
Copy link
Contributor

comment:3

Typo: arithmatic --> arithmetic

@robertwb
Copy link
Contributor

comment:4

Attachment: 383-binop-decorator.patch.gz

Oops. Thanks, fixed.

@williamstein
Copy link
Contributor

comment:5

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$

@robertwb
Copy link
Contributor

Attachment: 383-fixes.patch.gz

@robertwb
Copy link
Contributor

comment:6

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

@williamstein
Copy link
Contributor

comment:7

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$

@robertwb
Copy link
Contributor

comment:8

Attachment: 383-more-fixes.patch.gz

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

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jan 19, 2010

Merged: sage-4.3.1.rc2

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jan 19, 2010

Author: Robert Bradshaw

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jan 19, 2010

Reviewer: William Stein

@rlmill rlmill mannequin removed the s: positive review label Jan 19, 2010
@rlmill rlmill mannequin closed this as completed Jan 19, 2010
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

2 participants