-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
a PariThreadPool to handle multithreading via Python #116
base: master
Are you sure you want to change the base?
Conversation
c0d2391
to
c493877
Compare
Currently not compatible with python 3.6: https://github.com/kliem/cypari2/actions/runs/2362588414 We can of course push the required python version, but this needs to be addressed. (Btw, the current github workflow needs to be modified such that cysignals links correctly to pari, otherwise lots of tests have a time out.) |
Sage requires python >= 3.7, so there is nothing wrong with requiring this for the next release here. |
Sorry for the delay: I was at a conference last week. I just checked, and this PR solves our issue in the LMFDB. Thank you very much @videlec! |
When I tried the original test in #107, it still segfaults
but the LMFDB debugger seems to work (I'm not sure where the difference is coming from). |
For me, the debugger still failed. |
What did you expect? |
My fault for the confusion I think. We'd taken some measures to avoid the bug, and I tried a test that seemed to work and thought it was fixed in a very simple way. Let me try again for real and I'll report back. |
I tried the example in the documentation of
I think I reinstalled cypari into Sage correctly (I added a multithreading.patch file with the contents of this PR into I have to work on some other projects today, but will return to this. Thanks for the help Vincent, and sorry for my cluelessness yesterday. |
I fixed my Sage 9.6 installation but, of course, can't run this exact code there since the |
28f2382
to
3660f17
Compare
Working with @videlec at Sage Days 117, we modified the code above that failed by changing
The hypothesis is that the threads are manipulating the main pari stack since there is no lock implemented. |
What does |
It means that the individual threads aren't responsible for converting python ints into pari integers, and thus don't touch the main pari stack. |
As another data point, when running on linux both versions work (with and without |
I happened to run doctests on
|
Create a class
PariThreadPool
hepler that allows to use the multithreading capabilities fromconcurrent.futures
.Fix #107