Opened 6 years ago
Closed 2 months ago
#21129 closed enhancement (fixed)
ArakelovZhang pairing of rational maps
Reported by:  Paul Fili  Owned by:  

Priority:  minor  Milestone:  sage9.8 
Component:  dynamics  Keywords:  sd104, gsoc2022 
Cc:  Holly Krieger, Yuan Kong, Ben Hutz, Alexander Galarraga  Merged in:  
Authors:  Paul Fili, Holly Krieger, Jing Guo  Reviewers:  Alexander Galarraga 
Report Upstream:  N/A  Work issues:  
Branch:  9ce8f81 (Commits, GitHub, GitLab)  Commit:  9ce8f81a254707d3c5504fef58b329e4ec153f2f 
Dependencies:  #28779  Stopgaps: 
Description
We add a function to compute the dynamical ArakelovZhang pairing of two rational maps defined over number fields.
Change History (48)
comment:1 Changed 6 years ago by
Branch:  → u/paulfili/arakelov_zhang_pairing_of_rational_maps 

comment:2 Changed 6 years ago by
Cc:  Holly Krieger added 

Commit:  → a6cca7c9827e746d49cc0d1bbd44eb3bc9fab2e2 
Status:  new → needs_review 
comment:3 Changed 6 years ago by
Commit:  a6cca7c9827e746d49cc0d1bbd44eb3bc9fab2e2 → b3ea8584f52d9355629c4f82f6495a5aff3e742a 

comment:4 Changed 6 years ago by
Commit:  b3ea8584f52d9355629c4f82f6495a5aff3e742a → ec5eac2de69f479e86d087b38d7b6bb95b0fe4b9 

Branch pushed to git repo; I updated commit sha1. New commits:
ec5eac2  21129: added a needed import statement

