Fix belyi dessin images 7026 - #7189
Conversation
Dessin images were looked up by the shared passport label, so different Galois orbits within the same passport (e.g. 9T23- 6.2.1_6.2.1_6.2.1-a and -b) could display the same, ambiguous image. Look them up by the specific galmap label instead, and update _belyidb_to_lmfdb_plabel to decode the orbit-letter suffix. Also drop the checked-in belyi_images.txt (35MB) in favor of a gzip-compressed belyi_images.txt.gz, kept out of version control for now, with all 4733 dessin images regenerated to match.
…d b/w vertices, unique per-embedding SVG ids
There was a problem hiding this comment.
Pull request overview
This PR updates the Belyi dessin-image loading and lookup logic in the Flask Belyi pages so that dessins are keyed by galmap orbit (not just passport), and switches the repository-tracked image dump to a compressed .gz file to reduce repo size. This aligns the rendering pipeline with the data model and addresses the incorrect dessin reported in #7026.
Changes:
- Load dessin SVGs from
belyi_images.txt.gzusinggzip.openinstead of an uncompressed text file. - Include the orbit letter (defaulting to
"a") when converting BelyiDB labels to LMFDB labels, and look up images bygalmap['label']to avoid cross-orbit sharing. - Update the public helper
get_belyi_images(...)semantics/docs to reflect galmap-label lookup.
Reviewed changes
Copilot reviewed 1 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lmfdb/belyi/web_belyi.py | Switches image source to gzipped dump and changes image lookup to use per-galmap labels (including orbit letter). |
| lmfdb/static/images/belyi_images.txt.gz | Compressed, regenerated dessin SVG dump used by the Belyi web pages. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| txt_path = os.path.join(os.path.dirname(__file__), '..', 'static', 'images', 'belyi_images.txt.gz') | ||
| txt_path = os.path.abspath(txt_path) | ||
| if not os.path.exists(txt_path): | ||
| return _belyi_images | ||
| with open(txt_path, encoding='utf-8') as f: | ||
| with gzip.open(txt_path, 'rt', encoding='utf-8') as f: |
| # "4T2-[2,2,2]-22-22-22-g0" -> "4T2-2.2_2.2_2.2-a" | ||
| # "9T23-[6,6,6]-621-621-621-g1-b" -> "9T23-6.2.1_6.2.1_6.2.1-b" | ||
| # (the optional 7th, orbit-letter component distinguishes galmaps within | ||
| # a passport; passports with a single orbit omit it, defaulting to "a") |
| letter = parts[6] if len(parts) >= 7 else 'a' | ||
| return '{}-{}_{}_{}-{}'.format(group, sigma0, sigma1, sigmaoo, letter) | ||
|
|
||
|
|
||
| def get_belyi_images(plabel): | ||
| """Return list of SVG strings for the given LMFDB passport label, or [].""" | ||
| return _load_belyi_images().get(plabel, []) | ||
| def get_belyi_images(label): | ||
| """Return list of SVG strings for the given LMFDB galmap label, or [].""" | ||
| return _load_belyi_images().get(label, []) |
|
Do you want to change this line also? The new contract here is "one SVG per embedding", but the template still falls and remove the index-0 fallback, e.g.: {{ data.dessin_svgs[loop.index0]|safe }}Also, add some tests, given how small the database is, it might more worthwhile to write a check in SQL. Should we put these images in the database, instead of loading them from a textfile? |
Per edgarcosta's review on LMFDB#7189, the template's index-0 fallback for missing dessin SVGs could silently show the wrong embedding's dessin if get_belyi_images() ever returned fewer images than embeddings. Assert the counts match in web_belyi.py and index directly in the template so a mismatch fails loudly instead of misleading. Add tests: orbit-letter handling in _belyidb_to_lmfdb_plabel, that distinct orbits get distinct images (regression test for LMFDB#7026), and a DB-wide check that every galmap's image count matches its embedding count.
Extend the two dessin-image tests from single hardcoded examples to exercise all real data: the label-conversion test now round-trips every raw label in belyi_images.txt.gz against the live DB and checks the mapping is injective, and the orbit-distinctness test now checks all 82 multi-orbit passports (129 orbit pairs) instead of just one.
|
Fixed — removed the index-0 fallback in Added tests, including the whole-table check you suggested: rather than one example, it validates the image/embedding count invariant across all 1111 galmaps and checks all 82 multi-orbit passports for distinct images — cheap enough to just check everything (~5s total). |
This also regenerates all the dessin d'enfant images:
Closes #7026
Net effect: belyi_images.txt (35MB, tracked) → belyi_images.txt.gz (~8MB, tracked). See #7099 for background.
Note for whoever merges: this shrinks the live file, but the old 35MB blob stays in main's history until someone runs a history rewrite and force-pushes main — a repo-wide disruptive operation (forces everyone to re-clone or clean local history), so probably worth batching with any o rather than doing it just for this.