#15920 closed enhancement (fixed)
Parallelize Possible Periods functions for Projective Morphisms
Reported by: | drose | Owned by: | drose |
---|---|---|---|
Priority: | minor | Milestone: | sage-6.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, GitHub, GitLab) | Commit: | |
Dependencies: | #15966 | Stopgaps: |
Description
Parallelize Possible Periods functions for Projective Morphisms
Change History (22)
comment:1 Changed 9 years ago by
Authors: | → Dillon Rose |
---|---|
Branch: | → u/drose/15920 |
Reviewers: | → Ben Hutz |
comment:2 Changed 9 years ago by
Branch: | u/drose/15920 → u/drose/ticket/15920 |
---|
comment:3 Changed 9 years ago by
Owner: | set to drose |
---|
comment:4 Changed 9 years ago by
Commit: | → 6cf68fb06efe3160bf6cea766b26b2d176e9309d |
---|
comment:5 Changed 9 years ago by
Cc: | bhutz added |
---|---|
Keywords: | Projective Morphism Parallelization added |
comment:6 Changed 9 years ago by
Commit: | 6cf68fb06efe3160bf6cea766b26b2d176e9309d → 500a0caa053ef4756e762e40bbd4f9455fba9b5d |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
500a0ca | Parallelized possible_periods function
|
comment:7 Changed 9 years ago by
Commit: | 500a0caa053ef4756e762e40bbd4f9455fba9b5d → 8ab83fd966eb9e194e163b046f05400a439d712e |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
8ab83fd | Fixed paralleization of all_rational_preimages
|
comment:8 Changed 9 years ago by
Commit: | 8ab83fd966eb9e194e163b046f05400a439d712e → 69dd970e65ad635573be4cc0cb0d8def1d442991 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
69dd970 | Changed doctest to reflect changes
|
comment:9 Changed 9 years ago by
Dependencies: | → 15966 |
---|---|
Status: | new → 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 9 years ago by
Status: | needs_review → needs_work |
---|
comment:11 Changed 9 years ago by
Commit: | 69dd970e65ad635573be4cc0cb0d8def1d442991 → 0fe665b4789cfb9fbc86f8c50df27607fa225da3 |
---|
comment:12 Changed 9 years ago by
Commit: | 0fe665b4789cfb9fbc86f8c50df27607fa225da3 → ec4486365c82e3ab4266a73055904b0027af7652 |
---|
comment:13 Changed 9 years ago by
Dependencies: | 15966 |
---|
comment:14 Changed 9 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 9 years ago by
Commit: | ec4486365c82e3ab4266a73055904b0027af7652 → 8bcc13ad139b2b3f83ae3c2c105915310795b1b8 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
8bcc13a | Fixed bug in rational_periodic_points function.
|
comment:16 Changed 9 years ago by
Created: | Mar 12, 2014, 7:36:11 AM → Mar 12, 2014, 7:36:11 AM |
---|---|
Dependencies: | → #15966 |
Modified: | Apr 2, 2014, 9:44:34 PM → Apr 2, 2014, 9:44:34 PM |
comment:17 Changed 9 years ago by
Status: | needs_work → needs_review |
---|
comment:18 Changed 9 years ago by
Status: | needs_review → needs_work |
---|
This still has the 3 doctest failures due to sorting from
sage -t --long projective_morphism.py
comment:19 Changed 9 years ago by
Status: | needs_work → positive_review |
---|
Actually, this is fine, I had failed to pull down all the commits.
comment:20 Changed 9 years ago by
Status: | positive_review → 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 9 years ago by
Branch: | u/drose/ticket/15920 → 8bcc13ad139b2b3f83ae3c2c105915310795b1b8 |
---|---|
Resolution: | → fixed |
Status: | needs_work → closed |
comment:22 Changed 9 years ago by
Commit: | 8bcc13ad139b2b3f83ae3c2c105915310795b1b8 |
---|
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