Opened 5 years ago

Closed 5 years ago

#16168 closed defect (fixed)

use p_iter_fork in projective_morphism

Reported by: bhutz Owned by: drose
Priority: major Milestone: sage-6.3
Component: algebraic geometry Keywords: projective morphism parallel
Cc: Merged in:
Authors: Dillon Rose Reviewers: Ben Hutz
Report Upstream: N/A Work issues:
Branch: be46079 (Commits) Commit: be46079a2b064ce01aa7a7921a2086938e6d9022
Dependencies: Stopgaps:

Description

projective_morphism uses parallel calls in several functions. It uses the multiprocessing.py function parallel_iter. It seems on OSX that there is some issue with the function. For example, calling possible_periods on a function multiple times (usually 50 is sufficient) will result in a error which varies. This does seem appear to occur on linux.

The p_iter_fork function seems more stable and does not have this issue.

While we are adjusting how parallel works, I think a ncpus kwd should also be added.

Change History (12)

comment:1 Changed 5 years ago by bhutz

  • Keywords projective added; propjective removed
  • Owner changed from (none) to drose

comment:2 Changed 5 years ago by drose

  • Branch set to u/drose/ticket/16168
  • Created changed from 04/15/14 11:32:04 to 04/15/14 11:32:04
  • Modified changed from 04/15/14 11:32:26 to 04/15/14 11:32:26

comment:3 Changed 5 years ago by git

  • Commit set to bf3c19f56747cc4c01f6ce85d3aa66e93f8d2067

Branch pushed to git repo; I updated commit sha1. New commits:

bf3c19fChanged code to comply with style.

comment:4 Changed 5 years ago by drose

  • Status changed from new to needs_review

comment:5 Changed 5 years ago by bhutz

  • Status changed from needs_review to needs_work

parallel_iter is still being used in possible_periods.

Switch that to p_iter_fork and add the ncpus kwd.

comment:6 Changed 5 years ago by git

  • Commit changed from bf3c19f56747cc4c01f6ce85d3aa66e93f8d2067 to c858d5467daed909d63961fe5b997b51bc524965

Branch pushed to git repo; I updated commit sha1. New commits:

c858d54Switched parallelization to p_iter_fork.

comment:7 Changed 5 years ago by drose

  • Status changed from needs_work to needs_review

comment:8 Changed 5 years ago by git

  • Commit changed from c858d5467daed909d63961fe5b997b51bc524965 to 0a1cf6d74facf3cb8dc25611a158b77021799691

Branch pushed to git repo; I updated commit sha1. New commits:

0a1cf6dAdded documentation on new keyword ncpus.

comment:9 Changed 5 years ago by bhutz

  • Branch changed from u/drose/ticket/16168 to u/bhutz/ticket/16168
  • Modified changed from 04/23/14 03:52:15 to 04/23/14 03:52:15

comment:10 Changed 5 years ago by bhutz

  • Authors set to Dillon Rose
  • Commit changed from 0a1cf6d74facf3cb8dc25611a158b77021799691 to be46079a2b064ce01aa7a7921a2086938e6d9022
  • Reviewers set to Ben Hutz
  • Status changed from needs_review to positive_review

This looks good to me. I am unable to reproduce the errors I was seeing on OSX after this fix.

I made one minor change, which was to move the import out of the function and to the top of the file.


New commits:

be46079moved import to header

comment:11 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:12 Changed 5 years ago by vbraun

  • Branch changed from u/bhutz/ticket/16168 to be46079a2b064ce01aa7a7921a2086938e6d9022
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.