Allow more Core tests to pass via "capture"#4359
Conversation
…ures ext-total.m2 sets flagInhomogeneity = true (a debugging flag that turns inhomogeneous GB computations into errors) but never resets it. When tests run via capture in the same process, this global mutation persists: any subsequent test that triggers a GB computation on an inhomogeneous matrix (e.g. galois1.m2, which calls GF()) would fail with "internal error: gb: inhomogeneous matrix flagged" — even though the test is correct. The fix is to reset the flag at the end of ext-total.m2. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… mismatch
Root cause: capture() replaces User#"private dictionary" with a fresh empty
one and builds a dictionaryPath that omits the old dictionary. Symbols such
as T (used by degreesMonoid for degrees-ring generators) and x (used for toric
variety ring generators) were created via getSymbol at M2 startup or package
load time and therefore live in the OLD private dictionary. When a captured
test encounters T_0, x_1, etc., the parser cannot find the parent symbol in
the new dictionaryPath and creates a fresh symbol there instead. Subsequent
calls to "use ring" correctly update the IndexedVariableTable of the OLD symbol,
but the captured code reads the NEW symbol (empty table) and gets IndexedVariable
objects rather than ring elements. A comment in examples.m2 acknowledges the
T case explicitly ("necessary mainly due to T from degreesMonoid"); the
workaround there was removed for an unrelated reason.
Two manifestations, two fixes applied:
hilbertFunction.m2, hilbertSeries.m2, multidegree.m2, schenck-book-7.m2:
Replace bare T_0/T_1/x_i references with explicit ring subscripts
(e.g. HR_0, DR_0, A_0, C_0) using Ring _ ZZ, bypassing symbol lookup.
"use ring" lines that existed only to bring T_0/T_1 into scope are dropped.
poincare2.m2:
r uses degreesRing{1,1} (List-memoized) and R uses degreesRing 2
(ZZ-memoized); in capture these may be memoized with different T symbols,
so map(ring poincare r, ring poincare R) matches nothing and sends all
generators to 0. Fix: pass explicit generator images so the map does not
rely on name-based symbol matching.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| t = tally degrees target presentation E | ||
| assert ( t === u ) | ||
|
|
||
| flagInhomogeneity = false |
There was a problem hiding this comment.
flagInhomogeneity should probably be added to the no-capture regex list.
There was a problem hiding this comment.
It's not exported, so I can't imagine too many tests would use it
There was a problem hiding this comment.
Oh wait I just saw there's debug Core at the top of these file, shouldn't debug automatically disable capture?
There was a problem hiding this comment.
Hmm I guess the side effects of debug itself are usually reverted by other stuff that capture does, but also debug Core enables a whole bunch of shenanigans we don't look for ... maybe debug Core should specifically force not using capture? idk!
| assert( degree x === degree X ) | ||
| assert( degree y === degree Y ) | ||
| f = map(ring poincare r, ring poincare R) | ||
| f = map(ring poincare r, ring poincare R, generators ring poincare r) |
There was a problem hiding this comment.
Why did this fail without specifying the generators?
There was a problem hiding this comment.
Great question! Here's the test failure:
-- i1 : -- test source: ../tests/normal/poincare2.m2:1:1-50:7
-- r = QQ[x,y,Degrees => {{2,1},{1,3}}, Heft => {1,1}]
--
-- o1 = r
--
-- o1 : PolynomialRing
--
-- i2 : R = QQ[X,Y,Degrees => {{2,1},{1,3}}]
--
-- o2 = R
--
-- o2 : PolynomialRing
--
-- i3 : assert( degrees source vars R === degrees source vars r )
--
-- i4 : assert( degrees R^{{2,3},{4,1},{1,0},{0,1}} === degrees r^{{2,3},{4,1},{1,0},{0,1}} )
--
-- i5 : assert( degree x === degree X )
--
-- i6 : assert( degree y === degree Y )
--
-- i7 : f = map(ring poincare r, ring poincare R)
--
-- o7 = map (ZZ[T ..T ], ZZ[T ..T ], {0, 0})
-- 0 1 0 1
--
-- o7 : RingMap ZZ[T ..T ] <-- ZZ[T ..T ]
-- 0 1 0 1
--
-- i8 : assert( poincare r === f poincare R )
--
-- i9 : assert( poincare (r/(x)) === f poincare (R/(X)) )
-- currentString:10:6:(3):[14]: error: assertion failedI'm guessing that map(Ring, Ring) runs into some shenanigans with the symbol T.
|
I agree that replacing |
Good point. After these changes, |
|
Sorry, let me rephrase: it's worth thinking whether there's a fix in Core (involving use/global symbols/etc) we could make to fix this capture errors. (I mentioned no-capture-flag earlier because to me every no-capture-flag indicates a bug that should be fixed in the future) added: these may be very very edge cases, but I'm sure some users come upon them and don't know what's going on. |
|
Good point. I think the underlying bug for most of these might be #2232. |
2 participants
When running
check Core, each test is first attempted viacapture(in-process) before falling back to an external M2 subprocess. This PR fixes several tests that were failing the capture step unnecessarily, due to two distinct root causes:ext-total.m2set a global debugging flag and never reset it, causing subsequent tests in the same process to fail spuriously.use ringto install named ring generators as global variables, which does not work correctly in thecapturecontext due to how it manages symbol dictionaries. These tests have been rewritten to reference ring generators directly.After this, all
Coretests should pass on the first try, i.e., no more "capture failed" messages.AI Disclosure
Claude Code did all the hard work. I just copy-pasted error messages and it took it from there.