comment:5 Changed 6 years ago by
please use python3compatible syntax for print (see https://wiki.sagemath.org/Python3compatible%20code)
comment:7 Changed 5 years ago by
Branch:  u/paulfili/arakelov_zhang_pairing_of_rational_maps → public/arakelov_zhang 

Commit:  ec5eac2de69f479e86d087b38d7b6bb95b0fe4b9 → 6ab8e9166ab907d94cbfd3b7be4612828587d26a 
comment:8 Changed 5 years ago by
Milestone:  sage7.4 → sage8.1 

Summary:  arakelovzhang pairing of rational maps → ArakelovZhang pairing of rational maps 
comment:9 Changed 5 years ago by
Component:  algebraic geometry → dynamics 

comment:10 Changed 5 years ago by
Status:  needs_review → needs_work 

comment:11 Changed 3 years ago by
Branch:  public/arakelov_zhang → u/ghyuankmath/AZpairing 

Cc:  Yuan Kong added 
Commit:  6ab8e9166ab907d94cbfd3b7be4612828587d26a → 357a0a1829c2045be996da2c58d14b4386a8fecb 
Milestone:  sage8.1 → sage9.0 
comment:12 Changed 3 years ago by
Commit:  357a0a1829c2045be996da2c58d14b4386a8fecb → 664acad2dab27d748412876f140f0a2e6d1d484c 

Branch pushed to git repo; I updated commit sha1. New commits:
664acad  Update Examples in Arakelov_Zhang_pairing

comment:13 Changed 3 years ago by
Dependencies:  → 28779 

Keywords:  sd104 added 
There is currently a bug in prime_of_bad_reduction, reported now in #28779, which affects one of the examples here. (Apparently the bug wasn't present when the examples were first computed!). I'd rather not remove a good example, so we are marking this as dependent on that bug fix.
comment:14 Changed 3 years ago by
Status:  needs_work → needs_review 

The bug in #28779 should be fixed. If you apply that patch, then this one, it should now be working.
comment:15 followup: 17 Changed 3 years ago by
notice that this is listed as 13000 lines of code change. It looks like git thinks you've deleted the entire file and rewritten it. It would be better not to lose the code change history. Can you see if you can get it to just have your changes?
comment:16 Changed 3 years ago by
Branch:  u/ghyuankmath/AZpairing → u/paulfili/AZpairing 

comment:17 Changed 3 years ago by
Commit:  664acad2dab27d748412876f140f0a2e6d1d484c → 2a8b393878cdf2162816f502a681d0aa944cfb94 

Replying to bhutz:
notice that this is listed as 13000 lines of code change. It looks like git thinks you've deleted the entire file and rewritten it. It would be better not to lose the code change history. Can you see if you can get it to just have your changes?
Done!
New commits:
ba813c7  Added Arakelov Zhang pairing function to fresh copy of projective_ds.py

0c4e9b2  Fixed error, clears LCM of denominators always, GCD as appropriate

2778cdb  Merge branch 'u/paulfili/normalize' of git://trac.sagemath.org/sage into azpairing

2a8b393  Updated AZ pairing and checked tests

comment:19 Changed 3 years ago by
Milestone:  sage9.0 → sage9.1 

Ticket retargeted after milestone closed
comment:20 Changed 3 years ago by
Milestone:  sage9.1 → sage9.2 

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.
comment:21 Changed 2 years ago by
Milestone:  sage9.2 → sage9.3 

comment:22 Changed 21 months ago by
Milestone:  sage9.3 → sage9.4 

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
comment:23 Changed 17 months ago by
Milestone:  sage9.4 → sage9.5 

Setting a new milestone for this ticket based on a cursory review.
comment:24 Changed 12 months ago by
Milestone:  sage9.5 → sage9.6 

Stalled in needs_review
or needs_info
; likely won't make it into Sage 9.5.
comment:25 Changed 8 months ago by
Milestone:  sage9.6 → sage9.7 

comment:26 Changed 3 months ago by
Authors:  Paul Fili, Holly Krieger → Paul Fili, Holly Krieger, Jing Guo 

Branch:  u/paulfili/AZpairing → u/ghguojing0/pairing 
Cc:  Ben Hutz Alexander Galarraga added 
Dependencies:  28779 → #28779 
Keywords:  gsoc2022 added 
Status:  needs_review → needs_work 
comment:27 Changed 3 months ago by
Commit:  2a8b393878cdf2162816f502a681d0aa944cfb94 → be35eb0bd582b35f767a5e7caf91421be5271d57 

comment:28 Changed 3 months ago by
Commit:  be35eb0bd582b35f767a5e7caf91421be5271d57 → b36bf4dacef583f02a90e4ec69ac74d7c22ab593 

Branch pushed to git repo; I updated commit sha1. New commits:
b36bf4d  21129: Format doc; TODO: Turn args to kwds properly

comment:29 Changed 3 months ago by
Commit:  b36bf4dacef583f02a90e4ec69ac74d7c22ab593 → f3e67ab931a0db50c1694250ed9eac5996a7943f 

Branch pushed to git repo; I updated commit sha1. New commits:
f3e67ab  21129: Change args to kwds

comment:30 Changed 3 months ago by
Commit:  f3e67ab931a0db50c1694250ed9eac5996a7943f → 9b43de16b094a12b6873f08ab6802647719adecb 

comment:31 Changed 3 months ago by
Commit:  9b43de16b094a12b6873f08ab6802647719adecb → c83217c488eebc39f19075762a0baa316d60698e 

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

comment:32 Changed 3 months ago by
Commit:  c83217c488eebc39f19075762a0baa316d60698e → 174b6864b59680453f65c22e91818dd1058e1f09 

Branch pushed to git repo; I updated commit sha1. New commits:
174b686  21129: Rename `RealField` and `f.domain.base_ring` vars to avoid

comment:33 Changed 3 months ago by
Commit:  174b6864b59680453f65c22e91818dd1058e1f09 → dcce8071104d83582fb9a74699c6e150f8872016 

Branch pushed to git repo; I updated commit sha1. New commits:
dcce807  21129: format examples

comment:34 Changed 3 months ago by
Status:  needs_work → needs_review 

comment:35 Changed 3 months ago by
Milestone:  sage9.7 → sage9.8 

comment:36 Changed 3 months ago by
Reviewers:  → Alexander Galarraga 

Status:  needs_review → needs_work 
The following throws an error:
K.<k> = CyclotomicField(3) P.<x,y> = ProjectiveSpace(K,1) f = DynamicalSystem_projective([x^2 + (2*k+2)*y^2, y^2]) g = DynamicalSystem_projective([x^2, y^2]) pairingval = f.arakelov_zhang_pairing(g, n=5); pairingval
The last example in the documentation is failing on my end. Does it pass for you?
Finally, some minor issues in the code. There is extra whitespace on line 1302. Line 1281 is too long, it should be wrapped into two lines, as should any line longer than that.
comment:37 Changed 3 months ago by
Commit:  dcce8071104d83582fb9a74699c6e150f8872016 → f1f7fb1985ca5aa64dafdb052b6ce6d19e520301 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
e64cb5f  Added Arakelov Zhang pairing function to fresh copy of projective_ds.py

580a8dc  Updated AZ pairing and checked tests

7dd124b  21129: Format doc; TODO: Turn args to kwds properly

6bf37d5  21129: Change args to kwds

bbe6180  21129: Complete `)` in misc.py

67e3590  21129: Better format and doc

be6f236  21129: Indent

0fdcfd2  21129: Rename `RealField` and `f.domain.base_ring` vars to avoid

f1f7fb1  21129: Fix docs and a bug when `f_disc` is not rational

comment:38 Changed 3 months ago by
Commit:  f1f7fb1985ca5aa64dafdb052b6ce6d19e520301 → ecf50141497051ed31083820cbd11c9710c8e2fd 

Branch pushed to git repo; I updated commit sha1. New commits:
ecf5014  21129: Wrap long lines

comment:39 Changed 3 months ago by
Status:  needs_work → needs_review 

Thank you, Alex. Good catch. That was caused by calling abs
before norm
for nonrational f_disc
. Everything is fixed now.
comment:40 Changed 2 months ago by
Status:  needs_review → needs_work 

Code itself looks good now. The test I was trying to run also doesn't fail now. Using Prop 4.3 in Birdy and Larson's paper on the pairing, I tested pairing two different dynamical systems, and the results were mathematically correct correct.
The only remaining problem is the example beginning on line 1224. According to Prop 18 of Petsche, Szpiro and Tucker, the value of the pairing should be exactly the global height of a. Thus, this is mathematically incorrect. Since it was passing before the code was modernized, it could have been something in the modernization. It is also possible that we just need to increase n
, or change the noise modifier to get this example to work. Take a look and try and see what could be going wrong, and if you get stuck, this is worth emailing Paul about.
comment:41 Changed 2 months ago by
Commit:  ecf50141497051ed31083820cbd11c9710c8e2fd → 22c177847d061cb6c183660cc07cf900a38026e2 

Branch pushed to git repo; I updated commit sha1. New commits:
22c1778  21129: Better indentation

comment:42 Changed 2 months ago by
Commit:  22c177847d061cb6c183660cc07cf900a38026e2 → 0f1e6a445e967e843d6a60c3a37a7a10c1aeaf67 

Branch pushed to git repo; I updated commit sha1. New commits:
0f1e6a4  21129: Add noise_multiplier check for bad reductions

comment:43 Changed 2 months ago by
Commit:  0f1e6a445e967e843d6a60c3a37a7a10c1aeaf67 → a9d40facdf33bec480d3ba0a0c51fe5567261ed1 

Branch pushed to git repo; I updated commit sha1. New commits:
a9d40fa  21129: Correct example

comment:44 Changed 2 months ago by
Commit:  a9d40facdf33bec480d3ba0a0c51fe5567261ed1 → 3287b2dc386e0d49751f8224444c4088ae20f42a 

comment:45 Changed 2 months ago by
Commit:  3287b2dc386e0d49751f8224444c4088ae20f42a → 9ce8f81a254707d3c5504fef58b329e4ec153f2f 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9ce8f81  21129: Correct nosie mult bug

comment:46 Changed 2 months ago by
Status:  needs_work → needs_review 

comment:47 Changed 2 months ago by
Status:  needs_review → positive_review 

comment:48 Changed 2 months ago by
Branch:  u/ghguojing0/pairing → 9ce8f81a254707d3c5504fef58b329e4ec153f2f 

Resolution:  → fixed 
Status:  positive_review → closed 
Branch pushed to git repo; I updated commit sha1. New commits:
Fixed an error in use of default precision of RealField