Opened 4 years ago
Closed 3 years ago
#15782 closed enhancement (fixed)
Increase Performance of Multiplier in Projective Morphism
Reported by:  drose  Owned by:  drose 

Priority:  minor  Milestone:  sage6.3 
Component:  algebraic geometry  Keywords:  multiplier, projective 
Cc:  bhutz  Merged in:  
Authors:  Dillon Rose, Ben Hutz  Reviewers:  Ben Hutz, Dillon Rose 
Report Upstream:  N/A  Work issues:  
Branch:  4f38b3d (Commits)  Commit:  4f38b3db93d3ec691fb61a23f07118fa5fa29c9a 
Dependencies:  #15781  Stopgaps: 
Description
Increase Performance of Multiplier in Projective Morphism
Change History (30)
comment:1 Changed 4 years ago by
 Branch set to 15782
 Dependencies set to 15781
comment:2 Changed 4 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:3 Changed 4 years ago by
 Branch changed from 15782 to u/drose/15782
 Cc bhutz added
 Component changed from performance to algebraic geometry
 Dependencies 15781 deleted
 Owner changed from (none) to drose
 Priority changed from major to minor
comment:4 Changed 4 years ago by
 Commit set to e26305b934c92e86f22498b7f973539deda38eac
comment:5 Changed 4 years ago by
 Commit changed from e26305b934c92e86f22498b7f973539deda38eac to 1018224a13497f53c4e10136b64be32c5c555693
Branch pushed to git repo; I updated commit sha1. New commits:
52ec0b3  trac 15780. Change to projective morphism _call_ function.

15c25ec  Merge branch 'u/drose/15780' of git://trac.sagemath.org/sage into ticket/15781

1018224  Merge branch 'u/drose/15781' of git://trac.sagemath.org/sage into ticket/15782

comment:6 Changed 4 years ago by
 Dependencies set to #15781
comment:7 Changed 3 years ago by
 Branch changed from u/drose/15782 to u/bhutz/ticket/15781
 Commit changed from 1018224a13497f53c4e10136b64be32c5c555693 to e4131881531e902225804986f1c34a1d5e97385c
rebased to 6.2.beta7
Last 10 new commits:
6be7361  rebase to 6.2.beta7 and remove whitespace

65eef1c  # Tue Oct 22 20:59:00 2013 0400

18d030f  # Wed Nov 06 19:13:50 2013 0500

eb476a9  trac 15781. Added documentation.

4db0494  trac 15781. Added documentation to projective_morphism_helper.pyx.

e30ba5c  # Wed Nov 06 19:13:50 2013 0500

2f4e384  trac 15781. Added documentation.

fb89b52  trac 15780. Added documentation to projective morphism.

69cd8f4  trac 15781. Added documentation to projective_morphism_helper.pyx.

e413188  rebase to 6.2.beta7 and current 15780

comment:8 Changed 3 years ago by
 Branch changed from u/bhutz/ticket/15781 to u/bhutz/ticket/15782
 Created changed from 02/03/14 15:07:27 to 02/03/14 15:07:27
 Modified changed from 04/10/14 15:00:21 to 04/10/14 15:00:21
comment:9 Changed 3 years ago by
 Commit changed from e4131881531e902225804986f1c34a1d5e97385c to 15849e4343e26d4a3d34b9496bef7145062842d6
comment:10 Changed 3 years ago by
 Commit changed from 15849e4343e26d4a3d34b9496bef7145062842d6 to 07a4855ab9dceda090fa6b905151871f4aefe626
Branch pushed to git repo; I updated commit sha1. New commits:
07a4855  fixed to allow dehomogenization at different coordinates for domain and codomain

comment:11 Changed 3 years ago by
 Reviewers set to Ben Hutz
 Status changed from new to needs_review
fixed the dehomogenization to allow for different coordinates in domain and codomain. Need dependencies fixed before functionality can be tested.
comment:12 Changed 3 years ago by
 Commit changed from 07a4855ab9dceda090fa6b905151871f4aefe626 to 6a65ce4de61f998d0de5a88a71f1240fa710a6c8
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
2ccf84d  Changed doctest for fast_eval.

232a58d  Fixed issues from trac server comment 12.

3bb4284  Completed changes to fufill issues from trac server comment 12.

f6df021  Changed documentation and reduced memory load in _enum_points.

170205d  Removed src/sage/schemes/projective/projective_morphism_helper.c from being tracked.

5094e43  rebase to 6.2.rc1

dd5c4cb  # Tue Oct 22 20:59:00 2013 0400

427c7e8  # Thu Dec 05 11:37:18 2013 0500

677a5b7  fixed to allow dehomogenization at different coordinates for domain and codomain

6a65ce4  rebase to new 15781 + fix _muiltiplier_mod

