#23713 closed defect (fixed)
Support SAGE_NUM_THREADS everywhere for parallellism
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | blocker | Milestone: | sage-8.1 |
Component: | doctest framework | Keywords: | |
Cc: | hivert | Merged in: | |
Authors: | Jeroen Demeyer | Reviewers: | Florent Hivert |
Report Upstream: | N/A | Work issues: | |
Branch: | f5e5765 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
There are several independent implementations in Sage to determine the number of threads or processors for a parallel computation. Instead, these should be one way to do that and it should support the environment variable SAGE_NUM_THREADS
, which is precisely meant for that.
On top of that, doctests should never use too many threads. So we set SAGE_NUM_THREADS=2
while doctesting, analogous to #23892.
This fixes doctests in:
- parallel map/reduce
- cryptominisat
- cbc package: coin backend for MILP
Change History (12)
comment:1 Changed 5 years ago by
- Description modified (diff)
- Summary changed from Parallel map/reduce doctests fail on machine with lots of CPUs to Support SAGE_NUM_THREADS_PARALLEL in parallel map/reduce
comment:2 Changed 5 years ago by
- Branch set to u/jdemeyer/support_sage_num_threads_parallel_in_parallel_map_reduce
comment:3 Changed 5 years ago by
- Commit set to 282c95423512fe2c14481ab83ab29cfa4c64061f
- Status changed from new to needs_review
comment:4 Changed 5 years ago by
- Component changed from doctest coverage to doctest framework
- Description modified (diff)
- Priority changed from critical to blocker
- Status changed from needs_review to needs_work
- Summary changed from Support SAGE_NUM_THREADS_PARALLEL in parallel map/reduce to Support SAGE_NUM_THREADS for parallellism
comment:5 Changed 5 years ago by
- Description modified (diff)
comment:6 Changed 5 years ago by
- Commit changed from 282c95423512fe2c14481ab83ab29cfa4c64061f to f5e5765874b291d48bf4d8fede927befbddddfb7
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f5e5765 | Support SAGE_NUM_THREADS for all parallellism
|
comment:7 Changed 5 years ago by
- Description modified (diff)
comment:8 Changed 5 years ago by
- Summary changed from Support SAGE_NUM_THREADS for parallellism to Support SAGE_NUM_THREADS everywhere for parallellism
comment:9 Changed 5 years ago by
- Status changed from needs_work to needs_review
comment:10 Changed 5 years ago by
- Reviewers set to Florent Hivert
- Status changed from needs_review to positive_review
Hi Jeroen,
Thanks for taking care of this one (and all the others) ! It's good to go for me. I assume that the limit SAGE_NUM_THREADS=2
si set up to be able to doctests on small machines. It seems a little low to me. I'd rather put 4 just to shake up a little the parallel feature. Except for that, everything loos ok to me.
Florent
comment:11 Changed 5 years ago by
- Branch changed from u/jdemeyer/support_sage_num_threads_parallel_in_parallel_map_reduce to f5e5765874b291d48bf4d8fede927befbddddfb7
- Resolution set to fixed
- Status changed from positive_review to closed
comment:12 Changed 4 years ago by
- Commit f5e5765874b291d48bf4d8fede927befbddddfb7 deleted
See #24937 for a follow up.
New commits:
Support SAGE_NUM_THREADS_PARALLEL in parallel map/reduce