Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to pynac-0.6.8 #21034

Closed
rwst opened this issue Jul 17, 2016 · 29 comments
Closed

Upgrade to pynac-0.6.8 #21034

rwst opened this issue Jul 17, 2016 · 29 comments

Comments

@rwst
Copy link

rwst commented Jul 17, 2016

The new Pynac version does:

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

CC: @paulmasson

Component: packages: standard

Author: Ralf Stephan

Branch/Commit: 0eb21db

Reviewer: Paul Masson

Issue created by migration from https://trac.sagemath.org/ticket/21034

@rwst rwst added this to the sage-7.3 milestone Jul 17, 2016
@rwst
Copy link
Author

rwst commented Jul 17, 2016

Branch: u/rws/upgrade_to_pynac_0_6_8

@rwst

This comment has been minimized.

@rwst
Copy link
Author

rwst commented Jul 17, 2016

New commits:

7fd4370pkg version / checksum
2f43ed9use Pynac instead of Maxima in ex.coefficients() and combine()
3784da9doc and doctest changes

@rwst
Copy link
Author

rwst commented Jul 17, 2016

Author: Ralf Stephan

@rwst
Copy link
Author

rwst commented Jul 17, 2016

Commit: 3784da9

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 17, 2016

comment:3

Started from a clean copy of 7.3.beta8 develop and pulled in changes for this ticket. Ran make and got this error:

The following package(s) may have failed to build (not necessarily
during this run of 'make all'):

* package: sagelib-7.3.beta5
  log file: /Users/Masson/Downloads/GitHub/sage/logs/pkgs/sagelib-7.3.beta5.log
  build directory: /Users/Masson/Downloads/GitHub/sage/local/var/tmp/sage/build/sagelib-7.3.beta5

Ran make again on develop with no problem.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 17, 2016

Reviewer: Paul Masson

@rwst

This comment has been minimized.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 20, 2016

comment:6

Any update on the 7.3.beta5 error message? Something on my end or yours?

@rwst
Copy link
Author

rwst commented Jul 21, 2016

comment:7

Replying to @paulmasson:

Started from a clean copy of 7.3.beta8 develop and pulled in changes for this ticket.

How did you "pull in" the changes? BTW, if you do git trac checkout 21034 you will always get the Sage version of the branch in 21034, regardless of your develop. Also a hint: make start will skip building the expensive step of building the docs.

You should also download the pynac tarball and copy it into upstream/ before making.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 21, 2016

comment:8

Originally did git trac pull 21034. This time did git trach checkout 21034 and getting same error. Then also copied pynac tarball to upstream folder and still getting error. Last bit not building is this:

[sagelib-7.3.beta8] [3/9] gcc -fno-strict-aliasing -I/Users/Masson/Downloads/GitHub/sage/local/var/tmp/sage/build/python2-2.7.10.p2/include -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -I/Users/Masson/Downloads/GitHub/sage/local/include -I/Users/Masson/Downloads/GitHub/sage/local/include/python2.7 -I/Users/Masson/Downloads/GitHub/sage/local/lib/python2.7/site-packages/numpy/core/include -I/Users/Masson/Downloads/GitHub/sage/src -I/Users/Masson/Downloads/GitHub/sage/src/sage/ext -I/Users/Masson/Downloads/GitHub/sage/src/build/cythonized -I/Users/Masson/Downloads/GitHub/sage/src/build/cythonized/sage/ext -I/Users/Masson/Downloads/GitHub/sage/local/include/python2.7 -c /Users/Masson/Downloads/GitHub/sage/src/build/cythonized/sage/symbolic/getitem.cpp -o build/temp.macosx-10.9-x86_64-2.7/Users/Masson/Downloads/GitHub/sage/src/build/cythonized/sage/symbolic/getitem.o -std=c++11 -fno-strict-aliasing
[sagelib-7.3.beta8] error: command 'gcc' failed with exit status 1
[sagelib-7.3.beta8] make[3]: *** [sage] Error 1

Can't review until it builds completely.

@rwst
Copy link
Author

rwst commented Jul 22, 2016

comment:9

In comment:5 you write about starting from beta8 but the output shows beta5. Can you please confirm that beta8 develop compiles cleanly? I'm asking because on the sage-devel there are several reports about problems of beta8 on MacOS.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 22, 2016

