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:  sage6.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
 Keywords projective added; propjective removed
 Owner changed from (none) to drose
comment:2 Changed 5 years ago by
 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
 Commit set to bf3c19f56747cc4c01f6ce85d3aa66e93f8d2067
comment:4 Changed 5 years ago by
 Status changed from new to needs_review
comment:5 Changed 5 years ago by
 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
 Commit changed from bf3c19f56747cc4c01f6ce85d3aa66e93f8d2067 to c858d5467daed909d63961fe5b997b51bc524965
Branch pushed to git repo; I updated commit sha1. New commits:
c858d54  Switched parallelization to p_iter_fork.

comment:7 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:8 Changed 5 years ago by
 Commit changed from c858d5467daed909d63961fe5b997b51bc524965 to 0a1cf6d74facf3cb8dc25611a158b77021799691
Branch pushed to git repo; I updated commit sha1. New commits:
0a1cf6d  Added documentation on new keyword ncpus.

comment:9 Changed 5 years ago by
 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
 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:
be46079  moved import to header

comment:11 Changed 5 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:12 Changed 5 years ago by
 Branch changed from u/bhutz/ticket/16168 to be46079a2b064ce01aa7a7921a2086938e6d9022
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Changed code to comply with style.