Opened 5 years ago

Closed 5 years ago

#18622 closed enhancement (fixed)

Improve true division support in coercion model

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-6.8
Component: cython Keywords: python3
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers: Wilfried Luebbe
Report Upstream: N/A Work issues:
Branch: f61588e (Commits) Commit: f61588ecabefc84103c4daa40e648b76e6e58828
Dependencies: Stopgaps:

Description


Change History (12)

comment:1 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/improve_true_division_support_in_coercion_model

comment:2 Changed 5 years ago by git

  • Commit set to c32c313c95bb47b93c9970eac4ddda7bdad00e7e

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c32c313Improve true division support in coercion framework

comment:3 Changed 5 years ago by git

  • Commit changed from c32c313c95bb47b93c9970eac4ddda7bdad00e7e to f61588ecabefc84103c4daa40e648b76e6e58828

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f61588eImprove true division support in coercion framework

comment:4 Changed 5 years ago by jdemeyer

  • Status changed from new to needs_review

comment:5 follow-up: Changed 5 years ago by wluebbe

For both coerce.pyx and element.pyx in the lines

-cdef add, sub, mul, div, iadd, isub, imul, idiv
-from operator import add, sub, mul, div, iadd, isub, imul, idiv
+cdef add, sub, mul, div, truediv, iadd, isub, imul, idiv
+from operator import add, sub, mul, div, truediv, iadd, isub, imul, idiv

you inserted truediv but not itruediv. Was this done for a reason?

comment:6 follow-up: Changed 5 years ago by wluebbe

  • Status changed from needs_review to needs_work

Testing --all --long I got 4 doctest failures:

sage -t --long src/sage/structure/element.pyx
**********************************************************************
File "src/sage/structure/element.pyx", line 1695, in sage.structure.element.MultiplicativeGroupElement.__truediv__
Failed example:
    operator.truediv(2, K.ideal(i+1))
Expected:
    Fractional ideal (-i + 1)
Got:
    Fractional ideal (i + 1)
**********************************************************************
File "src/sage/structure/element.pyx", line 3042, in sage.structure.element.Matrix.__truediv__
Failed example:
    operator.truediv(a, b)
Exception raised:
    Traceback (most recent call last):
      File "/home/wluebbe/sage-6.8.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/wluebbe/sage-6.8.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.structure.element.Matrix.__truediv__[4]>", line 1, in <module>
        operator.truediv(a, b)
    TypeError: unsupported operand type(s) for /: 'sage.matrix.matrix_integer_dense.Matrix_integer_dense' and 'sage.matrix.matrix_integer_dense.Matrix_integer_dense'
**********************************************************************
File "src/sage/structure/element.pyx", line 3046, in sage.structure.element.Matrix.__truediv__
Failed example:
    operator.truediv(c, a)
Exception raised:
    Traceback (most recent call last):
      File "/home/wluebbe/sage-6.8.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/wluebbe/sage-6.8.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.structure.element.Matrix.__truediv__[6]>", line 1, in <module>
        operator.truediv(c, a)
    TypeError: unsupported operand type(s) for /: 'sage.matrix.matrix_rational_dense.Matrix_rational_dense' and 'sage.matrix.matrix_integer_dense.Matrix_integer_dense'
**********************************************************************
2 items had failures:
   2 of   8 in sage.structure.element.Matrix.__truediv__
   1 of   3 in sage.structure.element.MultiplicativeGroupElement.__truediv__
    [538 tests, 3 failures, 12.56 s]

and

sage -t --long src/sage/rings/number_field/number_field_ideal.py
**********************************************************************
File "src/sage/rings/number_field/number_field_ideal.py", line 1815, in sage.rings.number_field.number_field_ideal.NumberFieldFractionalIdeal._div_
Failed example:
    I/J
Expected:
    Fractional ideal (-11/1420*a + 9/284)
Got:
    Fractional ideal (-17/1420*a + 1/284)
**********************************************************************
1 item had failures:
   1 of   7 in sage.rings.number_field.number_field_ideal.NumberFieldFractionalIdeal._div_
    [657 tests, 1 failure, 10.74 s]

comment:7 in reply to: ↑ 5 Changed 5 years ago by jdemeyer

Replying to wluebbe:

Was this done for a reason?

Well, inplace optimizations are disabled anyway (see #1069), so it doesn't really matter.

comment:8 in reply to: ↑ 6 Changed 5 years ago by jdemeyer

  • Status changed from needs_work to needs_review

Replying to wluebbe:

Testing --all --long I got 4 doctest failures:

Obvious question: did you run make?

comment:9 Changed 5 years ago by wluebbe

Well ... I always do ./sage -b;./sage -tp 4 --all --long >logs/sage-tp4-all-long-18622-2.log ... but this time ./sage -b; was missing ... I don't know how ... Mea culpa - grrr

comment:10 Changed 5 years ago by wluebbe

  • Status changed from needs_review to positive_review

All tests passed! LGTM.

comment:11 Changed 5 years ago by wluebbe

  • Keywords python3 added
  • Reviewers set to Wilfried Luebbe

comment:12 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/improve_true_division_support_in_coercion_model to f61588ecabefc84103c4daa40e648b76e6e58828
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.