#24838 closed defect (fixed)
Upgrade to pynac0.7.22
Reported by:  Ralf Stephan  Owned by:  

Priority:  major  Milestone:  sage8.3 
Component:  symbolics  Keywords:  
Cc:  François Bissey, Timo Kaufmann  Merged in:  
Authors:  Ralf Stephan  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  e324b64 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  #24927  Stopgaps: 
Description (last modified by )
In the new version:
 fast and working versons of quo_rem and rational decomposition (#25644, #25645)
 fix power inconsistency
 internal fix of another power inconsistency (#25639)
(x^a)^b > x^(ab)
IF x real, a odd integer, ab integer add
ex::treesize()
as complexity metric (#25643)  information for packagers
 implement commutative matching; fix GiNaC's matching deficiencies (#25168)
 substitute patternless if no wildcards present
 remove some compiler warnings
 update AX_CXX_COMPILE_STDCXX.m4
 fix
exp(c*f(x))
simplification  fix
cases(...).subs(...)
function::info
formin/max_symbolic
(#21945) performance and readability improvements
 correct
sinh/cosh/tanh
return type (#24299) atan2
fixes potential memleaks fixed
 fixes in Giac interface
subs
can now substitute numeric terms (#23964)
 fix memleak in inplace PyObject numerics (#24745)
 fix endless computation (#24883)
 Python interface / Py3 fixes and improvements (thx Erik Bray, #24561, #24752, #24522)
 fix internal
asin
/acos
of complex balls  draw factors out of
add^rational
(#24768, #25251, #25252) exp(f(x))
,exp(c*f(x))
, f inv. hyperbolic simplifications (#24841) always extend trig/hyperbolic functions to complex domain (#24428)
 extensive code readability improvements
https://github.com/pynac/pynac/releases/download/pynac0.7.22/pynac0.7.22.tar.bz2
Change History (75)
comment:1 Changed 5 years ago by
Branch:  → u/rws/upgrade_to_pynac_0_7_17 

comment:2 Changed 5 years ago by
Authors:  → Ralf Stephan 

Commit:  → d3511ce1cbd35b9d625b0f4215ee014e6cc99931 
Description:  modified (diff) 
Status:  new → needs_review 
comment:3 Changed 5 years ago by
Cc:  François Bissey added 

comment:4 Changed 5 years ago by
Description:  modified (diff) 

comment:6 followup: 7 Changed 5 years ago by
Description:  modified (diff) 

Yes, that PR is merged. I'll probably add a hotfix for #24883 here as patch, too.
comment:7 Changed 5 years ago by
comment:9 Changed 5 years ago by
Status:  needs_review → needs_work 

This used to return a purely imaginary number, now there is a small real part:
sage: arccosh(x).subs(x=0.9) 9.66146955461936e22 + 0.451026811796262*I
comment:10 Changed 5 years ago by
This is an artifact of conversion to CC:
sage: CC(ComplexBallField(68)(0.9).arccosh()) 9.66146955461936e22 + 0.451026811796262*I
We use arb with precision+15 and convert back to parent, all to make up for missing or limited (in the calculus sense, not the algebraic, note recent discussion) member functions of parent (RR in this case). Before this pynac version we had specific logic to zero the real part but this was errorprone. IMHO CC conversion of complex balls should be fixed.
comment:12 followup: 15 Changed 5 years ago by
You may also want to know why I'm using arb and not mpfr:
sage: r=srange(2.5,2.5,.01) sage: rr = [CC(i) for i in r] sage: %timeit for x in rr: _=x.arccosh() 100 loops, best of 3: 11.9 ms per loop sage: rr = [CBF(i) for i in r] sage: %timeit for x in rr: _=x.arccosh() 1000 loops, best of 3: 649 µs per loop
comment:14 followup: 17 Changed 5 years ago by
comment:15 Changed 5 years ago by
Replying to rws:
You may also want to know why I'm using arb and not mpfr:
Despite what you think, you are not using mpfr, but PARI:
def arccosh(self): """ Return the hyperbolic arccosine of ``self``. EXAMPLES:: sage: (1+CC(I)).arccosh() 1.06127506190504 + 0.904556894302381*I """ return self._parent(self.__pari__().acosh())
If you want to compare with something, it should be mpc (in Sage: CC = MPComplexField(53)
)
comment:16 Changed 5 years ago by
Interestingly,
sage: arccosh(x).subs(x=MPComplexField(53)(0.9)) ... TypeError: no canonical coercion from Complex Field with 53 bits of precision to Symbolic Ring
comment:17 Changed 5 years ago by
comment:18 Changed 5 years ago by
Status:  needs_work → needs_review 

comment:19 Changed 5 years ago by
Commit:  d3511ce1cbd35b9d625b0f4215ee014e6cc99931 → 9f86eb7b725b44939f22df4cb7f5a6b05935a6c8 

comment:21 Changed 5 years ago by
Description:  modified (diff) 

Summary:  Upgrade to pynac0.7.17 → Upgrade to pynac0.7.18 
comment:22 Changed 5 years ago by
Commit:  9f86eb7b725b44939f22df4cb7f5a6b05935a6c8 → ae5eac77131fca0266d63ee7c9b9f2b8ecf72ee8 

Branch pushed to git repo; I updated commit sha1. New commits:
ae5eac7  24838: version/chksum

comment:23 Changed 5 years ago by
Status:  needs_work → needs_review 

comment:24 Changed 5 years ago by
Description:  modified (diff) 

comment:25 Changed 5 years ago by
Status:  needs_review → needs_work 

0.7.17 introduced a bug in exp(ex*log(x))
simplification, and we need a new release, anyway.
comment:26 Changed 5 years ago by
Description:  modified (diff) 

Summary:  Upgrade to pynac0.7.18 → Upgrade to pynac0.7.19 
comment:27 Changed 5 years ago by
Commit:  ae5eac77131fca0266d63ee7c9b9f2b8ecf72ee8 → 88782837071d956743e49fb64ef972c1ee163db1 

comment:28 Changed 5 years ago by
Status:  needs_work → needs_review 

comment:29 Changed 5 years ago by
This looks fishy:
 sage: SR(CDF(1/2)).arccosh() # rel tol 1e15 + sage: SR(CDF(1/2)).arccosh() # abs tol 1e15
comment:30 followup: 32 Changed 5 years ago by
Status:  needs_review → needs_work 

comment:31 Changed 5 years ago by
Description:  modified (diff) 

comment:32 Changed 5 years ago by
Replying to jdemeyer:
The analogous thing as 9 should be fixed for pure imaginary results:
sage: CDF(1/2).arccosh() 1.0471975511965979*I sage: SR(CDF(1/2)).arccosh() 6.372599439108525e21 + 1.0471975511965979*I
As explained in 10 missing FP functionality is gained using arb. But there was that bug in arb which is now fixed. Consequentially we need that fix or a recent arb to get the right result, see 14.
Then you also revert 29
Will rerevert.
comment:33 Changed 5 years ago by
Dependencies:  → #24927 

comment:34 Changed 5 years ago by
I confirm that this works with https://github.com/fredrikjohansson/arb/commit/7afd3bfaf1697739751d96d3665ef83e78c76820 applied, which is in arb2.13.0 (#24927).
comment:35 Changed 5 years ago by
Commit:  88782837071d956743e49fb64ef972c1ee163db1 → 3efba1a6f9dd67401716dfdc56eb9bafa61e8963 

comment:36 Changed 5 years ago by
Some recent Sage commit makes doctests (involving powers of I
) fail together with pynac0.7.17 so we'll need a fix in 0.7.20.
comment:38 Changed 5 years ago by
Description:  modified (diff) 

Summary:  Upgrade to pynac0.7.19 → Upgrade to pynac0.7.20 
comment:39 Changed 5 years ago by
Commit:  3efba1a6f9dd67401716dfdc56eb9bafa61e8963 → ad07cd5c0b8789b768d23b93db4a4004467ed5df 

Branch pushed to git repo; I updated commit sha1. New commits:
ed36070  Merge branch 'develop' into t/24838/upgrade_to_pynac_0_7_17

3278103  Merge branch 'develop' into t/24838/upgrade_to_pynac_0_7_17

ca69aef  Merge branch 'develop' into t/24838/upgrade_to_pynac_0_7_17

97bf3b7  24838: doctest fixes

e6b01df  doctest fix

fdb4210  Merge branch 'develop' into t/24838/upgrade_to_pynac_0_7_17

ad07cd5  24838: 0.7.20 version/chksum

comment:40 Changed 4 years ago by
Branch:  u/rws/upgrade_to_pynac_0_7_17 

Commit:  ad07cd5c0b8789b768d23b93db4a4004467ed5df 
Description:  modified (diff) 
Milestone:  sage8.2 → sage8.3 
Summary:  Upgrade to pynac0.7.20 → Upgrade to pynac0.7.22 
comment:41 Changed 4 years ago by
Description:  modified (diff) 

comment:42 Changed 4 years ago by
Branch:  → u/rws/24838 

comment:43 Changed 4 years ago by
Commit:  → 3b244e94f2b9bced8249d5abefb206bd419f8f43 

Status:  needs_work → needs_review 
comment:44 Changed 4 years ago by
Description:  modified (diff) 

comment:46 Changed 4 years ago by
Please check again, I just now did not get any error with the file linked in the ticket. Did you reload the branch?
comment:47 Changed 4 years ago by
Indeed. Works now. I must have made some mistake in my previous attempt.
comment:48 Changed 4 years ago by
got
sage t long src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py # 2 doctests failed sage t long src/sage/symbolic/expression.pyx # 1 doctest failed
comment:49 followup: 50 Changed 4 years ago by
Doctesting 2 files. sage t long src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py ********************************************************************** File "src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py", line 153, in sage.rings.asymptotic.asymptotics_multivariate_generating_functions Failed example: F1.asymptotics(p, alpha, 2) Expected: (3*((2*a^2  5*a*b + 2*b^2)*r^2 + (a + b)*r + 3)*((1/3)^(a)*(1/3)^(b))^r, (1/3)^(a)*(1/3)^(b), 3*(2*a^2  5*a*b + 2*b^2)*r^2  3*(a + b)*r  9) Got: (3*((2*a^2  5*a*b + 2*b^2)*r^2 + (a + b)*r + 3)*(1/((1/3)^a*(1/3)^b))^r, 1/((1/3)^a*(1/3)^b), 3*(2*a^2  5*a*b + 2*b^2)*r^2  3*(a + b)*r  9) ********************************************************************** File "src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py", line 2161, in sage.rings.asymptotic.asymptotics_multivariate_generating_functions.FractionWithFactoredDenominator.? Failed example: F.asymptotics_multiple(p, alpha, 2, var('r')) # long time Expected: (3*((1/3)^(a)*(1/3)^(b))^r*e^(2/3), (1/3)^(a)*(1/3)^(b), 3*e^(2/3)) Got: (3*(1/((1/3)^a*(1/3)^b))^r*e^(2/3), 1/((1/3)^a*(1/3)^b), 3*e^(2/3)) ********************************************************************** 2 items had failures: 1 of 77 in sage.rings.asymptotic.asymptotics_multivariate_generating_functions
and
Failed example: SR(CDF(1/2)).arccosh() # rel tol 1e15 Expected: 1.0471975511965976*I Got: 6.372599439108525e21 + 1.0471975511965979*I
comment:50 Changed 4 years ago by
Replying to chapoton:
Failed example: SR(CDF(1/2)).arccosh() # rel tol 1e15 Expected: 1.0471975511965976*I Got: 6.372599439108525e21 + 1.0471975511965979*I
Not confirmed. You apparently don't have the dependency installed, the newest arb.
comment:51 Changed 4 years ago by
Commit:  3b244e94f2b9bced8249d5abefb206bd419f8f43 → 814e54994ad4ef7fa456638bf7c7e6c82aad347f 

Branch pushed to git repo; I updated commit sha1. New commits:
814e549  24838: fix to doctests

comment:52 followup: 53 Changed 4 years ago by
The second doctest is still failing for me.
sage: version() 'SageMath version 8.3.beta7, Release Date: 20180623' sage: package_versions('standard')['arb'] ('2.13.0', '2.13.0')
comment:53 Changed 4 years ago by
Replying to chapoton:
The second doctest is still failing for me.
Can you please give the output of:
sage: CDF(1/2).arccosh() 1.0471975511965979*I sage: CBF(1/2).arccosh() [1.047197551196598 +/ 7.94e16]*I
comment:54 Changed 4 years ago by
sage: CDF(1/2).arccosh() 1.0471975511965979*I sage: CBF(1/2).arccosh() [+/ 4.90e16] + [1.047197551196598 +/ 7.94e16]*I
comment:55 Changed 4 years ago by
This is peculiar. I was under the impression that this fix: https://github.com/fredrikjohansson/arb/commit/7afd3bfaf1697739751d96d3665ef83e78c76820 would be in arb2.13.0. I just found out that it was committed Mar 5, while the release was Feb 23. I should have actively propagated that patch into our arb upgrade ticket.
Should I open a separate ticket for the arb patch or just put it into this branch?
I would have noticed earlier but for some reason I don't get the error with 2.13.0.
comment:56 Changed 4 years ago by
Ah, I had the patch still in the patches directory. New files are not deleted when changing branches.
comment:57 followup: 74 Changed 4 years ago by
Commit:  814e54994ad4ef7fa456638bf7c7e6c82aad347f → 6645f7923ee5bdd97219b0a475e3ae98e03de4e8 

Branch pushed to git repo; I updated commit sha1. New commits:
6645f79  24838: add arb commit 7afd3afbf

comment:58 Changed 4 years ago by
This is reviewed by Fredrik in https://github.com/fredrikjohansson/arb/pull/210
comment:60 Changed 4 years ago by
Commit:  6645f7923ee5bdd97219b0a475e3ae98e03de4e8 → 8e4e081d09ff059a8f192ec57ba0443587da3900 

Branch pushed to git repo; I updated commit sha1. New commits:
8e4e081  24838: bump arb version

comment:61 Changed 4 years ago by
Thanks. This should be ready to go.
Launching now "make ptestlong" once again, just to be sure.
comment:62 Changed 4 years ago by
Reviewers:  → Frédéric Chapoton 

Status:  needs_review → positive_review 
ok, all tests pass
comment:64 followup: 65 Changed 4 years ago by
Status:  positive_review → needs_work 

On 32bit:
File "src/sage/functions/hypergeometric.py", line 148, in sage.functions.hypergeometric Failed example: hypergeometric_U(2, 2, x).series(x == 3, 100).subs(x=1).n() Expected: 0.403652637676806 Got: 0.403651580752398
comment:65 followup: 67 Changed 4 years ago by
Replying to vbraun:
On 32bit:
File "src/sage/functions/hypergeometric.py", line 148, in sage.functions.hypergeometric Failed example: hypergeometric_U(2, 2, x).series(x == 3, 100).subs(x=1).n() Expected: 0.403652637676806 Got: 0.403651580752398
Pynac is not involved in the numerics (hypergeometric_U
is not a GinacFunction
), so the hypothesis is that the expression before .n()
is different with 32bit pynac0.7.22.
comment:66 Changed 4 years ago by
Cc:  Timo Kaufmann added 

comment:67 Changed 4 years ago by
Replying to rws:
Replying to vbraun:
On 32bit:
Pynac is not involved in the numerics (
hypergeometric_U
is not aGinacFunction
), so the hypothesis is that the expression before.n()
is different with 32bit pynac0.7.22.
We will never know. I failed to install 32bit versions of CentOS and ArchLinux in a VM, and I will not try again. There will be a ticket for this doctest failure, and the test marked "known bug (see ...".
comment:68 Changed 4 years ago by
Commit:  8e4e081d09ff059a8f192ec57ba0443587da3900 → e324b642c1f7c7cb98eea400abfe7c9a4296fcca 

Branch pushed to git repo; I updated commit sha1. New commits:
5f6c08e  24838: pkg version / chksum / remove old patch

3b244e9  24838: doctest fixes

814e549  24838: fix to doctests

6645f79  24838: add arb commit 7afd3afbf

8e4e081  24838: bump arb version

e324b64  24838: mark 32bit fail as known bug, see 25688

comment:69 Changed 4 years ago by
Status:  needs_work → positive_review 

comment:70 followup: 71 Changed 4 years ago by
This isn't really about this specific ticket (thanks for putting in the work for the upgrade), but is there a reason everybody seems to make seperate "pkg version / chksum" and "doctest fixes" patches?
Shouldn't each commit be one unit of changes for which the doctests still pass?
comment:71 followup: 72 Changed 4 years ago by
Replying to ghtimokau:
Shouldn't each commit be one unit of changes for which the doctests still pass?
The only reason I see for this is to make statistics based on number of commits meaningful. By splitting commits into parts (if there is such a natural split, maybe here it's less needed) I make the job of reviewing easier.
comment:72 Changed 4 years ago by
Replying to rws:
Replying to ghtimokau:
Shouldn't each commit be one unit of changes for which the doctests still pass?
The only reason I see for this is to make statistics based on number of commits meaningful. By splitting commits into parts (if there is such a natural split, maybe here it's less needed) I make the job of reviewing easier.
A much more practical reason is the use of gitbisect
or just generally being able to check out some interesting commit and build from it.
comment:73 Changed 4 years ago by
Branch:  u/rws/24838 → e324b642c1f7c7cb98eea400abfe7c9a4296fcca 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:74 followup: 75 Changed 4 years ago by
Commit:  e324b642c1f7c7cb98eea400abfe7c9a4296fcca 

Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
6645f79 24838: add arb commit 7afd3afbf
For the future it would greatly simplify packaging if you would add some documentation when adding a patch. A link to the github issue/pr and maybe a sentence about what it does would be sufficient. (The commit is actually 7afd3bfaf)
comment:75 Changed 4 years ago by
Replying to ghtimokau:
For the future it would greatly simplify packaging if you would add some documentation when adding a patch. A link to the github issue/pr and maybe a sentence about what it does would be sufficient. (The commit is actually 7afd3bfaf)
+1
Patches without any documentation or links should be rejected.
New commits:
version / chkum / rm patch
24838: doctest fixes