From c40ff363c05d180c7f984bdbc43dcc2a8ad2e79a Mon Sep 17 00:00:00 2001 From: Rafael Vuijk Date: Thu, 10 Sep 2026 09:39:46 +0000 Subject: [PATCH] The scratch-pool lock is the guard's API rather than a convention, because one caller had skipped it FractionFreeDeterminantTest.EliminationAgreesWithLaplaceWhereverBothApply fails on master whenever it runs beside MatrixConcurrencyTest, and it fails by reporting the determinant of a matrix of integers and a, b, c as containing sin(a) -- entries that belong to the other test's matrices. The guard added in #1230 covers GenericTensor's process-wide scratch matrix at all three of the library's call sites. It did not cover this one, which reaches DeterminantLaplace directly in order to compare Laplace against the elimination, and one unguarded caller is enough: it corrupts every guarded caller and reads their minors back. Taking the lock in the test would fix this instance. Making the lock private and the three pool operations -- DeterminantLaplace, Adjoint, InvertMatrix -- the guard's whole surface fixes the shape, since a caller can then no longer reach GenericTensor without it. That is what this does. Adjoint returns the entity rather than the tensor for the same reason. The lock has to cover reading the elements out, and Entity.Matrix's constructor is where they are actually read, so a GenTensor-returning signature would put that read on the far side of the lock. Reproduction, on master, this machine: dotnet test --filter "FullyQualifiedName~MatrixConcurrencyTest|FullyQualifiedName~FractionFreeDeterminantTest" Failed: 1 (13 of 300 determinants disagree) and with this change, three runs of the same command, Failed: 0 each time. Suites: UnitTests 8514 and 1508, FSharpWrapperUnitTests 134, InteractiveWrapperUnitTests 18, TerminalUnitTests 41. https://github.com/asc-community/AngouriMath/issues/1219 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012sonx8iAspMiwRwokT1Ura --- .../Core/Entity/Omni/Entity.Matrix.cs | 12 ++---- .../AngouriMath/Functions/GenTensorGuard.cs | 42 ++++++++++++++++++- .../FractionFreeDeterminantTest.cs | 8 +++- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/Sources/AngouriMath/Core/Entity/Omni/Entity.Matrix.cs b/Sources/AngouriMath/Core/Entity/Omni/Entity.Matrix.cs index b1d2467bc..5f783057c 100644 --- a/Sources/AngouriMath/Core/Entity/Omni/Entity.Matrix.cs +++ b/Sources/AngouriMath/Core/Entity/Omni/Entity.Matrix.cs @@ -304,8 +304,8 @@ public Entity AsScalar() // GenericTensor's Laplace determinant writes into a process-wide scratch // matrix, so two threads here silently corrupt each other's minors. See // Functions.GenTensorGuard. - lock (Functions.GenTensorGuard.ScratchPool) - return @this.InnerMatrix.DeterminantLaplace().InnerSimplified; + return Functions.GenTensorGuard + .DeterminantLaplace(@this.InnerMatrix).InnerSimplified; }, this ); @@ -323,8 +323,7 @@ public Entity AsScalar() return null; // Inverting goes through the adjugate, which takes its minors in the same // process-wide scratch matrix the determinant does. See Functions.GenTensorGuard. - lock (Functions.GenTensorGuard.ScratchPool) - cp.InvertMatrix(); + Functions.GenTensorGuard.InvertMatrix(cp); return ToMatrix(new Matrix(cp).InnerSimplified); }, this); private LazyPropertyA inverse; @@ -498,10 +497,7 @@ public IEnumerator GetEnumerator() return null; // The adjugate takes every minor in one process-wide scratch matrix. // See Functions.GenTensorGuard. - Entity innerSimplified; - lock (Functions.GenTensorGuard.ScratchPool) - innerSimplified = new Matrix(@this.InnerMatrix.Adjoint()).InnerSimplified; - return ToMatrix(innerSimplified); + return ToMatrix(Functions.GenTensorGuard.Adjoint(@this.InnerMatrix)); }, this); private LazyPropertyA adjugate; diff --git a/Sources/AngouriMath/Functions/GenTensorGuard.cs b/Sources/AngouriMath/Functions/GenTensorGuard.cs index 194e71742..f7714f7c4 100644 --- a/Sources/AngouriMath/Functions/GenTensorGuard.cs +++ b/Sources/AngouriMath/Functions/GenTensorGuard.cs @@ -47,7 +47,47 @@ internal static class GenTensorGuard /// goes through the adjugate. DeterminantGaussianSafeDivision and /// MatrixMultiply do not touch the pool and are deliberately not covered. /// - [ConstantField] internal static readonly object ScratchPool = new object(); + /// + /// It is private, and the three operations below are the whole surface, because mutual + /// exclusion here is not a property any one caller can hold up: a single call that skips + /// the lock corrupts every call that takes it, and reads corrupted minors back. Exposing + /// the lock lets a caller reach GenericTensor without it; exposing only the operations + /// does not. + /// + [ConstantField] private static readonly object ScratchPool = new object(); + + /// Laplace expansion, serialised on . + internal static Entity DeterminantLaplace(GenTensor matrix) + { + lock (ScratchPool) + return matrix.DeterminantLaplace(); + } + + /// + /// The adjugate, serialised on , and returned already read out + /// of the tensor rather than as one. + /// + /// + /// Reading it out is what the lock has to cover, and returning a GenTensor would + /// put that read on the far side of the lock: Entity.Matrix's constructor copies, + /// so the copy is where the elements are actually touched, and nothing here documents + /// whether the adjugate is a fresh tensor or the pool's own. + /// + internal static Entity Adjoint(GenTensor matrix) + { + lock (ScratchPool) + return new Entity.Matrix(matrix.Adjoint()).InnerSimplified; + } + + /// + /// Inversion in place, serialised on -- it goes through the + /// adjugate, so it takes its minors in the same pool. + /// + internal static void InvertMatrix(GenTensor matrix) + { + lock (ScratchPool) + matrix.InvertMatrix(); + } [ConstantField] private static readonly Lazy piecewiseCache = new Lazy(WarmPiecewiseCache, LazyThreadSafetyMode.ExecutionAndPublication); diff --git a/Sources/Tests/UnitTests/Algebra/Polynomials/FractionFreeDeterminantTest.cs b/Sources/Tests/UnitTests/Algebra/Polynomials/FractionFreeDeterminantTest.cs index 7dc3dced4..414703e0e 100644 --- a/Sources/Tests/UnitTests/Algebra/Polynomials/FractionFreeDeterminantTest.cs +++ b/Sources/Tests/UnitTests/Algebra/Polynomials/FractionFreeDeterminantTest.cs @@ -148,7 +148,13 @@ public void EliminationAgreesWithLaplaceWhereverBothApply() if (ByElimination(matrix) is not { } byElimination) continue; compared++; - var byLaplace = matrix.InnerMatrix.DeterminantLaplace().InnerSimplified; + // Through the guard rather than straight to GenericTensor, whose Laplace + // expansion takes its minors in a process-wide scratch matrix. xUnit runs test + // classes in parallel and MatrixConcurrencyTest expands forty matrices across + // every core, so an unguarded call here reads that test's minors and reports the + // determinant of an integer matrix as containing sin(a). + var byLaplace = GenTensorGuard.DeterminantLaplace(matrix.InnerMatrix) + .InnerSimplified; if ((byElimination - byLaplace).Simplify() != 0) disagreements.Add($"{matrix.Stringize()}: " + $"{byElimination.Stringize()} vs {byLaplace.Stringize()}");