-
-
Notifications
You must be signed in to change notification settings - Fork 13
FEAT: Implementing is_integer and as_integer_ratio for QuadPrecision
#221
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
bbdd72c
num is losing prec
SwayamInSync cb3243d
adding is_integer
SwayamInSync b8a60a6
need to fix thread-safety
SwayamInSync 9f95d21
adding multithreading test
SwayamInSync 72b2023
lock init on py < 3.13
SwayamInSync 652c35f
only lock call
SwayamInSync 9c54981
debugging and TSan build info
SwayamInSync 798e503
Merge branch 'main' into 216
SwayamInSync 9ffcbfd
adding stubs
SwayamInSync 476ac4d
decorating stub
SwayamInSync 6856f5f
docs
SwayamInSync File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ngoldbaum precisely,
Sleef_snprintfhere is thread-unsafe, I believe just GIL won't help here as this is C routine and GIL only protects the Python objects.Not 100% sure so need your opinion that whether GIL would be enough or we can lock this region with
pthread_mutexThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that means that it's not safe to concurrently call
Sleef_snprintfsimultaneously in two threads (e.g. it's not re-entrant)?In that case, yeah, I think you need a global lock.
I'd avoid using pthreads directly because then you'd need to do something else on Windows. Instead, I'd use
PyMutexon Python 3.13 and newer andPyThread_type_lockon 3.12 and older. See e.g. the use oflapack_lite_lockinnumpy/linalg/umath_linalg.cppin NumPy, which solves a similar problem with the non-reentrant lapack_lite library.You could also switch to C++ and use a C++ standard library but then you need to be careful you don't deadlock with the GIL by making sure you release it before doing any possibly blocking calls. The built-in lock types have deadlock protection against the GIL so you don't need to go through that trouble.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also it'd be nice to add a multithreaded test for this as well. You can look at
test_multithreading.pyin NumPy for some patterns to use for multithreaded tests.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed a multithreaded test for now only for testing this. Will push the lock after the tests get done