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 cypari2 to 2.1.3 #33878

Closed
kliem opened this issue May 21, 2022 · 27 comments
Closed

Upgrade cypari2 to 2.1.3 #33878

kliem opened this issue May 21, 2022 · 27 comments

Comments

@kliem
Copy link
Contributor

kliem commented May 21, 2022

This is needed for python 3.11 support (#33842) and to make cypari compatible with current pari version

https://groups.google.com/g/sage-devel/c/jqmr6bDjDrk/m/XE2GlB_GBgAJ

Necessary follow ups:

  • make cypari thread safe: see cypari2 #116
  • remove optional build time dependency of cysignals on pari: see cypari2 #130

Depends on #33864

CC: @videlec @dimpase

Component: build

Author: Matthias Koeppe

Branch/Commit: 0ac9905

Reviewer: Dima Pasechnik

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

@kliem kliem added this to the sage-9.7 milestone May 21, 2022
@kliem
Copy link
Contributor Author

kliem commented May 21, 2022

Dependencies: #33864

@kliem
Copy link
Contributor Author

kliem commented May 21, 2022

Branch: public/33878

@kliem
Copy link
Contributor Author

kliem commented May 21, 2022

Commit: f3f1b5d

@kliem
Copy link
Contributor Author

kliem commented May 21, 2022

New commits:

92e9cffbuild/pkgs/cython: Update to 0.29.30
f3f1b5dtest new changes to pari

@kiwifb
Copy link
Member

kiwifb commented May 21, 2022

comment:3

Is there any reason you are using snapshots from your fork of cypari2? Or is it just for testing and development?

@kliem
Copy link
Contributor Author

kliem commented May 22, 2022

comment:4

Just for testing and development. My own GitHub account is lots faster with the GitHub actions than sagemath. Also I use this trac ticket for cysignal's CI. Once cypari and possibly cysignal are ready, we switch back to the official sources.

@mkoeppe
Copy link
Contributor

mkoeppe commented May 27, 2022

comment:5

Still getting

[cypari-2.1.3b1]   [7/7] Cythonizing cypari2/string_utils.pyx
[cypari-2.1.3b1]   cypari2/convert.c:3347:21: error: cannot take the address of an rvalue of type 'Py_ssize_t' (aka 'long')
[cypari-2.1.3b1]     __pyx_v_sizeptr = &Py_SIZE(((PyObject *)__pyx_v_x));
[cypari-2.1.3b1]                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

with this ticket

@kliem
Copy link
Contributor Author

kliem commented May 28, 2022

comment:6

Sorry about that. I messed up with the tags. I'm rather confident the fix is working, it's just that the fix is not contained in the tag I created.

@kliem
Copy link
Contributor Author

kliem commented May 28, 2022

comment:7

The new commit contains the recent changes.

The test release contains basically the following:

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2022

Changed commit from f3f1b5d to 481d782

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2022

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

481d782test cypari update

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2022

Changed commit from 481d782 to b494326

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2022

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

11a0e57build/bin/write-dockerfile.sh: Add bootstrap-conda
b494326Merge branch 'u/mkoeppe/fix_tox_docker_builds' of git://trac.sagemath.org/sage into public/33878

@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Aug 31, 2022
@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Upgrade cypari (and possibly cysignals) Upgrade cypari2 to 2.1.3 (and possibly cysignals) Nov 11, 2022
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 11, 2022

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

0ac9905build/pkgs/cypari: Update to 2.1.3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 11, 2022

Changed commit from b494326 to 0ac9905

@mkoeppe mkoeppe changed the title Upgrade cypari2 to 2.1.3 (and possibly cysignals) Upgrade cypari2 to 2.1.3 Nov 11, 2022
@mkoeppe

This comment has been minimized.

@videlec
Copy link
Contributor

videlec commented Nov 11, 2022

comment:16

Regarding the ticket description: to make pari threadsafe, the only way I explored so far was to use a "thread pool executor", see sagemath/cypari2#116. Because of its stack management, pari needs some care when used within threads.

@videlec

This comment has been minimized.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 11, 2022

comment:18

I don't remember the details about this, but there is also a separate related ticket: #28800

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 11, 2022

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 11, 2022

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 12, 2022

comment:21

Tests look OK, let's get this in

@dimpase
Copy link
Member

dimpase commented Nov 13, 2022

comment:22

lgtm - tested with Pari 2.,15.1

@dimpase
Copy link
Member

dimpase commented Nov 13, 2022

Changed reviewer from https://github.com/mkoeppe/sage/actions/runs/3448209182 to Dima Pasechnik

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 13, 2022

comment:23

Thanks!

@vbraun
Copy link
Member

vbraun commented Nov 15, 2022

Changed branch from public/33878 to 0ac9905

@vbraun vbraun closed this as completed in 9e0f156 Nov 15, 2022
kryzar pushed a commit to kryzar/sage that referenced this issue Feb 6, 2023
dimpase pushed a commit to tornaria/sage that referenced this issue Feb 7, 2023
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

6 participants