comment:13 Changed 3 years ago by
 Commit changed from 6a65ce4de61f998d0de5a88a71f1240fa710a6c8 to 95553473f35762804b97a3a52c715f3a2b8ac0a4
Branch pushed to git repo; I updated commit sha1. New commits:
9555347  documentation for affine_morphism.jacobian()

comment:14 Changed 3 years ago by
I've probably made too many changes to be the sole reviewer now. However, this ticket now looks complete to me and all functionality tests ok.
comment:15 Changed 3 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:16 Changed 3 years ago by
 Commit changed from 95553473f35762804b97a3a52c715f3a2b8ac0a4 to 2939a962e39957e7533331d2deb5be89d31d8d1b
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
512eb53  Changed doctest for fast_eval.

42ae3ea  Fixed issues from trac server comment 12.

ab38c53  Completed changes to fufill issues from trac server comment 12.

19fd231  Changed documentation and reduced memory load in _enum_points.

e471d48  rebase to 6.2.rc1

4d54c38  # Tue Oct 22 20:59:00 2013 0400

590251e  # Thu Dec 05 11:37:18 2013 0500

e206053  fixed to allow dehomogenization at different coordinates for domain and codomain

f323c7e  rebase to new 15781 + fix _muiltiplier_mod

2939a96  documentation for affine_morphism.jacobian()

comment:17 Changed 3 years ago by
history rewritten to match 15781: removing all references to the .c file.
comment:18 Changed 3 years ago by
 Status changed from needs_review to needs_work
comment:19 Changed 3 years ago by
All the updates work. All doctests pass and documentation builds. However, I believe more documentation is needed to explain these code changes and why they improve the performance.
comment:20 Changed 3 years ago by
 Commit changed from 2939a962e39957e7533331d2deb5be89d31d8d1b to 109dd5866197b5d8ed54ffe9aaa608726de7d8a2
Branch pushed to git repo; I updated commit sha1. New commits:
109dd58  15782: added documentation and fixed homogenize for affine morphisms

comment:21 Changed 3 years ago by
 Reviewers changed from Ben Hutz to Ben Hutz, Dillon Rose
 Status changed from needs_work to needs_review
When I added more documentation I realized that with the updated dehomogenize
function, the affine homogenize
no longer worked in a similar way. That has now been updated.
added Dillon as reviewer
comment:22 Changed 3 years ago by
 Commit changed from 109dd5866197b5d8ed54ffe9aaa608726de7d8a2 to ab25ae36d2a767ccc8591742356c916408f4fd3f
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
36e5116  rebase to 6.2.rc1

bf928a9  # Tue Oct 22 20:59:00 2013 0400

0602ffc  trac 15781. Added documentation.

b12dc03  # Wed Nov 06 19:13:50 2013 0500

4a30d5d  Changed code to comply with style.

2a73bdc  # Thu Dec 05 11:37:18 2013 0500

a5b2c31  fixed to allow dehomogenization at different coordinates for domain and codomain

d5eed71  documentation for affine_morphism.jacobian()

6464052  15782: added documentation and fixed homogenize for affine morphisms

ab25ae3  15782: rebase to 15781 to fix merge conflict

comment:23 Changed 3 years ago by
I have rereviewed this patch and everything builds and works correctly.
comment:24 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:25 Changed 3 years ago by
 Status changed from positive_review to needs_work
Conflicts with #15781 (illustrates why you shouldn't rebase)
comment:26 Changed 3 years ago by
 Commit changed from ab25ae36d2a767ccc8591742356c916408f4fd3f to 4f38b3db93d3ec691fb61a23f07118fa5fa29c9a
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
d8f8682  Fixed parallelizaion using p_iter_fork.

8762498  Fixed lazy attribute and merged 16168.

74a308a  Fixed doctest.

7c080ab  Reduced memory load and fixed doctest.

916b2f0  Fixed issues reported on trac server comment 28 and 29.

77c4abe  Fixed issues from trac server comment 12.

8704dfe  Completed changes to fufill issues from trac server comment 12.

71a35e8  Changed documentation and reduced memory load in _enum_points.

3adee1c  rebase to 6.2.rc1

4f38b3d  Merge branch 'ticket/15781' into ticket/15782

comment:27 Changed 3 years ago by
 Status changed from needs_work to needs_review
I merged in 15781 to correct the merge conflict.
comment:28 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:29 Changed 3 years ago by
I retested the ticket. Everything works the same as before.
comment:30 Changed 3 years ago by
 Branch changed from u/bhutz/ticket/15782 to 4f38b3db93d3ec691fb61a23f07118fa5fa29c9a
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
# Tue Oct 22 20:59:00 2013 0400
# Wed Nov 06 19:13:50 2013 0500
# Thu Dec 05 11:37:18 2013 0500