#15920 closed enhancement (fixed)
Parallelize Possible Periods functions for Projective Morphisms
Reported by:  drose  Owned by:  drose 

Priority:  minor  Milestone:  sage6.2 
Component:  algebraic geometry  Keywords:  Projective, Morphism, Parallelization 
Cc:  bhutz  Merged in:  
Authors:  Dillon Rose  Reviewers:  Ben Hutz 
Report Upstream:  N/A  Work issues:  
Branch:  8bcc13a (Commits)  Commit:  
Dependencies:  #15966  Stopgaps: 
Description
Parallelize Possible Periods functions for Projective Morphisms
Change History (22)
comment:1 Changed 5 years ago by
 Branch set to u/drose/15920
 Reviewers set to Ben Hutz
comment:2 Changed 5 years ago by
 Branch changed from u/drose/15920 to u/drose/ticket/15920
comment:3 Changed 5 years ago by
 Owner changed from (none) to drose
comment:4 Changed 5 years ago by
 Commit set to 6cf68fb06efe3160bf6cea766b26b2d176e9309d
comment:5 Changed 5 years ago by
 Cc bhutz added
 Keywords Projective Morphism Parallelization added
comment:6 Changed 5 years ago by
 Commit changed from 6cf68fb06efe3160bf6cea766b26b2d176e9309d to 500a0caa053ef4756e762e40bbd4f9455fba9b5d
Branch pushed to git repo; I updated commit sha1. New commits:
500a0ca  Parallelized possible_periods function

comment:7 Changed 5 years ago by
 Commit changed from 500a0caa053ef4756e762e40bbd4f9455fba9b5d to 8ab83fd966eb9e194e163b046f05400a439d712e
Branch pushed to git repo; I updated commit sha1. New commits:
8ab83fd  Fixed paralleization of all_rational_preimages

comment:8 Changed 5 years ago by
 Commit changed from 8ab83fd966eb9e194e163b046f05400a439d712e to 69dd970e65ad635573be4cc0cb0d8def1d442991
Branch pushed to git repo; I updated commit sha1. New commits:
69dd970  Changed doctest to reflect changes

comment:9 Changed 5 years ago by
 Dependencies set to 15966
 Status changed from new to needs_review
These two changes looks fine. However, there is a third place that should be done in parallel.
periodic_points=self.lift_to_rational_periodic(pos_points,B)
in self.rational_periodic_points()
Also, it is convention not to use CamelCase variables names. (i.e. parallelData, possiblePeriods should be not have the capital in them).
I'm also marking 15966 as a dependency since without the cleanup of the child process these create too many child process to function well.
comment:10 Changed 5 years ago by
 Status changed from needs_review to needs_work
comment:11 Changed 5 years ago by
 Commit changed from 69dd970e65ad635573be4cc0cb0d8def1d442991 to 0fe665b4789cfb9fbc86f8c50df27607fa225da3
comment:12 Changed 5 years ago by
 Commit changed from 0fe665b4789cfb9fbc86f8c50df27607fa225da3 to ec4486365c82e3ab4266a73055904b0027af7652
comment:13 Changed 5 years ago by
 Dependencies 15966 deleted
comment:14 Changed 5 years ago by
Remember to set the status on tickets when it is ready for review again, but it is still needs_work so I'll leave it.
The functionality all checks out on my tests. However, with that last parallelization bit, there are 3 more doctests that are failing due to point order in projective_morphism.py. So you should sort those as well.
comment:15 Changed 5 years ago by
 Commit changed from ec4486365c82e3ab4266a73055904b0027af7652 to 8bcc13ad139b2b3f83ae3c2c105915310795b1b8
Branch pushed to git repo; I updated commit sha1. New commits:
8bcc13a  Fixed bug in rational_periodic_points function.

comment:16 Changed 5 years ago by
 Created changed from 03/12/14 07:36:11 to 03/12/14 07:36:11
 Dependencies set to #15966
 Modified changed from 04/02/14 21:44:34 to 04/02/14 21:44:34
comment:17 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:18 Changed 5 years ago by
 Status changed from needs_review to needs_work
This still has the 3 doctest failures due to sorting from
sage t long projective_morphism.py
comment:19 Changed 5 years ago by
 Status changed from needs_work to positive_review
Actually, this is fine, I had failed to pull down all the commits.
comment:20 Changed 5 years ago by
 Status changed from positive_review to needs_work
I'm getting some weird errors when I run the possible_periods over and over again (even on the same function). It seems to be somewhat random which error and whether or not an error occurs). I may or may not have corrupted my sage, but I don't want to leave this positive_review until I resolve it.
Errors like
Traceback (most recent call last): ... TypeError: number of arguments does not match number of variables in parent
and
Traceback (most recent call last): .... TypeError: unsupported operand parent(s) for '*': 'Finite Field of size 2' and 'Finite Field of size 13'
comment:21 Changed 5 years ago by
 Branch changed from u/drose/ticket/15920 to 8bcc13ad139b2b3f83ae3c2c105915310795b1b8
 Resolution set to fixed
 Status changed from needs_work to closed
comment:22 Changed 5 years ago by
 Commit 8bcc13ad139b2b3f83ae3c2c105915310795b1b8 deleted
I had it already tested and it worked on the buildbot. Reopen if you can reproduce your failure...
Branch pushed to git repo; I updated commit sha1. New commits:
Parallelized all_rational_preimages function