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: sage-6.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 drose

  • Branch set to 15782
  • Dependencies set to 15781

comment:2 Changed 4 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:3 Changed 4 years ago by drose

  • 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 git

  • Commit set to e26305b934c92e86f22498b7f973539deda38eac

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

5bafb1b# Tue Oct 22 20:59:00 2013 -0400
e822a8a# Wed Nov 06 19:13:50 2013 -0500
e26305b# Thu Dec 05 11:37:18 2013 -0500

comment:5 Changed 4 years ago by git

  • Commit changed from e26305b934c92e86f22498b7f973539deda38eac to 1018224a13497f53c4e10136b64be32c5c555693

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

52ec0b3trac 15780. Change to projective morphism _call_ function.
15c25ecMerge branch 'u/drose/15780' of git://trac.sagemath.org/sage into ticket/15781
1018224Merge branch 'u/drose/15781' of git://trac.sagemath.org/sage into ticket/15782

comment:6 Changed 4 years ago by drose

  • Dependencies set to #15781

comment:7 Changed 3 years ago by bhutz

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

6be7361rebase 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
eb476a9trac 15781. Added documentation.
4db0494trac 15781. Added documentation to projective_morphism_helper.pyx.
e30ba5c# Wed Nov 06 19:13:50 2013 -0500
2f4e384trac 15781. Added documentation.
fb89b52trac 15780. Added documentation to projective morphism.
69cd8f4trac 15781. Added documentation to projective_morphism_helper.pyx.
e413188rebase to 6.2.beta7 and current 15780

comment:8 Changed 3 years ago by bhutz

  • 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 bhutz

  • Commit changed from e4131881531e902225804986f1c34a1d5e97385c to 15849e4343e26d4a3d34b9496bef7145062842d6

fixed the branch.


New commits:

bf9dd3b# Tue Oct 22 20:59:00 2013 -0400
9557e3c# Thu Dec 05 11:37:18 2013 -0500
01ddf06trac 15780. Change to projective morphism _call_ function.
15849e4rebased to 6.2.beta7 and current 15781

comment:10 Changed 3 years ago by git

  • Commit changed from 15849e4343e26d4a3d34b9496bef7145062842d6 to 07a4855ab9dceda090fa6b905151871f4aefe626

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

07a4855fixed to allow dehomogenization at different coordinates for domain and codomain

comment:11 Changed 3 years ago by bhutz

  • 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 git

  • Commit changed from 07a4855ab9dceda090fa6b905151871f4aefe626 to 6a65ce4de61f998d0de5a88a71f1240fa710a6c8

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

2ccf84dChanged doctest for fast_eval.
232a58dFixed issues from trac server comment 12.
3bb4284Completed changes to fufill issues from trac server comment 12.
f6df021Changed documentation and reduced memory load in _enum_points.
170205dRemoved src/sage/schemes/projective/projective_morphism_helper.c from being tracked.
5094e43rebase to 6.2.rc1
dd5c4cb# Tue Oct 22 20:59:00 2013 -0400
427c7e8# Thu Dec 05 11:37:18 2013 -0500
677a5b7fixed to allow dehomogenization at different coordinates for domain and codomain
6a65ce4rebase to new 15781 + fix _muiltiplier_mod

comment:13 Changed 3 years ago by git

  • Commit changed from 6a65ce4de61f998d0de5a88a71f1240fa710a6c8 to 95553473f35762804b97a3a52c715f3a2b8ac0a4

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

9555347documentation for affine_morphism.jacobian()

comment:14 Changed 3 years ago by bhutz

  • Authors changed from Dillon Rose to Dillon Rose, Ben Hutz

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 vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:16 Changed 3 years ago by git

  • Commit changed from 95553473f35762804b97a3a52c715f3a2b8ac0a4 to 2939a962e39957e7533331d2deb5be89d31d8d1b

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

512eb53Changed doctest for fast_eval.
42ae3eaFixed issues from trac server comment 12.
ab38c53Completed changes to fufill issues from trac server comment 12.
19fd231Changed documentation and reduced memory load in _enum_points.
e471d48rebase to 6.2.rc1
4d54c38# Tue Oct 22 20:59:00 2013 -0400
590251e# Thu Dec 05 11:37:18 2013 -0500
e206053fixed to allow dehomogenization at different coordinates for domain and codomain
f323c7erebase to new 15781 + fix _muiltiplier_mod
2939a96documentation for affine_morphism.jacobian()

comment:17 Changed 3 years ago by bhutz

history re-written to match 15781: removing all references to the .c file.

comment:18 Changed 3 years ago by drose

  • Status changed from needs_review to needs_work

comment:19 Changed 3 years ago by drose

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 git

  • Commit changed from 2939a962e39957e7533331d2deb5be89d31d8d1b to 109dd5866197b5d8ed54ffe9aaa608726de7d8a2

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

109dd5815782: added documentation and fixed homogenize for affine morphisms

comment:21 Changed 3 years ago by bhutz

  • 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 git

  • Commit changed from 109dd5866197b5d8ed54ffe9aaa608726de7d8a2 to ab25ae36d2a767ccc8591742356c916408f4fd3f

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

36e5116rebase to 6.2.rc1
bf928a9# Tue Oct 22 20:59:00 2013 -0400
0602ffctrac 15781. Added documentation.
b12dc03# Wed Nov 06 19:13:50 2013 -0500
4a30d5dChanged code to comply with style.
2a73bdc# Thu Dec 05 11:37:18 2013 -0500
a5b2c31fixed to allow dehomogenization at different coordinates for domain and codomain
d5eed71documentation for affine_morphism.jacobian()
646405215782: added documentation and fixed homogenize for affine morphisms
ab25ae315782: rebase to 15781 to fix merge conflict

comment:23 Changed 3 years ago by drose

I have re-reviewed this patch and everything builds and works correctly.

comment:24 Changed 3 years ago by drose

  • Status changed from needs_review to positive_review

comment:25 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

Conflicts with #15781 (illustrates why you shouldn't rebase)

comment:26 Changed 3 years ago by git

  • Commit changed from ab25ae36d2a767ccc8591742356c916408f4fd3f to 4f38b3db93d3ec691fb61a23f07118fa5fa29c9a

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

d8f8682Fixed parallelizaion using p_iter_fork.
8762498Fixed lazy attribute and merged 16168.
74a308aFixed doctest.
7c080abReduced memory load and fixed doctest.
916b2f0Fixed issues reported on trac server comment 28 and 29.
77c4abeFixed issues from trac server comment 12.
8704dfeCompleted changes to fufill issues from trac server comment 12.
71a35e8Changed documentation and reduced memory load in _enum_points.
3adee1crebase to 6.2.rc1
4f38b3dMerge branch 'ticket/15781' into ticket/15782

comment:27 Changed 3 years ago by bhutz

  • Status changed from needs_work to needs_review

I merged in 15781 to correct the merge conflict.

comment:28 Changed 3 years ago by drose

  • Status changed from needs_review to positive_review

comment:29 Changed 3 years ago by drose

I retested the ticket.  Everything works the same as before.

comment:30 Changed 3 years ago by vbraun

  • Branch changed from u/bhutz/ticket/15782 to 4f38b3db93d3ec691fb61a23f07118fa5fa29c9a
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.