Closed orlitzky closed 8 years ago
Description changed:
---
+++
@@ -5,11 +5,13 @@
sage: f1(tau) = (pU + (tau - 1)*(pB - pU)).row()*(pU + (tau - 1)*(pB - pU)).column()
sage: d = (pB-pU).row()*(pB-pU).column()
sage: f2(tau) = tau^2 * d + 2*tau*(pU.row()*(pB-pU).column() - d) + (pB-2*pU).row()*(pB-2*pU).column()
-sage: f1(x) == f2(x)
-([4*(tau - 1)^2]) == ([4*tau^2 - 8*tau + 4])
sage: bool(f1(x) == f2(x))
terminate called after throwing an instance of 'std::runtime_error'
what(): Number_T::hash() python function (__hash__) raised exception
-...
+------------------------------------------------------------------------
+Unhandled SIGABRT: An abort() occurred in Sage.
+This probably occurred because a *compiled* component of Sage has a bug
+in it and is not properly wrapped with sig_on(), sig_off().
+Sage will now terminate.
+------------------------------------------------------------------------
-
Upstream: Reported upstream. Developers acknowledge bug.
Reported as https://github.com/pynac/pynac/issues/34
Minimal case given. In Sage hash(matrix())
will not allow hashing probably because PyObject_Hash
will fail and to prevent that. The fail happens in Pynac nevertheless because the check is avoided due to function expansion and there leads to the crash.
One part of the fix is preventing Sage from crashing. Surrounding the call to Pynac in Expression::__nonzero__
with guards:
sig_on()
pynac_result = relational_to_bool(self._gobj)
sig_off()
leads to:
sage: bool(f(x)==f(x))
terminate called after throwing an instance of 'std::runtime_error'
what(): Number_T::hash() python function (__hash__) raised exception
---------------------------------------------------------------------------
TypeError: mutable matrices are unhashable
which is the same you get with hash(matrix())
, so quite sensible. What more? Why are matrices generated by vector*vector
(the original case) or those in a function definition (like above) mutable at all?
Description changed:
---
+++
@@ -1,11 +1,8 @@
Changed upstream from Reported upstream. Developers acknowledge bug. to Reported upstream. Developers deny it's a bug.
Replying to @rwst:
which is the same you get with
hash(matrix())
, so quite sensible. What more? Why are matrices generated byvector*vector
(the original case) or those in a function definition (like above) mutable at all?
Because the default for matrix creation is to create a mutable matrix. That's because a mutable matrix can be turned into an immutable one, but not the other way around (that requires copying the matrix to a new, mutable, one). That default is probably inconvenient for symbolics.
Are you sure this is the sole problem, though? If we try this:
def imm_mt(*args,**kwargs):
M=matrix(*args,**kwargs)
M.set_immutable()
return M
f(x)=imm_mt()
then we get
sage: hash(f(x))
0
but bool(f(x)==f(x))
still crashes. is it trying to see if f(x)*f(x)^(-1)
is one?
Replying to @nbruin:
Are you sure this is the sole problem, though?
I have reopened the Pynac issue. The crash is in exactly the same call tree. It happens while trying to multiply the rhs with -1
to get lhs-rhs
. So:
sage: f(x)=matrix()
sage: bool(f(x)==1)
True
sage: bool(1==f(x))
<crash>
Also no, I don't think it's the sole problem. Either Sage or Pynac will have to check somewhere before evaluating that all mutable Python objects of the expression are switched to immutable (or make immutable copies).
EDIT: But that would be a different ticket: #18360
Please read http://docs.cython.org/src/userguide/wrapping_CPlusPlus.html#exceptions
Changed upstream from Reported upstream. Developers deny it's a bug. to none
Author: Ralf Stephan
Replying to @nbruin:
If we try this:
def imm_mt(*args,**kwargs): M=matrix(*args,**kwargs) M.set_immutable() return M f(x)=imm_mt()
then we get
sage: hash(f(x)) 0
but
bool(f(x)==f(x))
still crashes.
My best guess at the moment is that somewhere in between a copy is made which, as you suggested, is then mutable again. This can only be fixed by a dedicated ticket such as #18360.
As to this code, the reviewer should check doubly if what I did here was right. There was not a single example in Sage with a special function catching exceptions from which I could have learned.
New commits:
b9698e0 | 14211: catch exceptions from Expression.__nonzero__ |
Obviously, a doctest is needed.
Besides that, it makes no sense to add an additional layer _nonzero_
. The exception should be handled in the pynac interface, i.e. the relational_to_bool()
function.
Changed branch from u/rws/crash_due_to_missing_cython_exception_handling_for_relational_to_bool__ to u/rws/14211
Interesting. The C++ layer outputs an error message, and the Python layer deals with the error too, because presumably the error from calling PyHash
from the C++ layer is still pending.
New commits:
e1374da | 14211: catch Pynac exceptions |
Replying to @rwst:
Interesting. The C++ layer outputs an error message, and the Python layer deals with the error too, because presumably the error from calling
PyHash
from the C++ layer is still pending.
Well, behaviour shouldn't depend on what "presumably" happens (does the traceback look sensible? If not, you're certainly doing it wrong). I admit I don't know the proper way to deal with this (a C++ exception and PyErr_Occurred() != NULL
), but it's not right like this.
Besides that, I would never print the C++ exception.
I can have a look at how to properly deal with this, but not now.
Changed author from Ralf Stephan to none
Changed branch from u/rws/14211 to none
Now that the actual bug is fixed in future Pynac (#18360) the given doctest will not work in the future, so I removed the commit.
Branch reset because other exceptions can be thrown.
New commits:
e1374da | 14211: catch Pynac exceptions |
Branch: u/rws/14211
Changed branch from u/rws/14211 to none
No longer necessary (doctest added in #18360)
Component: symbolics
Issue created by migration from https://trac.sagemath.org/ticket/14211