Opened 7 years ago
Closed 4 months ago
#21129 closed enhancement (fixed)
Arakelov-Zhang pairing of rational maps
Reported by: | Paul Fili | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-9.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 Arakelov-Zhang pairing of two rational maps defined over number fields.
Change History (48)
comment:1 Changed 7 years ago by
Branch: | → u/paulfili/arakelov_zhang_pairing_of_rational_maps |
---|
comment:2 Changed 7 years ago by
Cc: | Holly Krieger added |
---|---|
Commit: | → a6cca7c9827e746d49cc0d1bbd44eb3bc9fab2e2 |
Status: | new → needs_review |
comment:3 Changed 7 years ago by
Commit: | a6cca7c9827e746d49cc0d1bbd44eb3bc9fab2e2 → b3ea8584f52d9355629c4f82f6495a5aff3e742a |
---|
comment:4 Changed 7 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 python3-compatible syntax for print (see https://wiki.sagemath.org/Python3-compatible%20code)
comment:7 Changed 6 years ago by
Branch: | u/paulfili/arakelov_zhang_pairing_of_rational_maps → public/arakelov_zhang |
---|---|
Commit: | ec5eac2de69f479e86d087b38d7b6bb95b0fe4b9 → 6ab8e9166ab907d94cbfd3b7be4612828587d26a |
comment:8 Changed 6 years ago by
Milestone: | sage-7.4 → sage-8.1 |
---|---|
Summary: | arakelov-zhang pairing of rational maps → Arakelov-Zhang pairing of rational maps |
comment:9 Changed 6 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/gh-yuankmath/AZpairing |
---|---|
Cc: | Yuan Kong added |
Commit: | 6ab8e9166ab907d94cbfd3b7be4612828587d26a → 357a0a1829c2045be996da2c58d14b4386a8fecb |
Milestone: | sage-8.1 → sage-9.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 follow-up: 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 re-written 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/gh-yuankmath/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 re-written 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: | sage-9.0 → sage-9.1 |
---|
Ticket retargeted after milestone closed
comment:20 Changed 3 years ago by
Milestone: | sage-9.1 → sage-9.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: | sage-9.2 → sage-9.3 |
---|
comment:22 Changed 23 months ago by
Milestone: | sage-9.3 → sage-9.4 |
---|
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
comment:23 Changed 19 months ago by
Milestone: | sage-9.4 → sage-9.5 |
---|
Setting a new milestone for this ticket based on a cursory review.
comment:24 Changed 14 months ago by
Milestone: | sage-9.5 → sage-9.6 |
---|
Stalled in needs_review
or needs_info
; likely won't make it into Sage 9.5.
comment:25 Changed 10 months ago by
Milestone: | sage-9.6 → sage-9.7 |
---|
comment:26 Changed 5 months ago by
Authors: | Paul Fili, Holly Krieger → Paul Fili, Holly Krieger, Jing Guo |
---|---|
Branch: | u/paulfili/AZpairing → u/gh-guojing0/pairing |
Cc: | Ben Hutz Alexander Galarraga added |
Dependencies: | 28779 → #28779 |
Keywords: | gsoc2022 added |
Status: | needs_review → needs_work |
comment:27 Changed 5 months ago by
Commit: | 2a8b393878cdf2162816f502a681d0aa944cfb94 → be35eb0bd582b35f767a5e7caf91421be5271d57 |
---|
comment:28 Changed 5 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 5 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 5 months ago by
Commit: | f3e67ab931a0db50c1694250ed9eac5996a7943f → 9b43de16b094a12b6873f08ab6802647719adecb |
---|
comment:31 Changed 5 months ago by
Commit: | 9b43de16b094a12b6873f08ab6802647719adecb → c83217c488eebc39f19075762a0baa316d60698e |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
c83217c | 21129: Indent
|
comment:32 Changed 5 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 5 months ago by
Commit: | 174b6864b59680453f65c22e91818dd1058e1f09 → dcce8071104d83582fb9a74699c6e150f8872016 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
dcce807 | 21129: format examples
|
comment:34 Changed 5 months ago by
Status: | needs_work → needs_review |
---|
comment:35 Changed 4 months ago by
Milestone: | sage-9.7 → sage-9.8 |
---|
comment:36 Changed 4 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 4 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 4 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 4 months ago by
Status: | needs_work → needs_review |
---|
Thank you, Alex. Good catch. That was caused by calling abs
before norm
for non-rational f_disc
. Everything is fixed now.
comment:40 Changed 4 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. 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 4 months ago by
Commit: | ecf50141497051ed31083820cbd11c9710c8e2fd → 22c177847d061cb6c183660cc07cf900a38026e2 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
22c1778 | 21129: Better indentation
|
comment:42 Changed 4 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 4 months ago by
Commit: | 0f1e6a445e967e843d6a60c3a37a7a10c1aeaf67 → a9d40facdf33bec480d3ba0a0c51fe5567261ed1 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
a9d40fa | 21129: Correct example
|
comment:44 Changed 4 months ago by
Commit: | a9d40facdf33bec480d3ba0a0c51fe5567261ed1 → 3287b2dc386e0d49751f8224444c4088ae20f42a |
---|
comment:45 Changed 4 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 4 months ago by
Status: | needs_work → needs_review |
---|
comment:47 Changed 4 months ago by
Status: | needs_review → positive_review |
---|
comment:48 Changed 4 months ago by
Branch: | u/gh-guojing0/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