Clarify genus 2 minimal/simplified model snippets - #7157
Open
roed-math wants to merge 4 commits into
Open
Conversation
Rename C/X to Cmin/Csim in the genus 2 curve code snippets and add // minimal equation and // simplified equation comments, so it is clear which model each variable refers to; display Csim after defining it. Point the simplified-model rational points snippet at Csim instead of Cmin, and fix simplify_hyperelliptic_point/comp_poly to divide the y-coordinate by sqrt(n/squarefree_part(n)) rather than n, so the displayed simplified-model points (and the snippet) actually lie on the simplified model when h = 0. Mirror the renaming in the Code-to-Magma download and add a Simplified equation step to it. Verified by running the generated Magma snippets for 169.a.169.1, 400.a.409600.1 and 336.a.172032.1 (all pass except pre-existing Magma package issues), the sage snippets in a sage REPL, page renders and the Magma download via the flask test client, and the genus2_curves test suite (42 passed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mordell-Weil generators are stored with denominators cleared, as a relation d*y = yD(x,z). The y-coordinate of the minimal model transforms to the simplified model as y -> (2*y + h(x,z))/s, so multiplying through by d gives d*Y = (2*yD + d*h(x,z))/s: h picks up the same factor d. comp_poly added only a single copy of h, which was wrong for every curve having both h != 0 and a generator with a denominator (700 of the 4039 curves sampled). On 336.a.172032.1 the generator 3x^2 - 32z^2 = 0, 6y = -35xz^2 was displayed on the simplified model as 6y = x^3 - 69xz^2 rather than 6y = 6x^3 - 64xz^2, which is 2x(3x^2 - 32z^2) as it must be for a 2-torsion generator. comp_poly now takes the denominator as an argument, and mw_gens_table takes a function of it, leaving the minimal-model path untransformed. Adds regression tests for this and for the scale correction earlier in the branch: 400.a.409600.1 (h = 0, content 4, so points scale by 2 and not 4) and 336.a.172032.1, plus a check that the Magma download defines Cmin, Csim and J in dependency order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Magma refuses to apply Ogg's formula once v_2(disc) >= 12, so for those
curves the conductor snippet passes the local L-factor at 2 explicitly.
The snippet on the curve page did this, but the download built its
conductor line from the static entry in code.yaml, so downloads for
curves with 4096 | abs_disc died at Conductor(LSeries(Cmin)) with
Runtime error: v_2(disc)=14>=12, cannot use Ogg's formula
unless forced by LocalData:="Ogg"
Move the option into magma_cond_option() and have both callers use it,
with the code.yaml entry taking a placeholder the way 'curve' already
does. Verified in Magma that the downloads for 400.a.409600.1 and
336.a.172032.1 now run to completion and report the right conductor.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5344.
The genus 2 curve-page snippets defined a
C(minimal model) and anX(simplified model) without saying which was which. Following the suggestion in the issue, the snippets now useCminandCsimwith// minimal equation/// simplified equationcomments, and the simplified-model snippet displays the resulting curve. The simplified-model rational points snippet now constructs points onCsimrather than on the minimal model, and the download file gains a matching "Simplified equation" step.Along the way this fixes a bug in
simplify_hyperelliptic_point/comp_poly: the y-coordinate was divided by the content n of 4f+h² instead of sqrt(n/squarefree_part(n)). For the 3160 curves with h = 0 the displayed simplified-model points were not actually on the curve (e.g.(0 : -1/2 : 1)on 400.a.409600.1), and the corrected points are needed for the new snippet to run.Also included, since it lands on the same
condline ofcode.yaml: the Magma download failed for every curve with 4096 | abs_disc. Magma will not apply Ogg's formula once v_2(disc) >= 12, so the conductor snippet has to pass it the local L-factor at 2 viaExcFactors. The curve page did that, but the download built its conductor line from the staticcondentry incode.yaml, which had no such option, so downloads for those curves died withThe option now comes from a single
magma_cond_option()used by both the page snippet and the download, with thecode.yamlentry taking a placeholder the waycurvealready does, so the two cannot drift apart again. This bug is pre-existing and independent of the renaming above.Verified by running the generated Magma/Sage snippets for 169.a.169.1, 400.a.409600.1 and 336.a.172032.1, running the full regenerated Magma downloads for 400.a.409600.1 and 336.a.172032.1 end to end (both now complete and report conductors and discriminants matching their labels), rendering the affected pages and the Magma download with the test client, and the genus2_curves test suite (46 passed).
Note that Magma prints
WARNING: ExcFactors and BadPrimes are obsolete, use LocalDatafor the affected curves. That warning is pre-existing on the curve page, and switching to the modernLocalDataspelling is left for a follow-up rather than changing page behavior here.Ported from roed-math#21, where the full write-up and comment history live.
🤖 Generated with Claude Code