Opened 5 years ago
Closed 5 years ago
#23325 closed defect (fixed)
Upgrade to Pynac0.7.10
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:  5387ba5 (Commits, GitHub, GitLab)  Commit:  5387ba543d78320b1a87c72b45068e4c4ae4c7ef 
Dependencies:  #23329  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; nice speedup in some parts
 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 (45)
comment:1 Changed 5 years ago by
 Description modified (diff)
comment:2 Changed 5 years ago by
 Branch set to u/rws/upgrade_to_pynac_0_7_9
comment:3 Changed 5 years ago by
 Commit set to f976b8c07bc783cc088f6c964aa2113cef6d19ee
 Description modified (diff)
 Status changed from new to needs_review
comment:4 Changed 5 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 5 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 5 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 5 years ago by
I will have a look.
comment:8 Changed 5 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 5 years ago by
I think the best solution here is simply to implement characteric
for the "Ring of coordinate functions".
comment:10 Changed 5 years ago by
 Dependencies set to #23329
comment:11 Changed 5 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 5 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 5 years ago by
Correct is 0.7.10 not 2.7.10.
comment:14 Changed 5 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 5 years ago by
 Dependencies changed from #23329 to #23329, #23582
comment:16 Changed 5 years ago by
 Cc fbissey added
comment:17 Changed 5 years ago by
 Dependencies changed from #23329, #23582 to #23329, #23582, #23587
comment:18 Changed 5 years ago by
 Commit changed from 0879761e743d948c0fb461c88168d8bb2ac6a7cd to b0dd29b7c9f9aeb84f523c01f664ca3def1d5ccf
comment:19 Changed 5 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 5 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 5 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 5 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 5 years ago by
 Description modified (diff)
comment:24 Changed 5 years ago by
 Branch changed from u/rws/upgrade_to_pynac_0_7_9 to u/rws/23325
comment:25 Changed 5 years ago by
 Commit changed from ad2e96d39f094575f5a92818987fc4355856b93b to 9520ada5b12d602e1e2135f9d7bdfed1c4862041
 Description modified (diff)
 Status changed from needs_work to needs_review
I made a new branch. Please review.
New commits:
86d0ef3  trac 23329 characteristic of coordinate chart function ring

2e40ec5  Merge branch 'u/rws/robustify_doctest_in_hyperbolic_geodesic_py' of git://trac.sagemath.org/sage into tmp11

0f51d45  23325: chksum,version

3cc0708  23325: remove obsolete patch

95cb0a5  23325: explicitly look for sagepython23

7793f1c  23325: code change to make use of new pynac

9520ada  23325: doctest fixes

comment:26 Changed 5 years ago by
For ease of reviewing, I would prefer to wait until the dependencies have been merged.
comment:27 Changed 5 years ago by
 Commit changed from 9520ada5b12d602e1e2135f9d7bdfed1c4862041 to f52f60265da343edd985a58b6c55929e209883de
Branch pushed to git repo; I updated commit sha1. New commits:
f52f602  23325: revert merge of 23582

comment:28 Changed 5 years ago by
 Dependencies changed from #23329, #23582 to #23329
comment:29 Changed 5 years ago by
 Status changed from needs_review to needs_work
I don't think this is a valid way to revert a merge. Either you really revert the merge (by rewriting git history as if the merge never happened), or you keep the dependency on #23582.
comment:30 Changed 5 years ago by
 Branch changed from u/rws/23325 to u/rws/233251
comment:31 followup: ↓ 33 Changed 5 years ago by
 Commit changed from f52f60265da343edd985a58b6c55929e209883de to 62cebba82d2da14fa1aa2c3e6324c2f5055365de
comment:32 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:33 in reply to: ↑ 31 Changed 5 years ago by
comment:34 Changed 5 years ago by
 Branch changed from u/rws/233251 to u/jdemeyer/233251
comment:35 Changed 5 years ago by
 Commit changed from 62cebba82d2da14fa1aa2c3e6324c2f5055365de to b54f51bab33a602448c1b3e2f719fb375661aac3
 Status changed from needs_review to positive_review
New commits:
b54f51b  Doc formatting

