Skip to content

Make promote/lift and ring maps honest about canonical homomorphisms#62

Draft
d-torrance wants to merge 9 commits into
MichaelABurr:aringsfrom
d-torrance:ring-maps
Draft

Make promote/lift and ring maps honest about canonical homomorphisms#62
d-torrance wants to merge 9 commits into
MichaelABurr:aringsfrom
d-torrance:ring-maps

Conversation

@d-torrance

@d-torrance d-torrance commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Claude and I worked on some pretty big refactoring after our discussions on Friday. Claude-written summary below.

Draft for now to test the builds.

Edit: The set_from_* -> set commits were split off into #63.

Summary

This branch makes Macaulay2's promote, lift, and map(Ring, Ring, ...)
machinery agree with the actual mathematics: a conversion between two basic
rings is only allowed when a canonical ring homomorphism exists. Previously
several structurally invalid conversions silently succeeded (e.g.
promote(2/1, ZZ[x]) or map(ZZ, QQ)), and isPromotable reported true
for maps that don't exist.

Along the way it cleans up the engine-side ARing conversion API
(set_from_*set, get_from_*try_set), replaces two large hand-
written promote/lift switch statements with compiler-checked std::variant
dispatch (fixing three latent lift bugs in the process), and fixes a set of
regressions over the ARingGFM2 Galois-field backend that the rename exposed.

Motivation

The canonical ring-map DAG for Macaulay2's basic rings is

ZZ ──▶ QQ ──▶ RR ──▶ CC
 │            │       │
 │            ▼       ▼
 │           RRi ──▶ CCi
 ▼
Fp ──▶ GF(p^n)

Nothing enforced this. isPromotable and promote would accept conversions
that run against the arrows (QQ → ZZ, QQ → Fp, RR → QQ), and a global
promote(QQ, RingElement) fallback would promote a rational element-wise into
any ring whenever the denominator happened to be a unit there. As a result,
ring maps could be constructed that have no underlying homomorphism, and
predicates that callers rely on returned wrong answers.

Key changes

Canonical-map predicate (engine)

  • New basic-rings/ring-structure.{hpp,cpp} introduces a RingStructure
    enum that collapses precision and backend (RR/RRR → RealField, all ZZp
    backends → PrimeField, etc.) and a structurePrecedes partial order
    computed as the reflexive-transitive closure of the canonical-map DAG.
  • hasCanonicalMap(R, S) is the single authoritative predicate: it combines
    the structural order with characteristic matching for finite-field families
    and GF(p^m) → GF(p^n) dimension divisibility (m | n).
  • gfDimension reaches the concrete ARing via static_cast keyed on
    ringID(), deliberately avoiding a new virtual on Ring (which would
    change the vtable layout for every Ring subclass). ARingGFM2 gains a
    dimension() accessor to match ARingGFFlint{,Big}.
  • ConcreteRing::promote/lift now reject structurally invalid conversions
    early via hasCanonicalMap, before reaching the ring-pair switch.

promote/lift/isPromotable honesty (interpreter)

  • Removed the global promote(QQ, RingElement) element-wise fallback. The
    per-ring promote(QQ, R) registrations in commonEngineRingInitializations
    already cover every ring QQ genuinely embeds into, so isPromotable is
    now honest.
  • map(Ring, Ring, Matrix) now errors at construction time when it reaches a
    basic coefficient ring A with no canonical map A → R, instead of
    silently producing an invalid map.
  • isPromotable(InexactField, Ring) looks up by ring family (class 0_R)
    rather than the specific precision ring, fixing map(R[x], R) for
    RR/CC/RRi/CCi.
  • isPromotable(QuotientRing, Ring) and an in-constructor
    promote(S, RingElement) := basicPromote fix the dispatch mismatch for
    cross-backend ZZp rings (elements dispatch on the ring, Ring-typed
    methods on QuotientRing).

ARing conversion API cleanup (engine)

  • set_from_long, set_from_mpz, set_from_mpq, set_from_double,
    set_from_doubles, set_from_BigReal(s), set_from_BigComplex,
    set_from_Interval, set_from_ComplexInterval, and set_from_int are all
    renamed to overloads of set() — the source type is already encoded in the
    argument. Duplicate set_from_complex_double/set_from_complex_mpfr
    methods are removed. Added set(ElementType&, int) overloads where long
    and double overloads would otherwise make an int argument ambiguous.
  • The six free dispatch helpers get_from_* are renamed to try_set (they
    attempt R.set(elem, val) and report whether the ring supports it). The
    has_set_from_* concept detectors keep their from_ infix.
  • ConcreteRing::promote/lift replace a ~130-line and a ~100-line nested
    switch with a RingVariant + toVariant helper and two-argument
    std::visit. Adding a new ring type now requires only extending
    RingVariant/toVariant; the compiler enforces coverage. This refactor
    fixed three latent lift bugs automatically: missing ring_RRi/ring_CCi
    source arms, and swapped CC ↔ CCC template arguments.

ARingGFM2 regression fixes

