fix GAP field-element encoding over extension fields + other minor fixes - #576
Merged
Conversation
…work I/O The GAP bridge assumed GAP's Int()/f*One(F) integer encoding of finite-field elements matches galois's, which holds only over prime fields. Over GF(p^k) with k>1 this silently corrupted matrices written to GAP (wrong get_distance_bound) and crashed matrices read back (IntFFE error). Encode/decode each element via its discrete logarithm base the shared primitive root Z(q) instead (Z(q)^log on write, LogFFE on read, rebuilt as primitive_element**log), which is byte-identical over prime fields and correct over extension fields. Also: treat a nonzero GAP exit code as an error (not only nonempty stderr); drop a duplicate unguarded pyperclip.copy that defeated the headless copy/paste fallback; import urllib submodules explicitly; add urlopen timeouts and catch fetch failures in the GroupNames lookups; guard get_small_group_number on an empty match set; and skip blank lines in parse_gap_permutations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ache bypass Disclose the surprising side effects (blocking input(), subprocess, clipboard, git clone, live HTTP fetch) and Raises of the GAP/GroupNames/qecdb entry points; note get_distance_bound returns a randomized upper bound and document its maxav argument; explain why get_small_group_structure caches manually (so the GAP-absent fallback is never cached); note the pytest bypass of get_disk_cache/use_disk_cache; and link the CAS/GAP-package/web references (GAP, GUAVA, Wedderga, SmallGrp, GroupNames, qecdb). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Catch TimeoutError alongside URLError in get_quantum_code so a socket
read-timeout during urlopen becomes the intended "Cannot access {url}"
RuntimeError, matching the sibling network functions in groups.py.
In get_classical_code, only parse lines that begin with "[" as check-matrix
logarithm rows. Any other stray GAP output line is skipped, so an unexpected
diagnostic line falls through to the descriptive "Could not determine the base
field" / "Code has no parity checks" errors instead of an opaque SyntaxError
from ast.literal_eval.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rewrite the primitive-element comment in _gap_define_sparse_matrix to stand on its own, rename the generated GAP list `elts` to `elements`, and give the "Runs GAP in a subprocess" docstring note an explicit subject in get_classical_code, get_distance_bound, and get_output. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The prompt-and-git-clone behavior of a missing GAP package is documented on require_package; get_classical_code and get_distance_bound only need to state the package requirement, not restate how it is installed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The module summary already states the code comes from GAP, so noting the subprocess mechanism adds no caller-actionable information; keep only the package requirements and the randomized-upper-bound semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Say that the function runs the commands and falls back to the manual workflow, rather than leaving the subject of "runs" and "falls back" implied. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A nonzero exit code with empty stderr previously left a stray blank line in the error message; build the message from parts so the stderr section appears only when there is stderr to show. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace dangling "this" and subject-less "Uses/Tries/Fetches ..." sentences in the added docstrings and comments with an explicit subject (this function, this decorator, the workflow), so each reads on its own. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The pytest-only behavior of get_disk_cache and use_disk_cache is an internal testing detail that callers do not act on; leave the summary lines to speak for themselves. Co-Authored-By: Claude Opus 4.8 (1M context) <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
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.
What & why
get_classical_codeand_gap_define_sparse_matrixassumed GAP's integer encoding of finite-field elements (Int(x)/f*One(F)) matches the onegaloisuses. That holds only over prime fields. OverGF(p^k)withk > 1,f*One(F)collapses to(f mod p)*One(F), so matrices sent to GAP were silently corrupted (get_distance_boundreturned a wrong distance), andInt(x)raisedIntFFE: <z> must lie in prime fieldwhen reading extension-field elements back.Each element is now mapped through the discrete logarithm of the field's primitive root instead:
Z(q)^logwhen writing to GAP,LogFFE(x, z)when reading back (rebuilt in Python asprimitive_element ** log). GAP andgaloisshare that primitive element (both use Conway polynomials for the field orders that arise here), so the encoding is byte-identical over prime fields and correct over extension fields.Verified against live GAP: a
GF(4)hypergraph-product code with true Z-distance 2 previously received a bound of 1 (impossible for a correct upper bound) and now receives 2; reading aGF(4)GUAVA code no longer raises.Other fixes
get_outputnow treats a nonzero GAP exit code as an error, not only nonempty stderr.pyperclip.copythat crashed the manual copy/paste fallback on headless machines instead of printing instructions.codes.pyimportsurllib.request/urllib.errorexplicitly rather than relying on another module to import them first.urlopentimeout and catch connection failures, so an unreachable server degrades gracefully instead of hanging or raising uncaught.get_quantum_codecatchesTimeoutErroralongsideurllib.error.URLError, so a socket read-timeout during the qecdb fetch becomes the intendedCannot access {url}error instead of propagating raw, matching the GroupNames lookups.get_classical_codeparses only lines that begin with[as check-matrix rows, so a stray GAP diagnostic line falls through to the descriptive "Could not determine the base field" / "Code has no parity checks" errors instead of an opaqueSyntaxErrorfromast.literal_eval.get_small_group_numberraises a clear error instead ofValueError: max() arg is an empty sequencewhen the index page has no matching entries.parse_gap_permutationsskips blank lines instead of emitting a spurious identity generator.Documentation
Documented the modules' side effects (subprocess, network fetch, clipboard write,
git clone, blocking stdin) and the randomized-upper-bound semantics ofget_distance_bound(and itsmaxavargument); linked the GAP, GUAVA, Wedderga, SmallGrp, GroupNames, and qecdb references.Testing
Passes the project checks (
./checks/all_.py: mypy, ruff lint + format, 100% coverage). The extension-field encode/decode round trip was additionally checked against a live GAP + QDistRnd install.🤖 Generated with Claude Code