comment:36 Changed 5 years ago by
 Branch changed from u/jdemeyer/233251 to b54f51bab33a602448c1b3e2f719fb375661aac3
 Resolution set to fixed
 Status changed from positive_review to closed
comment:37 Changed 5 years ago by
 Commit b54f51bab33a602448c1b3e2f719fb375661aac3 deleted
 Resolution fixed deleted
 Status changed from closed to new
Fails on 32bit Linux
********************************************************************** File "src/doc/de/thematische_anleitungen/sage_gymnasium.rst", line 191, in doc.de.thematische_anleitungen.sage_gymnasium Failed example: sqrt(18) Expected: 3*sqrt(2) Got: sqrt(18) ********************************************************************** 1 item had failures: 1 of 294 in doc.de.thematische_anleitungen.sage_gymnasium [207 tests, 1 failure, 5.39 s] ********************************************************************** File "src/sage/functions/orthogonal_polys.py", line 1668, in sage.functions.orthogonal_polys.Func_assoc_legendre_Q._eval_ Failed example: gen_legendre_Q(2,1,3) Expected: 1/4*sqrt(2)*(36*I*pi + 36*log(4)  36*log(2)  25) Got: 1/8*sqrt(8)*(36*I*pi + 36*log(4)  36*log(2)  25) ********************************************************************** 1 item had failures: 1 of 2 in sage.functions.orthogonal_polys.Func_assoc_legendre_Q._eval_ [378 tests, 1 failure, 7.79 s] ********************************************************************** File "src/sage/rings/number_field/number_field_base.pyx", line 219, in sage.rings.number_field.number_field_base.NumberField.minkowski_bound Failed example: B = K.minkowski_bound(); B Expected: 16/3*sqrt(3)/pi Got: 8/9*sqrt(108)/pi ********************************************************************** File "src/sage/rings/number_field/number_field_base.pyx", line 230, in sage.rings.number_field.number_field_base.NumberField.minkowski_bound Failed example: B = K.minkowski_bound(); B Expected: sqrt(10) Got: 1/2*sqrt(40) ********************************************************************** 1 item had failures: 2 of 19 in sage.rings.number_field.number_field_base.NumberField.minkowski_bound [74 tests, 2 failures, 2.09 s] ********************************************************************** File "src/sage/rings/real_mpfr.pyx", line 3961, in sage.rings.real_mpfr.RealNumber.sqrt Failed example: r.sqrt() Expected: 2*sqrt(1086) Got: sqrt(4344) ********************************************************************** 1 item had failures: 1 of 13 in sage.rings.real_mpfr.RealNumber.sqrt [1000 tests, 1 failure, 3.14 s]
comment:38 Changed 5 years ago by
Okay, I'm installing virtualization (a first) and Ubuntu 32bit to get the ability to test on 32bit.
comment:39 Changed 5 years ago by
 Branch changed from b54f51bab33a602448c1b3e2f719fb375661aac3 to u/rws/b54f51bab33a602448c1b3e2f719fb375661aac3
comment:40 Changed 5 years ago by
 Branch changed from u/rws/b54f51bab33a602448c1b3e2f719fb375661aac3 to b54f51bab33a602448c1b3e2f719fb375661aac3
comment:41 Changed 5 years ago by
STDERR: error: Server does not allow request for unadvertised object b54f51bab33a602448c1b3e2f719fb375661aac3
So I'll push to my previous branch u/rws/233251
(including Jeroen's fix) in the hope you can pick up its last commit, which contains a lastminute fix.
comment:42 Changed 5 years ago by
 Branch changed from b54f51bab33a602448c1b3e2f719fb375661aac3 to u/rws/233251
comment:43 Changed 5 years ago by
 Commit set to 5387ba543d78320b1a87c72b45068e4c4ae4c7ef
 Status changed from new to needs_review
comment:44 Changed 5 years ago by
 Status changed from needs_review to positive_review
I think it can just be put back to positive review. Especially if the last fix is the stuff for 32bits problems.
comment:45 Changed 5 years ago by
 Branch changed from u/rws/233251 to 5387ba543d78320b1a87c72b45068e4c4ae4c7ef
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
23325: pkg version/chksum, remove obsolete patch
23325: fix py_get_parent_char()
23325: fix doctests