Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#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:

Status badges

Description (last modified by jdemeyer)

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 4 years ago by jdemeyer

  • 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 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/support_sage_num_threads_parallel_in_parallel_map_reduce

comment:3 Changed 4 years ago by jdemeyer

  • Commit set to 282c95423512fe2c14481ab83ab29cfa4c64061f
  • Status changed from new to needs_review

New commits:

​282c954Support SAGE_NUM_THREADS_PARALLEL in parallel map/reduce

comment:4 Changed 4 years ago by jdemeyer

  • 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 4 years ago by jdemeyer

  • Description modified (diff)

comment:6 Changed 4 years ago by git

  • Commit changed from 282c95423512fe2c14481ab83ab29cfa4c64061f to f5e5765874b291d48bf4d8fede927befbddddfb7

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

​f5e5765Support SAGE_NUM_THREADS for all parallellism

comment:7 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:8 Changed 4 years ago by jdemeyer

  • Summary changed from Support SAGE_NUM_THREADS for parallellism to Support SAGE_NUM_THREADS everywhere for parallellism

comment:9 Changed 4 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:10 Changed 4 years ago by hivert

  • 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

Last edited 4 years ago by mderickx (previous) (diff)

comment:11 Changed 4 years ago by vbraun

  • 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 egourgoulhon

  • Commit f5e5765874b291d48bf4d8fede927befbddddfb7 deleted

See #24937 for a follow up.

Note: See TracTickets for help on using tickets.