Skip to content

Guard PARI polgalois on unsupported number-field degrees (LMFDB#6759) - #32

Closed
roed-math wants to merge 3 commits into
mainfrom
ai/t41-nf-snippet-errors
Closed

Guard PARI polgalois on unsupported number-field degrees (LMFDB#6759)#32
roed-math wants to merge 3 commits into
mainfrom
ai/t41-nf-snippet-errors

Conversation

@roed-math

@roed-math roed-math commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Some auto-generated code snippets on number-field pages error when run (LMFDB#6759). The snippet CI harness cannot catch them because it only exercises degree <= 2 fields, so I ran every number-field snippet across a spread of fields (Q, imaginary/real quadratic, monogenic and non-monogenic cubics, a quartic, and a degree-12 field). That surfaced three genuine bugs; two of them, the Magma Galois-group assignment (= instead of :=) and Magma's collapse of a degree-1 polynomial to the rationals on the Q page, have since been fixed on main by LMFDB#7079, which represents Q as RationalsAsNumberField(). Those changes are dropped here, leaving a single rational-field strategy, and this PR now carries only the third fix.

PARI's polgalois hard-errors above degree 11 ("sorry, galois of degree higher than 11 is not yet implemented"), and it is unconditional only through degree 7: degrees 8 to 11 need the optional galdata package, which is not part of a stock PARI installation. The galois_group snippet in lmfdb/number_fields/code.yaml is now guarded so that either case returns an explanatory string instead of aborting the rest of the snippet file, while any other polgalois failure still propagates.

Verified in PARI 2.17.2 (via sage --gp) for degrees 2, 8 and 12, both with galdata present and with datadir pointed at a directory without it. The two committed gp snippet-test logs are updated for the new Galois line; their computed output is unchanged, and re-running the Magma snippet harness reports no change to the committed Magma logs. Two regression tests are added in lmfdb/number_fields/test_numberfield.py: one checks that the degree-12 gp download is guarded and never emits a bare polgalois(K.pol) line, the other pins the Magma representation of Q to RationalsAsNumberField() in both the field definition and the self-contained class-number-formula snippet.

One issue is intentionally left as a documented limitation: the PARI analytic-class-number-formula snippet overflows the default PARI stack on high-degree fields (a resource limit, not a code error), so no allocatemem directive is injected into a user-facing snippet.

Addresses LMFDB#6759.

🤖 Generated with Claude Code

roed314 and others added 3 commits July 19, 2026 12:23
Running the number field snippets across a spread of fields (Q, quadratic,
cubic, quartic, degree 12) surfaced three genuine errors that the snippet CI
harness cannot catch, since it only tests degree <= 2 fields and skips Magma:

- The Magma galois_group snippet used `=` (equality) instead of `:=`
  (assignment), erroring on every field.
- Magma's NumberField collapses a degree-1 polynomial to the rationals, so
  the Q field page (1.1.1.1) broke on unit rank, regulator, subfields and the
  class number formula; fixed with `DoLinearExtension := true` in the field
  and class-number-formula snippets.
- PARI's polgalois hard-errors above degree 11; now guarded with an if() that
  returns an explanatory message (degree <= 11 is unchanged).

Updates the two committed gp snippet-test logs for the new PARI Galois command
line (its output is unchanged). Verified by executing the fixed snippets in
sage, sage --gp and Magma across the field spread.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t-errors

# Conflicts:
#	lmfdb/number_fields/code.yaml
)

The Magma Galois assignment and rational-field fixes carried by this branch
were superseded by LMFDB#7079, which represents Q as
RationalsAsNumberField().  Drop the DoLinearExtension templates so there is a
single rational-field strategy: with them in place the exact
replace("NumberField(x)", "RationalsAsNumberField()") in
WebNumberField.make_code_snippets() silently became dead code, and the
committed Magma logs no longer matched the templates.

Extend the PARI guard to the real support boundary.  polgalois is
unconditional only through degree 7; degrees 8 to 11 need the optional galdata
package, and above 11 it hard-errors.  Both cases now yield an explanatory
string, while any other polgalois failure still propagates.

Verified in PARI 2.17.2 via sage --gp for degrees 2, 8 and 12, with galdata
present and with datadir pointed at a directory without it.  The Magma snippet
harness reports no change to the committed Magma logs; the two gp logs differ
only in the echoed command line, with identical computed output.  Adds
regression tests for the guarded degree-12 gp download and for the Magma
representation of Q.  sage -python -m pytest
lmfdb/number_fields/test_numberfield.py -> 36 passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@roed-math roed-math changed the title Fix number-field code-snippet errors: Magma Galois assignment & Q-collapse, PARI high-degree Galois (LMFDB#6759) Guard PARI polgalois on unsupported number-field degrees (LMFDB#6759) Aug 5, 2026
@roed-math

Copy link
Copy Markdown
Owner Author

Updated in f6b304f to address the review. Summary of what changed and how it was checked.

1. Dropped the duplicate Magma rational-field implementation. Both DoLinearExtension := true templates are gone, so main's RationalsAsNumberField() handling (from LMFDB#7079) is again the single strategy. This was the real bug in the merge resolution: with NumberField(%s : DoLinearExtension := true) in place, the exact-string replace("NumberField(x)", "RationalsAsNumberField()") in WebNumberField.make_code_snippets() never matched, so it was dead code while the committed Magma fixtures still contained RationalsAsNumberField(). code.yaml now differs from main in exactly one line.

Re-running the Magma harness against the committed fixtures confirms no drift:

$ PATH=/Applications/Magma:$PATH sage -python lmfdb/tests/generate_snippet_tests.py test -o magma -f lmfdb/number_fields/code.yaml
No change in lmfdb/tests/snippet_tests/number_fields/code-1.1.1.1-magma.log
No change in lmfdb/tests/snippet_tests/number_fields/code-2.0.4.1-magma.log

2. Widened the PARI guard to the documented support boundary. polgalois is unconditional only through degree 7; degrees 8 to 11 need the optional galdata package. The snippet is now

if(poldegree(K.pol) > 11, "polgalois only supports degree <= 11", iferr(polgalois(K.pol), E, "polgalois in degrees 8 to 11 requires the optional galdata package", errname(E) == "e_FILE" || errname(E) == "e_PACKAGE"))

One deviation from the suggested expression, and it is deliberate: the review proposed the predicate errname(E) == "e_PACKAGE", but that is not the error PARI actually raises here. Verified against PARI 2.17.2 (sage --gp) by pointing datadir at a directory with no galdata/:

case result
degree 2, galdata present [2, -1, 1, "S2"]
degree 8, galdata present [8, 1, 2, "4[x]2"]
degree 8, galdata absent polgalois in degrees 8 to 11 requires the optional galdata package
degree 12 polgalois only supports degree <= 11
polgalois(y^2 - 1) (unrelated failure) propagates: not an irreducible polynomial in galois

With galdata removed, iferr(polgalois(K.pol), E, errname(E), 1) returns e_FILE ("error opening galois file: .../galdata/COS8_49_45"), not e_PACKAGE. Both names are accepted so the guard also works on builds that raise e_PACKAGE, and the predicate stays narrow enough that any other polgalois failure still propagates, as the review asked.

3. Regenerated the gp fixtures. Only the echoed command line differs; the computed outputs are byte-identical, so the mathematical content of the two logs is unchanged.

$ sage -python lmfdb/tests/generate_snippet_tests.py test -o pari -f lmfdb/number_fields/code.yaml

Note for anyone reproducing: the harness intersects the -o set with the language keys in code.yaml before mapping pari to gp, so -o gp prints "No valid languages selected" and -o pari is the working invocation. The remaining diff the harness reports on the regenerated copies is the pre-existing nondeterminism in bnfinit internals and one lfunrootres float format, on lines 3 and 30, i.e. output of commands this PR does not touch.

4. Added the two regression tests in lmfdb/number_fields/test_numberfield.py:

  • test_high_degree_gp_galois_download fetches /NumberField/12.4.320979616137216.3/download/gp and asserts the guard, the explanatory text for both the degree and the galdata case, and that no line is a bare polgalois(K.pol). The degree-12 field is deliberately not added to the snippet_test matrix, since the full gp download also runs the expensive analytic-class-number-formula snippet.
  • test_rational_field_magma_download fetches /NumberField/1.1.1.1/download/magma and pins RationalsAsNumberField() in both the field definition and the self-contained class-number-formula block, with DoLinearExtension and NumberField(x) absent. No Magma executable needed.
$ sage -python -m pytest lmfdb/number_fields/test_numberfield.py
36 passed, 58 warnings in 1052.79s

Lint is clean (pyflakes, pylint -e W0129,W0108, ruff check --preview --select=E722).

5. Title and body now describe the final diff: the Magma assignment and Q-collapse fixes are attributed to LMFDB#7079 and this PR is just the PARI guard plus its tests. The high-degree lfunrootres stack overflow is retained as a documented limitation.

@roed314

roed314 commented Aug 5, 2026

Copy link
Copy Markdown

GPT signed off.

@roed-math

Copy link
Copy Markdown
Owner Author

Superseded by LMFDB#7168, opened upstream from this same branch. Closing here; review continues upstream.

@roed-math roed-math closed this Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants