Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#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:

GitHub link to the corresponding issue

Description

Parallelize Possible Periods functions for Projective Morphisms

Change History (22)

comment:1 Changed 9 years ago by drose

Authors: Dillon Rose
Branch: u/drose/15920
Reviewers: Ben Hutz

comment:2 Changed 9 years ago by drose

Branch: u/drose/15920u/drose/ticket/15920

comment:3 Changed 9 years ago by drose

Owner: set to drose

comment:4 Changed 9 years ago by git

Commit: 6cf68fb06efe3160bf6cea766b26b2d176e9309d

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

6cf68fbParallelized all_rational_preimages function

comment:5 Changed 9 years ago by drose

Cc: bhutz added
Keywords: Projective Morphism Parallelization added

comment:6 Changed 9 years ago by git

Commit: 6cf68fb06efe3160bf6cea766b26b2d176e9309d500a0caa053ef4756e762e40bbd4f9455fba9b5d

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

500a0caParallelized possible_periods function

comment:7 Changed 9 years ago by git

Commit: 500a0caa053ef4756e762e40bbd4f9455fba9b5d8ab83fd966eb9e194e163b046f05400a439d712e

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

8ab83fdFixed paralleization of all_rational_preimages

comment:8 Changed 9 years ago by git

Commit: 8ab83fd966eb9e194e163b046f05400a439d712e69dd970e65ad635573be4cc0cb0d8def1d442991

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

69dd970Changed doctest to reflect changes

comment:9 Changed 9 years ago by bhutz

Dependencies: 15966
Status: newneeds_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 bhutz

Status: needs_reviewneeds_work

comment:11 Changed 9 years ago by git

Commit: 69dd970e65ad635573be4cc0cb0d8def1d4429910fe665b4789cfb9fbc86f8c50df27607fa225da3

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

db46450Merge branch 'u/drose/ticket/15966' of git://trac.sagemath.org/sage into ticket/15920
0fe665bRemoved camelCase variables.

comment:12 Changed 9 years ago by git

Commit: 0fe665b4789cfb9fbc86f8c50df27607fa225da3ec4486365c82e3ab4266a73055904b0027af7652

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

d51711cParallelized rational_periodic_points function.
ec44863Changed documentation and doctests because of parallelization.

comment:13 Changed 9 years ago by drose

Dependencies: 15966

comment:14 Changed 9 years ago by bhutz

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 git

Commit: ec4486365c82e3ab4266a73055904b0027af76528bcc13ad139b2b3f83ae3c2c105915310795b1b8

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

8bcc13aFixed bug in rational_periodic_points function.

comment:16 Changed 9 years ago by drose

Created: Mar 12, 2014, 7:36:11 AMMar 12, 2014, 7:36:11 AM
Dependencies: #15966
Modified: Apr 2, 2014, 9:44:34 PMApr 2, 2014, 9:44:34 PM

comment:17 Changed 9 years ago by drose

Status: needs_workneeds_review

comment:18 Changed 9 years ago by bhutz

Status: needs_reviewneeds_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 bhutz

Status: needs_workpositive_review

Actually, this is fine, I had failed to pull down all the commits.

comment:20 Changed 9 years ago by bhutz

Status: positive_reviewneeds_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 vbraun

Branch: u/drose/ticket/159208bcc13ad139b2b3f83ae3c2c105915310795b1b8
Resolution: fixed
Status: needs_workclosed

comment:22 Changed 9 years ago by vbraun

Commit: 8bcc13ad139b2b3f83ae3c2c105915310795b1b8

I had it already tested and it worked on the buildbot. Reopen if you can reproduce your failure...

Note: See TracTickets for help on using tickets.