Opened 4 years ago

Closed 3 years ago

Last modified 3 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) Commit:
Dependencies: #15966 Stopgaps:

Description

Parallelize Possible Periods functions for Projective Morphisms

Change History (22)

comment:1 Changed 4 years ago by drose

  • Authors set to Dillon Rose
  • Branch set to u/drose/15920
  • Reviewers set to Ben Hutz

comment:2 Changed 4 years ago by drose

  • Branch changed from u/drose/15920 to u/drose/ticket/15920

comment:3 Changed 4 years ago by drose

  • Owner changed from (none) to drose

comment:4 Changed 4 years ago by git

  • Commit set to 6cf68fb06efe3160bf6cea766b26b2d176e9309d

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

6cf68fbParallelized all_rational_preimages function

comment:5 Changed 4 years ago by drose

  • Cc bhutz added
  • Keywords Projective Morphism Parallelization added

comment:6 Changed 4 years ago by git

  • Commit changed from 6cf68fb06efe3160bf6cea766b26b2d176e9309d to 500a0caa053ef4756e762e40bbd4f9455fba9b5d

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

500a0caParallelized possible_periods function

comment:7 Changed 4 years ago by git

  • Commit changed from 500a0caa053ef4756e762e40bbd4f9455fba9b5d to 8ab83fd966eb9e194e163b046f05400a439d712e

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

8ab83fdFixed paralleization of all_rational_preimages

comment:8 Changed 4 years ago by git

  • Commit changed from 8ab83fd966eb9e194e163b046f05400a439d712e to 69dd970e65ad635573be4cc0cb0d8def1d442991

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

69dd970Changed doctest to reflect changes

comment:9 Changed 4 years ago by bhutz

  • 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 4 years ago by bhutz

  • Status changed from needs_review to needs_work

comment:11 Changed 3 years ago by git

  • Commit changed from 69dd970e65ad635573be4cc0cb0d8def1d442991 to 0fe665b4789cfb9fbc86f8c50df27607fa225da3

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 3 years ago by git

  • Commit changed from 0fe665b4789cfb9fbc86f8c50df27607fa225da3 to ec4486365c82e3ab4266a73055904b0027af7652

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 3 years ago by drose

  • Dependencies 15966 deleted

comment:14 Changed 3 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 3 years ago by git

  • Commit changed from ec4486365c82e3ab4266a73055904b0027af7652 to 8bcc13ad139b2b3f83ae3c2c105915310795b1b8

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

8bcc13aFixed bug in rational_periodic_points function.

comment:16 Changed 3 years ago by drose

  • 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 3 years ago by drose

  • Status changed from needs_work to needs_review

comment:18 Changed 3 years ago by bhutz

  • 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 3 years ago by bhutz

  • Status changed from needs_work to positive_review

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

comment:20 Changed 3 years ago by bhutz

  • 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 3 years ago by vbraun

  • Branch changed from u/drose/ticket/15920 to 8bcc13ad139b2b3f83ae3c2c105915310795b1b8
  • Resolution set to fixed
  • Status changed from needs_work to closed

comment:22 Changed 3 years ago by vbraun

  • Commit 8bcc13ad139b2b3f83ae3c2c105915310795b1b8 deleted

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.