Opened 4 years ago
Last modified 4 years ago
#23325 closed defect
Upgrade to Pynac0.7.10 — at Version 23
Reported by:  rws  Owned by:  

Priority:  major  Milestone:  sage8.1 
Component:  packages: standard  Keywords:  
Cc:  fbissey  Merged in:  
Authors:  Ralf Stephan  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  u/rws/upgrade_to_pynac_0_7_9 (Commits, GitHub, GitLab)  Commit:  ad2e96d39f094575f5a92818987fc4355856b93b 
Dependencies:  #23329, #23582  Stopgaps: 
Description (last modified by )
In the new version:
 remove static
PyObject
s; (#23134)  fix segmentation fault with coefficients() (#23545)
 fix compile error withgiac=yes
 fix tgamma/lgamma return types
 (real^{m})^{(1/n)} > abs(real)^{(m/n)}, m even, m/n integer (#14305)
 fix
add::conjugate()
(#23135)  fix
is_real(zeta(real))
 negative symbolic domain from assume(sym<0) (#21973)
 add missing calls to
set_domain
in symbol constructors (#23138)  implement
add.info(negative)
 exact comparison of
overall_coefficient
s  internal LONG numeric; general speedup
 fast internal inplace arithmetic operators
 obsolete some internal usages of Python
https://github.com/pynac/pynac/releases/download/pynac0.7.10/pynac0.7.10.tar.bz2
Change History (23)
comment:1 Changed 4 years ago by
 Description modified (diff)
comment:2 Changed 4 years ago by
 Branch set to u/rws/upgrade_to_pynac_0_7_9
comment:3 Changed 4 years ago by
 Commit set to f976b8c07bc783cc088f6c964aa2113cef6d19ee
 Description modified (diff)
 Status changed from new to needs_review
comment:4 Changed 4 years ago by
 Status changed from needs_review to needs_work
I don't agree with the change to py_get_parent_char
: why assume characteristic zero if the parent cannot decide?
comment:5 Changed 4 years ago by
 Reviewers set to Jeroen Demeyer
If you revert the change to py_get_parent_char
and all doctests pass, you can set this to positive review.
comment:6 Changed 4 years ago by
That's a no, because this then needs investigation (doctest fail at src/sage/manifolds/differentiable/diff_form.py:796):
sage: M = Manifold(3, 'R3', '\RR^3', start_index=1) sage: c_cart.<x,y,z> = M.chart() sage: eps = M.diff_form(3, 'epsilon', r'\epsilon') sage: eps[1,2,3] = 1 sage: a = M.one_form('A') sage: a[:] = (x*y*z, z*x, y*z) sage: b = M.one_form('B') sage: b[:] = (cos(z), sin(x), cos(y)) sage: ab = a.wedge(b) sage: dab = ab.exterior_derivative() sage: dab[[1,2,3]]/eps[[1,2,3]] ... /home/ralf/sage/src/sage/structure/element.pyx in sage.structure.element.RingElement._div_ (build/cythonized/sage/structure/element.c:17655)() 2453 return l 2454 > 2455 cpdef _div_(self, other): 2456 """ 2457 Default implementation of division using the fraction field. /home/ralf/sage/local/lib/python2.7/sitepackages/sage/manifolds/coord_func_symb.pyc in _div_(self, other) 947 if other._express.is_zero(): 948 raise ZeroDivisionError("division of a coordinate function by zero") > 949 res = self._simplify(self._express / SR(other)) 950 if res.is_trivial_zero(): # NB: "if res == 0" would be too 951 # expensive (cf. #22859) /home/ralf/sage/src/sage/structure/element.pyx in sage.structure.element.Element.__div__ (build/cythonized/sage/structure/element.c:12950)() 1638 cdef int cl = classify_elements(left, right) 1639 if HAVE_SAME_PARENT(cl): > 1640 return (<Element>left)._div_(right) 1641 if BOTH_ARE_ELEMENT(cl): 1642 return coercion_model.bin_op(left, right, div) /home/ralf/sage/src/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression._div_ (build/cythonized/sage/symbolic/expression.cpp:24795)() 3445 raise ZeroDivisionError("Symbolic division by zero") 3446 else: > 3447 raise 3448 finally: 3449 sig_off() /home/ralf/sage/src/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression._div_ (build/cythonized/sage/symbolic/expression.cpp:24684)() 3437 o) 3438 else: > 3439 x = left._gobj / _right._gobj 3440 return new_Expression_from_GEx(left._parent, x) 3441 except Exception as msg: /home/ralf/sage/src/sage/libs/pynac/pynac.pyx in sage.libs.pynac.pynac.py_get_parent_char (build/cythonized/sage/libs/pynac/pynac.cpp:9942)() 818 return 0 819 > 820 c = (<Element>o)._parent.characteristic() 821 822 # Pynac only differentiates between /home/ralf/sage/local/lib/python2.7/sitepackages/sage/categories/rings.pyc in characteristic(self) 461 from sage.rings.infinity import infinity 462 from sage.rings.integer_ring import ZZ > 463 order_1 = self.one().additive_order() 464 return ZZ.zero() if order_1 is infinity else order_1 465 /home/ralf/sage/src/sage/structure/element.pyx in sage.structure.element.RingElement.additive_order (build/cythonized/sage/structure/element.c:18049)() 2470 Return the additive order of ``self``. 2471 """ > 2472 raise NotImplementedError 2473 2474 def multiplicative_order(self): NotImplementedError: sage: dab[[1,2,3]] Scalar field on the 3dimensional differentiable manifold R3 sage: type(_) <class 'sage.manifolds.differentiable.scalarfield_algebra.DiffScalarFieldAlgebra_with_category.element_class'> sage: eps[[1,2,3]] Scalar field on the 3dimensional differentiable manifold R3 sage: type(_) <class 'sage.manifolds.differentiable.scalarfield_algebra.DiffScalarFieldAlgebra_with_category.element_class'> sage: parent(dab[[1,2,3]]) Algebra of differentiable scalar fields on the 3dimensional differentiable manifold R3 sage: parent(eps[[1,2,3]]) Algebra of differentiable scalar fields on the 3dimensional differentiable manifold R3
While this is triggered by changes in Pynac that should be addressed it's not clear to me why the division of two scalar fields must go through SR
at all.
comment:7 Changed 4 years ago by
I will have a look.
comment:8 Changed 4 years ago by
The reason why this is now failing is that the quotient here, which is 1
but not integer, is now recognized as is_one
and triggers the ...pos_char
call.
https://github.com/pynac/pynac/blob/master/ginac/mul.cpp#L702
Removing the call altogether in preliminary testing only fails one doctest involving Mod
and, frankly, I could care less. Full testing now.
comment:9 Changed 4 years ago by
I think the best solution here is simply to implement characteric
for the "Ring of coordinate functions".
comment:10 Changed 4 years ago by
 Dependencies set to #23329
comment:11 Changed 4 years ago by
 Commit changed from f976b8c07bc783cc088f6c964aa2113cef6d19ee to fb52b8b87a7f866a729b599807be27a04907a0e6
Branch pushed to git repo; I updated commit sha1. New commits:
fb52b8b  23325: revert libs/pynac commit

comment:12 Changed 4 years ago by
 Commit changed from fb52b8b87a7f866a729b599807be27a04907a0e6 to f5682bf3c03d657382282c307d0e9392cedf2008
Branch pushed to git repo; I updated commit sha1. New commits:
1a369cb  Merge branch 'develop' into t/23325/upgrade_to_pynac_0_7_9

1fa0f69  Merge branch 'develop' into t/23325/upgrade_to_pynac_0_7_9

725db22  Merge branch 'develop' into t/23325/upgrade_to_pynac_0_7_9

3b7db8b  pynac2.7.10 doctest fixes

f5682bf  necessary code change to make pynac2.7.10 work

comment:13 Changed 4 years ago by
Correct is 0.7.10 not 2.7.10.
comment:14 Changed 4 years ago by
 Commit changed from f5682bf3c03d657382282c307d0e9392cedf2008 to 0879761e743d948c0fb461c88168d8bb2ac6a7cd
Branch pushed to git repo; I updated commit sha1. New commits:
bafd41a  Merge branch 'develop' into t/23325/upgrade_to_pynac_0_7_9

a90ce6a  23582: Robustify doctest in hyperbolic_geodesic.py

0879761  Merge branch 'u/rws/robustify_doctest_in_hyperbolic_geodesic_py' of git://trac.sagemath.org/sage into t/23325/upgrade_to_pynac_0_7_9

comment:15 Changed 4 years ago by
 Dependencies changed from #23329 to #23329, #23582
comment:16 Changed 4 years ago by
 Cc fbissey added
comment:17 Changed 4 years ago by
 Dependencies changed from #23329, #23582 to #23329, #23582, #23587
comment:18 Changed 4 years ago by
 Commit changed from 0879761e743d948c0fb461c88168d8bb2ac6a7cd to b0dd29b7c9f9aeb84f523c01f664ca3def1d5ccf
comment:19 Changed 4 years ago by
May we please change the .configure line in spkginstall to
./configure disablestatic prefix=${SAGE_LOCAL} withgiac=no libdir= "$SAGE_LOCAL/lib" PYTHON=sagepython23
to ensure compatibility with python3 ?
comment:20 Changed 4 years ago by
 Commit changed from b0dd29b7c9f9aeb84f523c01f664ca3def1d5ccf to ad2e96d39f094575f5a92818987fc4355856b93b
Branch pushed to git repo; I updated commit sha1. New commits:
ad2e96d  23325: explicitly look for sagepython23

comment:21 Changed 4 years ago by
If you looked at the doctest changes you probably have noticed natural simplifications where quadratic numeric factors inside sqrt
are drawn out of the root. I will have to revert this behaviour, unfortunately. There is code in the Sage code base that relies on that root content is not changed, because it allows manipulations leading to expressions that can later be better simplified by Maxima's simplification functions. Apparently none of these functions is able to normalize root content in complicated expressions. I will maybe implement it separately, similar to gamma_normalize
, for inclusion in simplify_full
but releasing pynac0.7.10 is more urgent.
comment:22 Changed 4 years ago by
 Dependencies changed from #23329, #23582, #23587 to #23329, #23582
 Description modified (diff)
 Summary changed from Upgrade to Pynac0.7.9 to Upgrade to Pynac0.7.10
comment:23 Changed 4 years ago by
 Description modified (diff)
New commits:
23325: pkg version/chksum, remove obsolete patch
23325: fix py_get_parent_char()
23325: fix doctests