ARingGFM2 stores elements as table indices, not integers, so after the
rename ring().set(result, 1) resolved to the element-copy overload and wrote
a raw index instead of converting 1 through the GF table. Fix: drop
set(elem&, elem) from ARingGFM2 so set() always means "convert from
integer", and replace element-copy uses of ring().set(...) with
ring().copy(...) across the mutable-matrix and ring infrastructure
(dmat-lu*, mat-elem-ops, mat-arith, mat-util, smat, aring-glue).
This restores mutableMatrix, polynomial coefficient extraction, and det
over GF(2^n) rings built with the "New" ARing strategy.

Tests

  • New unit-tests/RingStructureTest.cpp covers structureOf,
    structurePrecedes reflexivity, valid promote paths through both the
    char-0 tower and the finite-field family, and invalid maps
    (QQ ↛ ZZ, QQ ↛ Fp, RR ↛ QQ, …).
  • Existing ARing unit tests are updated for the set()/try_set rename.

Notes

This is purely an engine + interpreter change; no documentation rebuild is
required beyond removing the now-deleted (promote, QQ, RingElement) from the
undocumented list.

d-torrance and others added 7 commits June 22, 2026 16:28
ARingGFFlint and ARingGFFlintBig already expose dimension() directly;
ARingGFM2 delegates to its internal GF object.  Needed for the
canonical-map predicate that checks GF(p^m) → GF(p^n) divisibility.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Encodes the canonical ring-map DAG (ZZ→QQ→RR→CC, ZZ→Fp→GF) as an
enum and BFS-based reflexive-transitive closure.  hasCanonicalMap
combines structural precedence with characteristic matching and
GF(p^m)→GF(p^n) dimension divisibility into a single authoritative
predicate.  gfDimension() accesses the concrete ARing type via
static_cast keyed on ringID(), avoiding a new virtual function on Ring
(which would change the vtable layout for all Ring subclasses).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rejects structurally invalid conversions (e.g. QQ→ZZ, QQ→Fp) early
via the new hasCanonicalMap predicate, before reaching the ring-pair
switch.  lift shares the same guard because it is the partial inverse
of the same canonical map.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tests structureOf (RingID→RingStructure mapping), structurePrecedes
reflexivity, valid promote paths through the char-0 tower and
finite-field family, and invalid maps (QQ↛ZZ, QQ↛Fp, RR↛QQ, etc.).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
In map(Ring, Ring, Matrix), when the while loop reaches a basic
coefficient ring A (numgens A == 0), check isPromotable(A, R)
before proceeding. This catches cases like map(ZZ, QQ) at
construction time rather than silently producing an invalid map.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This method tried to promote rational numbers element-wise (numerator
divided by denominator) to arbitrary rings, succeeding whenever the
denominator happened to be a unit. This made promote(2/1, ZZ[x]) work
even though there is no ring homomorphism QQ -> ZZ[x], and caused
isPromotable(QQ, ZZ[x]) to return true.

Per-ring initialization in commonEngineRingInitializations already
registers promote(QQ, R) for every ring R that QQ genuinely embeds
into (QQ[x], RR, CC, etc. via their baseRings). Removing the global
fallback makes isPromotable honest.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the ~130-line nested switch in ConcreteRing::promote and the
~100-line nested switch in ConcreteRing::lift with a RingVariant +
toVariant helper and a two-argument std::visit lambda that calls
mypromote/mylift directly on the unwrapped concrete ring references.

The old lift switch had three latent bugs now fixed automatically:
- ring_RRi and ring_CCi source arms were missing (those mylift
  specializations in aring-translate.hpp were unreachable)
- The ring_CC→ring_CCC and ring_CCC→ring_CC lift template args
  were swapped

Structural drift is now impossible: adding a new ring type requires only
adding it to RingVariant and toVariant; the compiler enforces coverage.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@d-torrance

Copy link
Copy Markdown
Collaborator Author

Hmm, this breaks the reducedRowEchelonForm documentation, which sub's a matrix over QQ into ZZ/5.

It's easy to fix the documentation node -- just build the matrix from scratch over ZZ/5 rather than calling sub -- but it brings up the broader question -- should sub work when there's no homomorphism?

@d-torrance

Copy link
Copy Markdown
Collaborator Author

Or maybe rather than calling map, sub could first lift to some common ancestor and then promote?

d-torrance and others added 2 commits June 23, 2026 20:26
promote(RR, R[x]) is installed using class(0_(RR_53)) = RR as the key, so
isPromotable(RR_53, R[x]) must look up by the ring family, not the specific
precision ring. Fixes map(R[x], R) for RR, CC, RRi, CCi and their variants.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Install promote(S, RingElement) := basicPromote inside the ZZp
constructor so that element-level dispatch finds it (elements dispatch
on class(x) = the ring, while Ring-typed methods dispatch on
class(ring) = QuotientRing).

Add isPromotable(QuotientRing, Ring) outside the constructor: for
finite prime fields it checks char equality; for other QuotientRings
it falls back to lookup(promote, R, S).  This fixes the dispatch
mismatch that caused isPromotable(ZZp 101, ZZp(101, Strategy => "Aring"))
to return false.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

1 participant