Repository navigation
A Laplace determinant came back with another expression's integers in it, once, under a parallel test run #1219
Description
Activity
Second sighting, and it narrows the cause. Same test, same signature — a Laplace determinant coming back with an enormous integer in it that has nothing to do with the matrix — on a different branch and a different day:
[[-1, -4, -3, b], [4, -1, 1, -1], [-3, -1, 2, a], [1, 4, -1, b]]: elimination: 68 * a + 104 * b - 44 Laplace: -(-(2 * b + -a * (-1)) + -(-b + -a * 4) + 41364133297468660860292422696597218…The new information is which matrix failed. The generator is seeded with
Random(4242), so all 300 matrices are byte-identical between runs — and the one that failed this time is not the one that failed the first time ([[c, -2, -2, -3], [0, a, 0, c], [2, 4, -2, 1], [2, 0, 0, a]]). Same inputs, different victim.That rules out the whole family of explanations where something about a particular matrix triggers it — a degenerate pivot, a coefficient that overflows, an unlucky evaluation point. What varies between the two runs is only what else was executing beside it. It is a race, and the corrupted value arrives from another test's arithmetic.
Both runs also agree on the shape of the corruption: the elimination side is right, and it is the Laplace side that acquires the foreign integer, in a cofactor position, leaving the rest of the expression structurally intact. So whatever is shared is reached from
DeterminantLaplace's path and not from the elimination's.Reproduction is unchanged — it does not appear on demand. In this instance it failed once in the
FullyQualifiedName!~Calculuschunk, then passed on an immediate re-run of the identical binary (8,471 passing), and passes in isolation (27 passing). Three sightings would give the third data point on whether the victim is uniformly distributed; I am not going to hunt it by repetition, but I will keep recording them here when they land.Audited the shared mutable state on the evaluation path. Three real races found and fixed; none of them explains this issue, and I want to be explicit about that rather than let the fixes look like a resolution.
First, this is user-facing, not a test-harness artefact
MathS.Multithreadingis public and documented as being for "distribut[ing] computations to other threads", with a worked example runningSolveon aTask. So concurrent use is a supported scenario, and shared mutable state anywhere underInnerSimplifiedis a defect for callers, not only for xUnit.The kernel itself is single-threaded — no
Parallel.,Task.RunorAsParallelanywhere in it — so a singleDeterminantLaplacecall is sequential. Whatever corrupts it comes from another thread running beside it.What was wrong
One pattern, three places: a cache grown under a lock and read without one.
where fixed in InternalAMExtensions.Factorial(int)— aListgrown under a lock, read outside it, and a finalreturnafter the lock was released; plus a single sharedIEnumeratoradvanced from every thread#1227 GetSpougeFactorialConstants— an unsynchronisedTryGetValueracing anAddmade under a lock#1227 Primes.GetPrime—EnsurePrimesExistgrows under a lock, then the list is indexed after it is released#1229 List<T>andDictionary<TKey,TValue>are both documented as safe for concurrent readers only while nobody writes.Addreallocates, so a reader outside the lock can see the newCountagainst the old array and return a different element — a wrong factorial or a wrong prime, well-formed, enormous, and silent.That is exactly the failure shape here, which is why I chased it.
Why I am not closing this
The determinant path does not obviously reach any of them.
DeterminantLaplaceruns onGenTensorparameterised byEntityTensorWrapperOperations, which I checked first: it is areadonly structwith no state, whoseAdd/Multiply/Negateare pure functions ending in.InnerSimplified. Simplifying a matrix of integers and two symbols does not callFactorialorGetPrime. So a corrupted factorial would corrupt the test that asked for one, not this one.If these had been the cause, the mechanism connecting them is missing, and I have learned the hard way on this repo that a hypothesis explaining the symptom is not the same as the cause.
Checked and cleared
SpecialSet.innerStorageandInternalAMExtensions.Constants— both[ThreadStatic], so per-thread and safe.- The lazy property caches (
Evaled,InnerSimplified,DirectChildren,Nodes) useLazyPropertyA, which is a struct with no documented thread-safety. I do not think it is this: two threads computing the same property of the same instance compute the same value, so a race there is idempotent, and a torn publish would yieldnulland aNullReferenceExceptionrather than a coherent foreign integer.
Still unaudited
The rule-application path under
Simplify, the e-graph, and the per-node sort-key cache. That is where I would look next, on the reasoning that the foreign value has to come from a table that is shared between unrelated expressions — which is the one property all three fixed caches had and which the lazy per-instance properties do not.- added a commit that references this issue
on Sep 9, 2026 Found it, and it reproduces in about a tenth of a second. The shared state is not in AngouriMath — it is in GenTensor.
That is why the audit in my last comment kept coming up empty: I was auditing the wrong assembly.
Reproduction
Sixty small matrices of integers and symbols, determinants computed once single-threaded for truth, then recomputed under
Parallel.For:DeterminantLaplace, concurrently → 16 of 60 disagreeAround 100 ms, every run, no waiting for a rare interleaving. Two examples, both structurally intact and numerically wrong — the same shape as the two sightings above.
It reaches the public API
Matrix.DeterminanttriesPolynomialDeterminant.Offirst and falls back toInnerMatrix.DeterminantLaplace()only where that declines. So a matrix of polynomials never touches the broken path — which is why my first attempt at this test found nothing, and is worth stating because it bounds the damage.Make the entries non-polynomial and the fallback is taken:
Matrix.Determinant, entries sin(a) etc., concurrently → 9 of 60 disagreeSo this is reachable from the documented public surface, and
MathS.Multithreadingdocuments concurrent use as supported.(One methodological note, because it nearly fooled me:
Determinantis a cached lazy property. Computing the expected values first and then re-reading the same instances in parallel shows nothing at all — the parallel loop only reads caches. The instances have to be rebuilt inside each round.)Where the state is
Reflecting over
GenericTensor, Version=1.0.4.0for mutable statics:GenericTensor.Core.SquareMatrixFactory`2.tensorTempFactorySquareMatrices : List GenericTensor.Core.Expressions.ExpressionCompiler`2.storage : DictionaryA static pool of temporary scratch matrices, and a static dictionary of compiled expressions. A shared scratch buffer handed to two threads computing determinants at once is exactly a determinant coming back with another computation's values in it.
AngouriMath's own side is clean, for the record:
EntityTensorWrapperOperationsis a statelessreadonly structwhose operations are pure, and the kernel contains noParallel./Task.Runat all.What I suggest
Two levels, and I would like your view on the second:
- A guard here, now. The single call site can take a lock, which costs serialised determinants only for matrices the elimination declines, against silent wrong answers. Small and immediate; I will open it as a PR unless you would rather not carry a lock.
- The real fix is upstream in GenTensor — a per-call scratch buffer rather than a static pool, or a
[ThreadStatic]one. Happy to write that too if you want it raised there; it is your call whether AngouriMath keeps the guard afterwards as belt and braces.
The three races I fixed on the way here (#1227, #1229) were real and worth fixing, but they were not this, and I have not claimed otherwise.
Fixed, both ends. Following up on the root cause above — I read GenericTensor's source rather than only reflecting over the assembly, and the pool is worse than "a shared cache read outside its lock". It is a shared scratch buffer:
internal static GenTensor<T, TWrapper> GetMatrix(int diagLength) { ... return tensorTempFactorySquareMatrices[diagLength - 1]; }
Every caller asking for size n gets the same instance, and then writes into it —
Inversion.GetCofactorMatrix(t, temp, ...)fills the matrix it is handed. Two threads taking a determinant of the same size overwrite each other's minors between the write and the read. There is no interleaving to wait for; it is broken on essentially every concurrent call.There is a second one I had not found:
ExpressionCompiler.storage, the cache of compiled elementwise loops, is a plainDictionarywritten from whichever thread asks first. That one reachesm1 + m2.Upstream
GenericTensor#40 and GenericTensor#41 —
[ThreadStatic]for the pool,ConcurrentDictionaryfor the cache. The Laplace determinant comes out about 5% faster, because the pool is now taken once at the top of the recursion instead of re-checked and re-indexed at each of its n! nodes.One correction on the way there, worth stating because I nearly shipped it: the naive
[ThreadStatic]version measured +10%, since a thread-local lookup inside that recursion is not free. Hoisting it fixed that and then some.I should flag that
asc-community/GenericTensorhas had no push since 2022-07-05, so I would not plan around that PR landing.Here
#1230. Forty 4×4 matrices with non-polynomial entries, sequential then parallel, on
c2c8eb83:disagreements Determinant38 of 40 Inverse18 of 40 Adjugate11 of 40 elementwise operators throws, corrupted DictionaryThe two statics get opposite treatments, which is the only interesting design decision in it. The pool needs a lock — there is nothing to warm and no way to avoid it from outside. The compiled-operation cache does not: it is unsafe only while being filled, and the set of keys this library can ever ask for is fixed at three, because
Entity.Matrixrefuses anything that is not rank 2 and every call site uses single-threaded mode. Filling those three once underLazymakes the dictionary read-only from then on, so elementwise addition keeps its concurrency.I would rather delete
GenTensorGuardthan keep it, so if you would prefer to wait for upstream and carry the defect meanwhile, say so and I will close #1230.Two notes on what nearly went wrong, since both produce a green test against broken code.
Determinant,InverseandAdjugateare cached lazy properties, so a test that builds its matrices once and reads them twice never runs the operation twice — the parallel pass only reads caches. And conversely, computing expected values before going parallel warms the compiled-operation cache, after which the dictionary race cannot fire at all. The first version of my test had both.The three races I fixed getting here (#1227, #1229) were real, but they were never this, and I have not claimed otherwise anywhere.
- added a commit that references this issue
on Sep 10, 2026
Seen once, in a parallel run of the unit tests (
--filter "FullyQualifiedName!~Calculus", on a branch whose only change is a new summation closed form, nothing on the determinant's path).FractionFreeDeterminantTest.EliminationAgreesWithLaplaceWhereverBothApply— seeded withRandom(4242), so the 300 matrices are the same every run — reported one disagreement:The elimination is right. The Laplace expansion,
matrix.InnerMatrix.DeterminantLaplace().InnerSimplified, came back with two of its cofactor terms replaced by ninety-digit integers that have nothing to do with the matrix. The same test:27ed5b53).So the input is fixed and the only variable is what else was running. A term of an expression being another expression's number under a parallel run is the signature of shared state read across threads — a cache keyed on something two unequal entities share, or a non-concurrent dictionary written from two tests at once — rather than anything in the determinant. The two integers are not obviously any known value (not a factorial, not a power of two), which suggests they are intermediate results of some other test's arithmetic.
Not reproduced on demand, so no fix is attached; this is the record so that the next sighting has a first one to compare with. Things worth doing when it is next seen: run the chunk with
-- xunit.parallelizeTestCollections=falseto confirm it goes away serially; logGetHashCodeof the offending cofactor products against the big integers' sources; check everystaticcache on theInnerSimplified/Evaledpath for a key that is a hash without an equality.Log lines from the run, for the record (the test name, the matrix and both outputs are verbatim above).
🤖 Generated with Claude Code
https://claude.ai/code/session_012sonx8iAspMiwRwokT1Ura