Opened 5 years ago
Closed 4 years ago
#20939 closed defect (fixed)
Fix crash and remove pexpectMaxima usage in Y(m,n)
Reported by:  rws  Owned by:  

Priority:  major  Milestone:  sage7.5 
Component:  symbolics  Keywords:  
Cc:  Merged in:  
Authors:  Ralf Stephan  Reviewers:  Travis Scrimshaw 
Report Upstream:  Fixed upstream, in a later stable release.  Work issues:  
Branch:  bb4630f (Commits, GitHub, GitLab)  Commit:  bb4630fa7a2ac6b9b0a1d98486378877fe60f97c 
Dependencies:  #21614, #21963  Stopgaps: 
Description (last modified by )
sage: spherical_harmonic(3, 2, 1, 2*pi/3) ... TypeError: ECL says: THROW: The catch MACSYMAQUIT is undefined.
The eval function of SphericalHarmonic
uses pexpectMaxima. Since this needs only a formula the call to Maxima could be replaced by a few lines of Python.
http://dlmf.nist.gov/14.30.E1
Replacing pexpectMaxima is part of #17753.
Change History (35)
comment:1 Changed 5 years ago by
 Dependencies set to 16813
comment:2 Changed 5 years ago by
 Dependencies changed from 16813 to #16813
comment:3 Changed 5 years ago by
It won't be dependent on P(l,m,x)
if the P special value formula is directly used in _eval_
. The Y floating point evaluation is done via mpmath anyway.
comment:4 Changed 5 years ago by
 Dependencies #16813 deleted
 Description modified (diff)
 Summary changed from Remove pexpectMaxima usage in Y(m,n) to Fix crash and remove pexpectMaxima usage in Y(m,n)
 Type changed from enhancement to defect
comment:5 Changed 5 years ago by
 Branch set to u/rws/remove_pexpect_maxima_usage_in_y_m_n_
comment:6 Changed 5 years ago by
 Commit set to f233afa35058ab40a1f6f33e6d635e6eac515f97
 Milestone changed from sage7.3 to sage7.4
 Status changed from new to needs_review
New commits:
f233afa  20939: Fix crash and remove pexpectMaxima usage in Y(m,n)

comment:7 Changed 5 years ago by
 Reviewers set to Travis Scrimshaw
