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

Failing weak order assertion on random symbolic expression #32185

Closed
kliem opened this issue Jul 12, 2021 · 11 comments
Closed

Failing weak order assertion on random symbolic expression #32185

kliem opened this issue Jul 12, 2021 · 11 comments

Comments

@kliem
Copy link
Contributor

kliem commented Jul 12, 2021

File "src/sage/symbolic/random_tests.py", line 430, in sage.symbolic.random_tests.test_symbolic_expression_order
Failed example:
    test_symbolic_expression_order(10000)  # long time
Exception raised:
    Traceback (most recent call last):
      File "/srv/public/kliem/sage/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 718, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/srv/public/kliem/sage/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 1137, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.symbolic.random_tests.test_symbolic_expression_order[2]>", line 1, in <module>
        test_symbolic_expression_order(Integer(10000))  # long time
      File "/srv/public/kliem/sage/local/lib/python3.7/site-packages/sage/symbolic/random_tests.py", line 456, in test_symbolic_expression_order
        assert_strict_weak_order(a, b, c, mixed_order)
      File "/srv/public/kliem/sage/local/lib/python3.7/site-packages/sage/symbolic/random_tests.py", line 387, in assert_strict_weak_order
        cmp_M[i, j] = (cmp_func(x[i], x[j]) == 1)   # or -1, doesn't matter
      File "sage/symbolic/comparison.pyx", line 228, in sage.symbolic.comparison.mixed_order (build/cythonized/sage/symbolic/comparison.cpp:4349)
        cpdef int mixed_order(lhs, rhs) except -2:
      File "sage/symbolic/comparison.pyx", line 268, in sage.symbolic.comparison.mixed_order (build/cythonized/sage/symbolic/comparison.cpp:4126)
        less_than = _mixed_key(lhs) < _mixed_key(rhs)
      File "sage/symbolic/comparison.pyx", line 355, in sage.symbolic.comparison._mixed_key.__lt__ (build/cythonized/sage/symbolic/comparison.cpp:5238)
        return det_ex < 0
    TypeError: '<' not supported between instances of 'Pi' and 'int'
**********************************************************************
1 item had failures:
   2 of   4 in sage.symbolic.random_tests.test_symbolic_expression_order
    [49 tests, 2 failures, 3.59 s]

The underlying problem is the following:

sage: pi = sage.symbolic.constants.Pi()                                                                                                                                                                                                                                                                                                                                    
sage: pi < 0                                                                                                                                                                                                                                                                                                                                                               
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-7-f23f08971c70> in <module>
----> 1 pi < Integer(0)

/srv/public/kliem/sage/local/lib/python3.7/site-packages/sage/rings/integer.pyx in sage.rings.integer.Integer.__richcmp__ (build/cythonized/sage/rings/integer.c:7888)()
    947             c = mpz_cmp_d((<Integer>left).value, d)
    948         else:
--> 949             return coercion_model.richcmp(left, right, op)
    950 
    951         return rich_to_bool_sgn(op, c)

/srv/public/kliem/sage/local/lib/python3.7/site-packages/sage/structure/coerce.pyx in sage.structure.coerce.CoercionModel.richcmp (build/cythonized/sage/structure/coerce.c:20854)()
   2006             raise bin_op_exception('<=', x, y)
   2007         elif op == Py_GT:
-> 2008             raise bin_op_exception('>', x, y)
   2009         else:
   2010             raise bin_op_exception('>=', x, y)

TypeError: unsupported operand parent(s) for >: 'Integer Ring' and '<class 'sage.symbolic.constants.Pi'>'

Component: symbolics

Keywords: pi, random doctests

Author: Michael Orlitzky

Branch/Commit: 551d3ae

Reviewer: Travis Scrimshaw

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

@kliem kliem added this to the sage-9.4 milestone Jul 12, 2021
@kliem

This comment has been minimized.

@orlitzky
Copy link
Contributor

Branch: u/mjo/ticket/32185

@orlitzky
Copy link
Contributor

comment:2

I wasn't able to reproduce the failure, but I'm pretty sure that Pi is arising from pi.pyobject() during the comparison. In any case, it looks like I've fixed at least one bug. I'll test the new branch overnight.


New commits:

1ab9784Trac #32243: patch gfan to look for cddlib headers in new location.
cfab03aTrac #32243: look for cddlib-0.94m headers in the right place.
b30ce37Trac #32185: special case constant symbolic relations.

@orlitzky
Copy link
Contributor

Commit: b30ce37

@orlitzky
Copy link
Contributor

Author: Michael Orlitzky

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 21, 2021

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

551d3aeTrac #32185: special case constant symbolic relations.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 21, 2021

Changed commit from b30ce37 to 551d3ae

@orlitzky
Copy link
Contributor

comment:4

Tests pass. I dropped the two commits from #32243 that I left in there by mistake.

@tscrim
Copy link
Collaborator

tscrim commented Jul 22, 2021

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jul 22, 2021

comment:5

LGTM.

@vbraun
Copy link
Member

vbraun commented Jul 24, 2021

Changed branch from u/mjo/ticket/32185 to 551d3ae

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

4 participants