Opened 2 years ago

Closed 2 years ago

#23325 closed defect (fixed)

Upgrade to Pynac-0.7.10

Reported by: rws Owned by:
Priority: major Milestone: sage-8.1
Component: packages: standard Keywords:
Cc: fbissey Merged in:
Authors: Ralf Stephan Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 5387ba5 (Commits) Commit: 5387ba543d78320b1a87c72b45068e4c4ae4c7ef
Dependencies: #23329 Stopgaps:

Description (last modified by rws)

In the new version:

  • remove static PyObjects; (#23134)
  • fix segmentation fault with coefficients() (#23545)
  • fix compile error --with-giac=yes
  • fix tgamma/lgamma return types
  • (realm)(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_coefficients
  • internal LONG numeric; nice speedup in some parts
  • fast internal in-place arithmetic operators
  • obsolete some internal usages of Python

https://github.com/pynac/pynac/releases/download/pynac-0.7.10/pynac-0.7.10.tar.bz2

Change History (45)

comment:1 Changed 2 years ago by rws

  • Description modified (diff)

comment:2 Changed 2 years ago by rws

  • Branch set to u/rws/upgrade_to_pynac_0_7_9

comment:3 Changed 2 years ago by rws

  • Authors set to Ralf Stephan
  • Commit set to f976b8c07bc783cc088f6c964aa2113cef6d19ee
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

1d1aeb823325: pkg version/chksum, remove obsolete patch
cf6218023325: fix py_get_parent_char()
f976b8c23325: fix doctests

comment:4 Changed 2 years ago by jdemeyer

  • 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 2 years ago by jdemeyer

  • 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 2 years ago by rws

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/site-packages/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/site-packages/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 3-dimensional 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 3-dimensional 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 3-dimensional differentiable manifold R3
sage: parent(eps[[1,2,3]])
Algebra of differentiable scalar fields on the 3-dimensional 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 2 years ago by jdemeyer

I will have a look.

comment:8 Changed 2 years ago by rws

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 2 years ago by jdemeyer

I think the best solution here is simply to implement characteric for the "Ring of coordinate functions".

comment:10 Changed 2 years ago by jdemeyer

  • Dependencies set to #23329

comment:11 Changed 2 years ago by git

  • Commit changed from f976b8c07bc783cc088f6c964aa2113cef6d19ee to fb52b8b87a7f866a729b599807be27a04907a0e6

Branch pushed to git repo; I updated commit sha1. New commits:

fb52b8b23325: revert libs/pynac commit

comment:12 Changed 2 years ago by git

  • Commit changed from fb52b8b87a7f866a729b599807be27a04907a0e6 to f5682bf3c03d657382282c307d0e9392cedf2008

Branch pushed to git repo; I updated commit sha1. New commits:

1a369cbMerge branch 'develop' into t/23325/upgrade_to_pynac_0_7_9
1fa0f69Merge branch 'develop' into t/23325/upgrade_to_pynac_0_7_9
725db22Merge branch 'develop' into t/23325/upgrade_to_pynac_0_7_9
3b7db8bpynac-2.7.10 doctest fixes
f5682bfnecessary code change to make pynac-2.7.10 work

comment:13 Changed 2 years ago by rws

Correct is 0.7.10 not 2.7.10.

comment:14 Changed 2 years ago by git

  • Commit changed from f5682bf3c03d657382282c307d0e9392cedf2008 to 0879761e743d948c0fb461c88168d8bb2ac6a7cd

Branch pushed to git repo; I updated commit sha1. New commits:

bafd41aMerge branch 'develop' into t/23325/upgrade_to_pynac_0_7_9
a90ce6a23582: Robustify doctest in hyperbolic_geodesic.py
0879761Merge 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 2 years ago by rws

  • Dependencies changed from #23329 to #23329, #23582

comment:16 Changed 2 years ago by fbissey

  • Cc fbissey added

comment:17 Changed 2 years ago by rws

  • Dependencies changed from #23329, #23582 to #23329, #23582, #23587

comment:18 Changed 2 years ago by git

  • Commit changed from 0879761e743d948c0fb461c88168d8bb2ac6a7cd to b0dd29b7c9f9aeb84f523c01f664ca3def1d5ccf

Branch pushed to git repo; I updated commit sha1. New commits:

28ae95823325: doctest fixes
5fc85e523587: Silence Cython warnings with expression.pyx
b0dd29bMerge branch 'u/rws/silence_cython_warnings_with_expression_pyx' of git://trac.sagemath.org/sage into t/23325/upgrade_to_pynac_0_7_9

comment:19 Changed 2 years ago by chapoton

May we please change the .configure line in spkg-install to

    ./configure --disable-static --prefix=${SAGE_LOCAL} --with-giac=no --libdir=
"$SAGE_LOCAL/lib" PYTHON=sage-python23

to ensure compatibility with python3 ?

comment:20 Changed 2 years ago by git

  • Commit changed from b0dd29b7c9f9aeb84f523c01f664ca3def1d5ccf to ad2e96d39f094575f5a92818987fc4355856b93b

Branch pushed to git repo; I updated commit sha1. New commits:

ad2e96d23325: explicitly look for sage-python23

comment:21 Changed 2 years ago by rws

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 pynac-0.7.10 is more urgent.

comment:22 Changed 2 years ago by rws

  • Dependencies changed from #23329, #23582, #23587 to #23329, #23582
  • Description modified (diff)
  • Summary changed from Upgrade to Pynac-0.7.9 to Upgrade to Pynac-0.7.10

comment:23 Changed 2 years ago by rws

  • Description modified (diff)

comment:24 Changed 2 years ago by rws

  • Branch changed from u/rws/upgrade_to_pynac_0_7_9 to u/rws/23325

comment:25 Changed 2 years ago by rws

  • 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:

86d0ef3trac 23329 characteristic of coordinate chart function ring
2e40ec5Merge branch 'u/rws/robustify_doctest_in_hyperbolic_geodesic_py' of git://trac.sagemath.org/sage into tmp11
0f51d4523325: chksum,version
3cc070823325: remove obsolete patch
95cb0a523325: explicitly look for sage-python23
7793f1c23325: code change to make use of new pynac
9520ada23325: doctest fixes

comment:26 Changed 2 years ago by jdemeyer

For ease of reviewing, I would prefer to wait until the dependencies have been merged.

comment:27 Changed 2 years ago by git

  • Commit changed from 9520ada5b12d602e1e2135f9d7bdfed1c4862041 to f52f60265da343edd985a58b6c55929e209883de

Branch pushed to git repo; I updated commit sha1. New commits:

f52f60223325: revert merge of 23582

comment:28 Changed 2 years ago by rws

  • Dependencies changed from #23329, #23582 to #23329

comment:29 Changed 2 years ago by jdemeyer

  • 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 2 years ago by rws

  • Branch changed from u/rws/23325 to u/rws/23325-1

comment:31 follow-up: Changed 2 years ago by rws

  • Commit changed from f52f60265da343edd985a58b6c55929e209883de to 62cebba82d2da14fa1aa2c3e6324c2f5055365de

Nice opportunity for the rarely used git rebase -i.


New commits:

a887a8b23325: chksum,version
98bab3723325: remove obsolete patch
fe100f723325: explicitly look for sage-python23
8fe77a723325: code change to make use of new pynac
62cebba23325: doctest fixes

comment:32 Changed 2 years ago by rws

  • Status changed from needs_work to needs_review

comment:33 in reply to: ↑ 31 Changed 2 years ago by jdemeyer

Replying to rws:

Nice opportunity for the rarely used git rebase -i.

I use that all the time...

comment:34 Changed 2 years ago by jdemeyer

  • Branch changed from u/rws/23325-1 to u/jdemeyer/23325-1

comment:35 Changed 2 years ago by jdemeyer

  • Commit changed from 62cebba82d2da14fa1aa2c3e6324c2f5055365de to b54f51bab33a602448c1b3e2f719fb375661aac3
  • Status changed from needs_review to positive_review

New commits:

b54f51bDoc formatting

comment:36 Changed 2 years ago by vbraun

  • Branch changed from u/jdemeyer/23325-1 to b54f51bab33a602448c1b3e2f719fb375661aac3
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:37 Changed 2 years ago by vbraun

  • Commit b54f51bab33a602448c1b3e2f719fb375661aac3 deleted
  • Resolution fixed deleted
  • Status changed from closed to new

Fails on 32-bit 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 2 years ago by rws

Okay, I'm installing virtualization (a first) and Ubuntu 32-bit to get the ability to test on 32-bit.

comment:39 Changed 2 years ago by rws

  • Branch changed from b54f51bab33a602448c1b3e2f719fb375661aac3 to u/rws/b54f51bab33a602448c1b3e2f719fb375661aac3

comment:40 Changed 2 years ago by rws

  • Branch changed from u/rws/b54f51bab33a602448c1b3e2f719fb375661aac3 to b54f51bab33a602448c1b3e2f719fb375661aac3

comment:41 Changed 2 years ago by rws

STDERR: error: Server does not allow request for unadvertised object b54f51bab33a602448c1b3e2f719fb375661aac3

So I'll push to my previous branch u/rws/23325-1 (including Jeroen's fix) in the hope you can pick up its last commit, which contains a last-minute fix.

comment:42 Changed 2 years ago by rws

  • Branch changed from b54f51bab33a602448c1b3e2f719fb375661aac3 to u/rws/23325-1

comment:43 Changed 2 years ago by rws

  • Commit set to 5387ba543d78320b1a87c72b45068e4c4ae4c7ef
  • Status changed from new to needs_review

New commits:

a887a8b23325: chksum,version
98bab3723325: remove obsolete patch
fe100f723325: explicitly look for sage-python23
8fe77a723325: code change to make use of new pynac
62cebba23325: doctest fixes
5a2da5823325: doc formatting
5387ba523325: last-minute 32-bit fix

comment:44 Changed 2 years ago by fbissey

  • 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 2 years ago by vbraun

  • Branch changed from u/rws/23325-1 to 5387ba543d78320b1a87c72b45068e4c4ae4c7ef
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.