LGTM modulo
Check that :trac:`20939` is fixed:: sage: spherical_harmonic(3,2,1,2*pi/3) 1/240*sqrt(30)*(15*I*sqrt(7)*sqrt(3)   ...15*sqrt(7))*cos(1)*sin(1)^2/sqrt(pi) + ... + 15*sqrt(7))*cos(1)*sin(1)^2/sqrt(pi) """ if n in ZZ and m in ZZ and n > 1: if abs(m) > n: return ZZ(0)  if (m == 0 and theta.is_zero()): + if m == 0 and theta.is_zero(): return sqrt((2*n+1)/4/pi)
comment:8 Changed 5 years ago by
 Commit changed from f233afa35058ab40a1f6f33e6d635e6eac515f97 to 1a5cf6b9dbf75041e634a5c0d27b0a2a09932834
Branch pushed to git repo; I updated commit sha1. New commits:
1a5cf6b  20939: cosmetics

comment:9 Changed 5 years ago by
 Status changed from needs_review to positive_review
That doesn't work as given. First the plus is a minus, and then the space after the ellipsis will make it fail too.
I'll assume the last commit is still what you wanted and set positive. Just revert if not. Thanks for the review.
comment:10 Changed 5 years ago by
 Branch changed from u/rws/remove_pexpect_maxima_usage_in_y_m_n_ to u/tscrim/remove_pexpect_maxima_in_ymn20939
 Commit changed from 1a5cf6b9dbf75041e634a5c0d27b0a2a09932834 to a080e1e8824bf56a760d54cbd7e6c9b79c351cef
 Status changed from positive_review to needs_review
So the ...
is actually unneeded; I was thinking there was more than 2 terms. Please check my changes.
New commits:
a080e1e  Removing uncessary ... in doctest.

comment:11 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:12 Changed 5 years ago by
 Status changed from positive_review to needs_work
Documentation doesn't build, see patchbot
comment:13 Changed 5 years ago by
Interesting. I get this
[dochtml] [plot3d ] /home/ralf/sage/src/doc/en/reference/plot3d/sage/plot/plot3d/plot3d.rst:1937: WARNING: Exception occurred in plotting plot3d35 [dochtml] [plot3d ] from /home/ralf/sage/src/doc/en/reference/plot3d/sage/plot/plot3d/plot3d.rst: [dochtml] [plot3d ] Traceback (most recent call last): [dochtml] [plot3d ] File "/home/ralf/sage/local/lib/python2.7/sitepackages/matplotlib1.5.1py2.7linuxx86_64.egg/matplotlib/sphinxext/plot_directive.py", line 517, in run_code
I don't know how to get to the plot3d35
plot commands but:
[dochtml] [plot3d ] result[0] = interp_rdf(args [dochtml] [plot3d ] File "sage/symbolic/function.pyx", line 821, in sage.symbolic.function.GinacFunction.__call__ (build/cythonized/sage/symbolic/function.cpp:9409) [dochtml] [plot3d ] res = super(GinacFunction, self).__call__(*args, **kwds) [dochtml] [plot3d ] File "sage/symbolic/function.pyx", line 972, in sage.symbolic.function.BuiltinFunction.__call__ (build/cythonized/sage/symbolic/function.cpp:11034) [dochtml] [plot3d ] res = super(BuiltinFunction, self).__call__( [dochtml] [plot3d ] File "sage/symbolic/function.pyx", line 488, in sage.symbolic.function.Function.__call__ (build/cythonized/sage/symbolic/function.cpp:6886) [dochtml] [plot3d ] res = g_function_eval2(self._serial, (<Expression>args[0])._gobj, [dochtml] [plot3d ] RuntimeError: arctan2_eval(): arctan2(0,0) encountered
The only calls to atan2(x,y)
are in revolution_plot3d()
and if I remove all its doctests it still crashes on build at this point, so I no longer know where to look. Either the 3d plot number 35 or the uncompiled plot3d.rst
would be necessary.
Apart from the bug at hand I think Pynac's atan2_eval
should return unsigned infinity instead of throwing an exception. There is a reason atan2(0,0)
is called however.
comment:14 Changed 4 years ago by
This is the trigger:
sage: phi, theta = var('phi, theta') sage: Y = spherical_harmonic(2, 1, theta, phi) sage: rea = spherical_plot3d(abs(real(Y)), (phi,0,2*pi), (theta,0,pi), color='bl ....: ue', opacity=0.6) sage: ima = spherical_plot3d(abs(imag(Y)), (phi,0,2*pi), (theta,0,pi), color='re ....: d', opacity=0.6) sage: (rea + ima).show(aspect_ratio=1)  RuntimeError: arctan2_eval(): arctan2(0,0) encountered
comment:15 Changed 4 years ago by
The new code makes Y
expand immediately, and the real/imag
calls then really blow it up.
sage: Y = 1/4*sqrt(6)*sqrt(5)*sqrt(sin(theta)^2)*cos(theta)*e^(I*phi)/sqrt(pi) sage: abs(real(Y)) abs(1/4*sqrt(6)*sqrt(5)*sqrt(abs(sin(theta))^2)*cos(1/2*\ arctan2(2*cos(real_part(theta))*cosh(imag_part(theta))*sin(real_part(theta))*sinh(imag_part(theta)), cosh(imag_part(theta))^2*sin(real_part(theta))^2cos(real_part(theta))^2*sinh(imag_part(theta))^2)) *cos(real_part(phi))*cos(real_part(theta))*cosh(imag_part(theta))*e^(imag_part(phi))/sqrt(pi)  1/4*sqrt(6)*sqrt(5)*sqrt(abs(sin(theta))^2)*cos(real_part(theta))*cosh(imag_part(theta))*e^(imag_part(phi))*sin(1/2*arctan2(2*cos(real_part(theta))*cosh(imag_part(theta))*sin(real_part(theta))*sinh(imag_part(theta)), cosh(imag_part(theta))^2*sin(real_part(theta))^2  cos(real_part(theta))^2*sinh(imag_part(theta))^2))*sin(real_part(phi))/sqrt(pi) + 1/4*sqrt(6)*sqrt(5)*sqrt(abs(sin(theta))^2)*cos(real_part(phi))*e^(imag_part(phi))*sin(1/2*arctan2(2*cos(real_part(theta))*cosh(imag_part(theta))*sin(real_part(theta))*sinh(imag_part(theta)), cosh(imag_part(theta))^2*sin(real_part(theta))^2  cos(real_part(theta))^2*sinh(imag_part(theta))^2))*sin(real_part(theta))*sinh(imag_part(theta))/sqrt(pi) + 1/4*sqrt(6)*sqrt(5)*sqrt(abs(sin(theta))^2)*cos(1/2*arctan2(2*cos(real_part(theta))*cosh(imag_part(theta))*sin(real_part(theta))*sinh(imag_part(theta)), cosh(imag_part(theta))^2*sin(real_part(theta))^2  cos(real_part(theta))^2*sinh(imag_part(theta))^2))*e^(imag_part(phi))*sin(real_part(phi))*sin(real_part(theta))*sinh(imag_part(theta))/sqrt(pi))
comment:16 Changed 4 years ago by
sage: x=abs(real(Y)) sage: x.subs(theta==0) ... RuntimeError: arctan2_eval(): arctan2(0,0) encountered sage: y=abs(imag(Y)) sage: y.subs(theta==0) ... RuntimeError: arctan2_eval(): arctan2(0,0) encountered sage: Y 1/4*sqrt(6)*sqrt(5)*sqrt(sin(theta)^2)*cos(theta)*e^(I*phi)/sqrt(pi) sage: Y.subs(theta==0) 0
So it seems the real/imag
expansions produce results in some cases that are not defined at zero, so are not correct.
comment:17 Changed 4 years ago by
 Dependencies set to #21614
comment:18 Changed 4 years ago by
 Status changed from needs_work to positive_review
So to summarize the code here is good, we just depend on #21614.
comment:19 Changed 4 years ago by
 Status changed from positive_review to needs_work
File "src/sage/plot/plot3d/plot3d.py", line 1195, in sage.plot.plot3d.plot3d.spherical_plot3d Failed example: (rea + ima).show(aspect_ratio=1) # long time (4s on sage.math, 2011) Exception raised: Traceback (most recent call last): File "/home/chapoton/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 498, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/chapoton/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 861, in compile_and_execute exec(compiled, globs) File "<doctest sage.plot.plot3d.plot3d.spherical_plot3d[13]>", line 1, in <module> (rea + ima).show(aspect_ratio=Integer(1)) # long time (4s on sage.math, 2011) File "sage/plot/plot3d/base.pyx", line 1379, in sage.plot.plot3d.base.Graphics3d.show (build/cythonized/sage/plot/plot3d/base.c:17925) dm.display_immediately(self, **kwds) File "/home/chapoton/sage/local/lib/python2.7/sitepackages/sage/repl/rich_output/display_manager.py", line 791, in display_immediately plain_text, rich_output = self._rich_output_formatter(obj, rich_repr_kwds) File "/home/chapoton/sage/local/lib/python2.7/sitepackages/sage/repl/rich_output/display_manager.py", line 623, in _rich_output_formatter rich_output = self._call_rich_repr(obj, rich_repr_kwds) File "/home/chapoton/sage/local/lib/python2.7/sitepackages/sage/repl/rich_output/display_manager.py", line 581, in _call_rich_repr return obj._rich_repr_(self, **rich_repr_kwds) File "sage/plot/plot3d/base.pyx", line 130, in sage.plot.plot3d.base.Graphics3d._rich_repr_ (build/cythonized/sage/plot/plot3d/base.c:4422) opts = self._process_viewing_options(kwds) File "sage/plot/plot3d/base.pyx", line 1257, in sage.plot.plot3d.base.Graphics3d._process_viewing_options (build/cythonized/sage/plot/plot3d/base.c:17397) self._determine_frame_aspect_ratio(opts['aspect_ratio']) File "sage/plot/plot3d/base.pyx", line 472, in sage.plot.plot3d.base.Graphics3d._determine_frame_aspect_ratio (build/cythonized/sage/plot/plot3d/base.c:9247) a_min, a_max = self._safe_bounding_box() File "sage/plot/plot3d/base.pyx", line 490, in sage.plot.plot3d.base.Graphics3d._safe_bounding_box (build/cythonized/sage/plot/plot3d/base.c:9420) a_min, a_max = self.bounding_box() File "sage/plot/plot3d/base.pyx", line 1831, in sage.plot.plot3d.base.Graphics3dGroup.bounding_box (build/cythonized/sage/plot/plot3d/base.c:24445) v = [obj.bounding_box() for obj in self.all] File "sage/plot/plot3d/parametric_surface.pyx", line 379, in sage.plot.plot3d.parametric_surface.ParametricSurface.bounding_box (build/cythonized/sage/plot/plot3d/parametric_surface.c:5079) self.triangulate() File "sage/plot/plot3d/parametric_surface.pyx", line 427, in sage.plot.plot3d.parametric_surface.ParametricSurface.triangulate (build/cythonized/sage/plot/plot3d/parametric_surface.c:5826) raise File "sage/plot/plot3d/parametric_surface.pyx", line 422, in sage.plot.plot3d.parametric_surface.ParametricSurface.triangulate (build/cythonized/sage/plot/plot3d/parametric_surface.c:5746) self.eval_grid(urange, vrange) File "sage/plot/plot3d/parametric_surface.pyx", line 601, in sage.plot.plot3d.parametric_surface.ParametricSurface.eval_grid (build/cythonized/sage/plot/plot3d/parametric_surface.c:7437) (<Wrapper_rdf>fx).call_c(uv, &self.vs[i*n+j].x) File "sage/ext/interpreters/wrapper_rdf.pyx", line 90, in sage.ext.interpreters.wrapper_rdf.Wrapper_rdf.call_c (build/cythonized/sage/ext/interpreters/wrapper_rdf.c:2163) result[0] = interp_rdf(args File "sage/symbolic/expression.pyx", line 1410, in sage.symbolic.expression.Expression.__float__ (build/cythonized/sage/symbolic/expression.cpp:11032) ret = float(self._eval_self(float)) File "sage/symbolic/expression.pyx", line 1197, in sage.symbolic.expression.Expression._eval_self (build/cythonized/sage/symbolic/expression.cpp:9741) res = self._gobj.evalf(0, {'parent':R}) File "sage/libs/pynac/pynac.pyx", line 2160, in sage.libs.pynac.pynac.py_eval_constant (build/cythonized/sage/libs/pynac/pynac.cpp:25827) constant = constants_table[serial] KeyError: 3
comment:20 Changed 4 years ago by
I should say I saw it too. plot3d35
is still problematic with this here.
comment:21 Changed 4 years ago by
 Branch changed from u/tscrim/remove_pexpect_maxima_in_ymn20939 to u/rws/remove_pexpect_maxima_in_ymn20939
comment:22 Changed 4 years ago by
 Commit changed from a080e1e8824bf56a760d54cbd7e6c9b79c351cef to 31e6c649c586363c621f2daabf805283be10775a
 Status changed from needs_work to needs_review
comment:23 Changed 4 years ago by
Would it be possible to have a test to make sure this stays fixed?
comment:24 Changed 4 years ago by
 Commit changed from 31e6c649c586363c621f2daabf805283be10775a to 04f79cdf8ed0c887b7f7d1797ce5023f782d0a50
Branch pushed to git repo; I updated commit sha1. New commits:
04f79cd  20939: make sure atan2(0,0) returns always NaN

comment:25 Changed 4 years ago by
This commit contains a test for the specific NaN
evaluation problem causing the second crash (the fix for the first crash is tested with #21614). The commit also fixes an inconsistency. As to a general test of the second problem I opened https://github.com/pynac/pynac/issues/211
comment:26 Changed 4 years ago by
 Status changed from needs_review to positive_review
Thanks. Now back to the buildbots.
comment:27 Changed 4 years ago by
I don know what Volker will report if anything but now I have a segfault in constant.py
sage: sig_on_count() # check sig_on/off pairings (virtual doctest) ## line 688 ## 0 sage: loads(dumps(NaN)) ## line 704 ##  /usr/lib64/python2.7/sitepackages/cysignals/signals.so(+0x4556)[0x7f4f180eb556] /usr/lib64/python2.7/sitepackages/cysignals/signals.so(+0x4a6c)[0x7f4f180eba6c] /usr/lib64/python2.7/sitepackages/cysignals/signals.so(+0x6d6f)[0x7f4f180edd6f] /lib64/libpthread.so.0(+0x10d70)[0x7f4f1ce53d70] /usr/lib64/python2.7/sitepackages/sage/libs/pynac/pynac.so(+0x2940d)[0x7f4d3ece640d] /usr/lib64/libpynac.so.8(_ZN5GiNaC8constant9unarchiveERKNS_12archive_nodeERNS_9containerINSt7__cxx114listEEE+0x196)[0x7f4d3e955dcc] /usr/lib64/libpynac.so.8(_ZNK5GiNaC12archive_node9unarchiveERNS_9containerINSt7__cxx114listEEE+0xf4)[0x7f4d3e936836] /usr/lib64/libpynac.so.8(_ZNK5GiNaC7archive12unarchive_exERKNS_9containerINSt7__cxx114listEEEj+0x130)[0x7f4d3e936a42] /usr/lib64/python2.7/sitepackages/sage/symbolic/expression.so(+0xec87f)[0x7f4d3e63787f] /usr/lib64/python2.7/sitepackages/sage/symbolic/expression.so(+0xecc2a)[0x7f4d3e637c2a] /usr/lib64/libpython2.7.so.1.0(PyCFunction_Call+0xf1)[0x7f4f1d0ddbd9] /usr/lib64/libpython2.7.so.1.0(PyObject_Call+0x5c)[0x7f4f1d0a8123]
comment:28 Changed 4 years ago by
 Status changed from positive_review to needs_work
Confirmed. I'm sorry. With Pynac changes I have a testing procedure in place here, but with Python I was relying on patchbot. This issue is also somewhat unusual as to the surprising problems presented.
comment:29 Changed 4 years ago by
Same here...
comment:30 Changed 4 years ago by
Positive stuff for me, it made me realise that some bits of cysignals
are not currently working properly on Gentoo  even so the tests pass :(
comment:31 Changed 4 years ago by
 Dependencies changed from #21614 to #21614, pynac0.7.1
 Report Upstream changed from N/A to Fixed upstream, in a later stable release.
comment:32 Changed 4 years ago by
 Dependencies changed from #21614, pynac0.7.1 to #21614, #21963
 Status changed from needs_work to needs_review
comment:33 Changed 4 years ago by
 Commit changed from 04f79cdf8ed0c887b7f7d1797ce5023f782d0a50 to bb4630fa7a2ac6b9b0a1d98486378877fe60f97c
Branch pushed to git repo; I updated commit sha1. New commits:
bb4630f  Merge branch 'develop' into t/20939/remove_pexpect_maxima_in_ymn20939

comment:34 Changed 4 years ago by
 Milestone changed from sage7.4 to sage7.5
 Status changed from needs_review to positive_review
Let's try again.
comment:35 Changed 4 years ago by
 Branch changed from u/rws/remove_pexpect_maxima_in_ymn20939 to bb4630fa7a2ac6b9b0a1d98486378877fe60f97c
 Resolution set to fixed
 Status changed from positive_review to closed
This depends on the associated Legendre polynomials being implemented, e,g, #16813.