Opened 12 years ago

Closed 12 years ago

#7011 closed enhancement (fixed)

[with patch, positive review] fiddle with the number of threads automatically used for parallel testing

Reported by: ddrake Owned by: ddrake
Priority: major Milestone: sage-4.2
Component: doctest coverage Keywords:
Cc: mvngu, drkirkby, jhpalmieri Merged in:
Authors: Dan Drake Reviewers: John Palmieri
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

At #6283, we changed the parallel testing framework so that it automatically uses all the cores/threads available, but perhaps this is not the best solution.

Dave says (9:ticket:6283) "I would have personally not allowed the default to exceed 8", so maybe we can incorporate his limit in a way that still lets ordinary multicore computers be well-used:

  • NUM_THREADS defaults to 0, which is now interpreted in the sage-ptest script as min(cpu_count(), 8) -- so the default doesn't exceed 8, as Dave suggested.
  • if NUM_THREADS is -1, it just uses cpu_count().

On sage-devel, I suggested that a solution that works really well for 99+% of people is a good one -- and since most "regular" machines on which Sage is doctested have 8 or fewer cores, this still works fine for them, and with the above suggestion, people won't bring sage.math or t2.math to their knees.

Thoughts?

Attachments (2)

trac_7011.patch (2.3 KB) - added by ddrake 12 years ago.
add in default maximum of 8 threads
trac_7011-sage-root-makefile.patch (1.6 KB) - added by ddrake 12 years ago.
patch for $SAGE_ROOT/makefile

Download all attachments as: .zip

Change History (7)

comment:1 Changed 12 years ago by ddrake

  • Cc mvngu drkirkby jhpalmieri added

On second thought, scratch the -1 idea. We can just make one change to sage-ptest: line 267 could be

numthreads = min(8, multiprocessing.cpu_count())

Anyone who is desperate to saturate a machine with more than 8 cores can just specify it on the command line.

Changed 12 years ago by ddrake

add in default maximum of 8 threads

Changed 12 years ago by ddrake

patch for $SAGE_ROOT/makefile

comment:2 Changed 12 years ago by ddrake

  • Authors set to Dan Drake
  • Milestone set to sage-4.1.3
  • Owner changed from tbd to ddrake
  • Status changed from new to assigned
  • Summary changed from fiddle with the number of threads automatically used for parallel testing to [with patch, needs review] fiddle with the number of threads automatically used for parallel testing

I've uploaded patches for the sage_scripts repo, and for the root makefile. The second attachment is an ordinary unified diff.

comment:3 Changed 12 years ago by jhpalmieri

  • Summary changed from [with patch, needs review] fiddle with the number of threads automatically used for parallel testing to [with patch, positive review] fiddle with the number of threads automatically used for parallel testing

Looks good to me. Very helpful comments, too.

comment:4 Changed 12 years ago by jhpalmieri

  • Reviewers set to John Palmieri

comment:5 Changed 12 years ago by was

  • Resolution set to fixed
  • Status changed from positive_review to closed

merged into sage-4.1.2

Note: See TracTickets for help on using tickets.