comment:10

7.3.beta8 develop compiles cleanly on Mac OS X El Capitan

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 23, 2016

Changed commit from 3784da9 to 0eb21db

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 23, 2016

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

0eb21dbMerge branch 'develop' into t/21034/upgrade_to_pynac_0_6_8

@rwst
Copy link
Author

rwst commented Jul 23, 2016

comment:12

beta9 is now merged into the branch and has all recent MacOS fixes. Please either do git trac pull 21034 in your local copy of this branch, or delete your local copy of this branch and do a git trac checkout 21034. And try again.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 23, 2016

comment:13

Sorry Ralf, but still getting the error. Curiously even though it references "sagelib-7.3.beta5.log" this file hasn't been modified building either beta8 or beta9.

Before you ask, 7.3.beta9 develop also compiles cleanly on Mac OS X El Capitan.

Looking back to the pynac 0.6.7 upgrade, there is a comment from Travis that looks relevant. This prompted me to see which beta included 0.6.7 and sure enough it was beta5.

I'll be the first to admit knowing nothing about building tarballs, but are you sure there isn't a problem there? The tarball for 0.6.8 isn't downloading automatically to the upstream folder.

@rwst
Copy link
Author

rwst commented Jul 24, 2016

comment:14

Paul, Sage cannot download pynac-0.6.8 because this is only possible after this ticket is merged. As a reviewer you need to download manually and copy it into upstream. Note however that the tarball might be deleted again by Sage at some time. So always make sure it's there before compiling this branch.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 24, 2016

comment:15

This time I downloaded the tarball to upstream, deleted the local branch, did git trac checkout 21034, confirmed the tarball was in place and ran make. Unfortunately, same error. Tarball still there after make errors out. What now?

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 25, 2016

comment:16

In an effort to move this forward, I did make distclean followed by make. The latter fails for both master and develop, so there are Mac issues at play. Will report on sage-devel.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 26, 2016

comment:17

Not a Mac problem after all. I accidentally merged #21006 some time ago which left behind an empty package folder named "prompt_toolkit". Any package following it alphabetically was ignored, including pynac.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 27, 2016

comment:18

Finally built this ticket! Some preliminary questions:

  1. The second plot in (complex) plot should ignore unsigned infinity value #18368 still throws an error. Have you modified the behavior of the zeta function in Pynac or just the polylog? If the latter, the doctest you included doesn't appear to address the problem with the zeta function on that ticket.

  2. Numerical evaluation of the beta function looks good, so that fixes Use mpmath for numerical evaluation of the beta function #15196. I have noticed however that evaluating the beta function at poles returns NaN rather than unsigned infinity. Is this something that needs to be addressed in Sage or Pynac?

  3. The description of Support GinacFunction._print_latex_() customization #20888 makes it sound like an existing LaTeX string in Pynac overrides one in Sage. Is that correct? If so, shouldn't entries in Sage always take precedence?

  4. This ticket doesn't appear to change anything for rewrite buggy Expression.coefficients() without Maxima #20455. Does that ticket need to be addressed in Sage using behavior now available from Pynac?

Will do more testing tomorrow. Thanks for putting up with my build issue.

@rwst
Copy link
Author

rwst commented Jul 27, 2016

comment:19
  1. Yes, only part of (complex) plot should ignore unsigned infinity value #18368 is fixed, 2) the infinity problem has to be addressed in Sage and it's the same problem as Corrections to infinities returned by mpmath #19439 so it should go in that ticket, 3) Yes, and this is only one example where Pynac code overrides Sage code (I don't think this is a problem in principle), 4) Good Question, in the process of implementation I had to revise my opinion, let's discuss that in rewrite buggy Expression.coefficients() without Maxima #20455.

@rwst

This comment has been minimized.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 27, 2016

comment:20

Doctests pass. Documentation builds. Outstanding issues will be addressed on indicated tickets.

Good to go.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 28, 2016

comment:21

Typo in ticket description referencing wrong ticket. Positive review stands.

@paulmasson

This comment has been minimized.

@rwst
Copy link
Author

rwst commented Jul 28, 2016

comment:22

Thanks for the review.

@vbraun
Copy link
Member

vbraun commented Aug 7, 2016

Changed branch from u/rws/upgrade_to_pynac_0_6_8 to 0eb21db

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants