GradedRing as in the paper#487
Merged
edgarcosta merged 7 commits intomasterfrom Apr 14, 2026
Merged
Conversation
Rename the 2-arg GradedRing(F, N) intrinsic to HilbertModularVariety(F, N) to better reflect that it returns models of Hilbert modular varieties. The old 4-arg HilbertModularVariety(F, N, MaxGenWt, MaxRelWt) coexists (Magma overloads by arity). GradedRing(f::ModFrmHilDElt) in Elements.m is unchanged (different function, returns parent ModFrmHilDGRng). - Add Symmetric, PrecomputedGens, IdealClassesSupport optional params to the new 2-arg intrinsic, threaded through ApproximateGradedRing - Update IO.m WriteCanonicalRingComputationToString to call the new 2-arg HilbertModularVariety instead of manually looping with the old 4-arg version - Rename 12 test files GradedRing_* → Surface_* with updated calls - Add 8 new tests: Qsqrt24, Qsqrt28, Qsqrt33, Qsqrt41 (level 1), Qsqrt5_level_2, Qsqrt2_level_3, Qsqrt13_level_pp, Qsqrt13_level_2 - Add literature weight assertions (Gundlach, Hammond, van der Geer- Zagier, Hermann, Williams) to 6 existing tests - Add CPU time and memory usage reporting to all Surface_* tests - Add unit tests for building blocks: HilbertSeriesOfPresentation, AlgebraicallyIndependent, CandidateAlgIndependentElements - Add verification tests cross-checking computed vs stored equations
Three new test files: - consistency_4arg_2arg.m: verifies 4-arg and 2-arg HilbertModularVariety produce schemes with matching Hilbert series, dimension, and equation count - surface_invariants_vs_ring.m: cross-checks ArithmeticGenus via two code paths (IntersectionRing vs SurfaceInvariants), verifies Noether formula, GeometricGenus, and Irregularity for D=5,13,29 and a nontrivial level - io_canonical_ring.m: tests WriteCanonicalRingComputationToString output contains expected scheme definitions and certification status Also updates resource estimates in four existing test files based on actual timings.
…ests from CI Rename tests that exceed CI limits or hit upstream Magma bugs to use the _ prefix convention: disc 12/21/41 (>4h timeout), disc 24/28/33 (precompute.m:1064 assertion failure). Update resource comments with actual CPU times and memory usage from benchmark runs. Add Overwrite flag to IO.m Write call.
Regenerated all 8 existing equation files for narrow class number 1 discriminants and added 4 new files (D=13 levels 3.1/4.1, D=37, D=41). Updated verification test to use LMFDBField/LMFDBIdeal for correct ideal construction and eval-return pattern for proper scoping. All 12 equation verifications plus the D=13 cross-check pass. Files for D=12,21,24,28,33 (narrow class number > 1) cannot be regenerated due to upstream Magma bug at precompute.m:1064.
Added STALE warning comments to the 4 equation files (D=12,21,24,28) that have failing Hilbert series sanity checks and cannot be regenerated due to the upstream Magma bug at precompute.m:1064. Added README.md documenting the status of all files in the directory.
….1.1) HilbertSeriesVdG computes the Hilbert series for individual components of a Hilbert modular surface using per-component invariants, unlike HilbertSeriesVasquez which only works for trivial narrow class group.
abhijit-mudigonda
approved these changes
Apr 5, 2026
Collaborator
abhijit-mudigonda
left a comment
abhijit-mudigonda
left a comment
There was a problem hiding this comment.
Overall looks good to me, I do have one question about the degree to which we search for relations but I don't think it affects the correctness of the algorithm either way (it should still work for large enough B).
| PrecomputedGens := AssociativeArray()) -> Tup | ||
| {Compute generators and relations for the graded ring of Hilbert modular forms | ||
| of level N. Generators are searched up to weight B, and relations up to weight 2*B | ||
| (since relations can have degree up to the sum of any two generator weights).} |
Collaborator
There was a problem hiding this comment.
I'm probably being silly, but why is this true?
Owner
Author
There was a problem hiding this comment.
We had this written at some point in the old tex, but we are no longer sure about its correctedness.
edgarcosta
commented
Apr 13, 2026
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.
No description provided.