Opened 4 years ago

Closed 4 years ago

Last modified 13 months ago

#20449 closed defect (fixed)

Let the doctest of map_reduce work for single-core computers

Reported by: tmonteil Owned by:
Priority: major Milestone: sage-7.2
Component: combinatorics Keywords: sdl
Cc: hivert Merged in:
Authors: Florent Hivert Reviewers: Thierry Monteil
Report Upstream: N/A Work issues:
Branch: d969623 (Commits) Commit: d969623fd858087879e559754631c081d7d311e8
Dependencies: Stopgaps:

Description

Followup of #13580, which creates a module whose doctests assume that the host has at least two cores.

In the longer term, we could imagine to have a # multicore option for doctests, but let us just fix the doctests for this ticket to test whether the host has a single core first.

Change History (14)

comment:1 Changed 4 years ago by tmonteil

  • Priority changed from major to blocker

Let me put this ticket as blocker since the doctest are currently broken on single core computers.

comment:2 Changed 4 years ago by hivert

  • Branch set to u/hivert/let_the_doctest_of_map_reduce_work_for_single_core_computers

comment:3 Changed 4 years ago by hivert

  • Commit set to d969623fd858087879e559754631c081d7d311e8

Hi thiery,

I just pushed a 1 character fix. It allows two parallel processes even on single core machines. Note that on those kinds of machines, it make no sense to use map_reduce. The same functionality is provided by the run_serial method which makes no use of multi-core.

Can you test this patch on your patchbot and report failure if any ?

Florent


New commits:

d96962320449 : Tentative 1 character fix
Last edited 4 years ago by hivert (previous) (diff)

comment:4 Changed 4 years ago by tmonteil

Patchbot launched, let us see.

comment:5 Changed 4 years ago by hivert

Hi thiery,

How long will it takes ? I've to get up early tomorrow...

While waiting, I'm playing with SSE/AVX. I've a speedup of x20 for a function which sorts 16 bytes.

Florent

comment:6 Changed 4 years ago by hivert

Going to bed... I'll check you report tomorrow evening.

comment:7 Changed 4 years ago by tmonteil

I do not really like the fact that we change the code while the problem comes from the doctest, but at least the doctests now pass with that change, see http://patchbot.sagemath.org/log/20449/debian/8.3/i686/3.16.0-4-586/tmonteil-debian-jessie-32/2016-04-16%2000:11:02?short

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

comment:8 Changed 4 years ago by slabbe

I also had a decision like this (what to do when ncpus=1) to make recently in the parallelization of dancing links code:

https://github.com/sagemath/sage/blob/master/src/sage/combinat/matrices/dancing_links.pyx#L373

comment:9 in reply to: ↑ description Changed 4 years ago by jdemeyer

Replying to tmonteil:

In the longer term, we could imagine to have a # multicore option for doctests, but let us just fix the doctests for this ticket to test whether the host has a single core first.

Is the problem really in the doctest or in the functionality?

comment:10 Changed 4 years ago by vbraun

  • Priority changed from blocker to major

Any progress? IMHO a failing doctest on some particular setup isn't a blocker, there are far worse bugs than that...

comment:11 Changed 4 years ago by tmonteil

  • Authors set to Florent Hivert
  • Reviewers set to Thierry Monteil
  • Status changed from new to needs_review

comment:12 Changed 4 years ago by tmonteil

  • Status changed from needs_review to positive_review

comment:13 Changed 4 years ago by vbraun

  • Branch changed from u/hivert/let_the_doctest_of_map_reduce_work_for_single_core_computers to d969623fd858087879e559754631c081d7d311e8
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:14 Changed 13 months ago by tmonteil

  • Keywords sdl added
Note: See TracTickets for help on using tickets.