Matrix operations give wrong answers when called from more than one thread

[Rafael-SOWNet]

Fixes #40.

Two process-wide statics are read and written with no synchronisation, so matrix operations called from more than one thread come back with wrong numbers. Nothing throws and no result is malformed, which is the worst way for this to fail.

Found from the other side: AngouriMath builds its symbolic matrices on GenTensor, and a determinant taken on a background thread was numerically wrong (AngouriMath#1219).

The scratch-matrix pool

SquareMatrixFactory.GetMatrix hands every caller the same GenTensor for a given size, and the caller writes into it — Inversion.GetCofactorMatrix(t, temp, ...) fills the matrix it is handed. Two threads at the same size overwrite each other's minors between the write and the read.

Sixty 5×5 int matrices, computed once sequentially for truth, then rebuilt and recomputed under Parallel.For:

	disagreements
DeterminantLaplace	53 of 60
Adjoint	60 of 60

[ThreadStatic] fixes it: the buffer is scratch, so there is nothing to share between threads in the first place. The lock goes with it, since nothing outside the owning thread can see the list any more — and that lock was incomplete anyway, because the list was indexed after it was released, so a concurrent Add could reallocate underneath the reader.

DeterminantGaussianSafeDivision is unaffected; it works on its own copy. There is a test recording that, so the boundary stays measured rather than assumed.

The compiled-operation cache

ExpressionCompiler.storage is a plain Dictionary written by whichever thread asks first. Reached cold from several threads it fails with the runtime's own diagnostic:

Operations that change non-concurrent collections must have exclusive access.
A concurrent update was performed on this collection and corrupted its state.

followed by a lookup missing a key that had been written. ConcurrentDictionary fixes it. A duplicate compilation under GetOrAdd is harmless, since two compilations of one key produce delegates that do the same thing.

One note on the test, because it nearly fooled me. My first version computed the expected values sequentially and only then went parallel — which populated every key before the threads started, so the parallel pass only read a finished dictionary and the test passed against the broken code. The cache holds a handful of entries, so any warm-up hides the defect completely. The test has none now.

Performance

Single-threaded, best of 15 runs, two independent runs of each arm on one machine:

ms	before	after
Laplace 6×6	0.00870 / 0.00875	0.00838 / 0.00825
Laplace 9×9	4.430 / 4.395	4.201 / 4.197
Adjoint 6×6	0.00808 / 0.00799	0.00821 / 0.00783

The Laplace determinant comes out about 5% faster. Getting there took one correction worth stating: reading a [ThreadStatic] field costs a thread-local lookup, and the naive version, which did that at every one of the n! nodes of the recursion, measured +10% — a real regression I would have shipped if I had not measured it. The pool is taken once at the top and threaded through instead; sizes only shrink on the way down, so growing it once covers every level. That also replaces the old per-node Count check and shared-list index with a single local index, which is where the 5% comes from.

221 of 221 existing tests pass, plus the four added here.

I would rather you carried the fix than AngouriMath carried a lock around your API, but I appreciate the repository has been quiet since 2022 — so if you would prefer not to take this, say so and I will guard it on our side instead.

🤖 Generated with Claude Code

https://claude.ai/code/session_012sonx8iAspMiwRwokT1Ura

[Rafael-SOWNet]

The red UnitTests check is not this branch. It is the coverage job, and it fails before a single line of code is compiled:

error : Microsoft.NET.Test.Sdk doesn't support net6.0 and has not been tested with it.
        Consider upgrading your TargetFramework to net8.0 or later.
Unable to create dependency graph file for project 'UnitTests/UnitTests.csproj'.
Cannot add package reference.
##[error]Process completed with exit code 1

That job runs an unpinned dotnet add package Microsoft.NET.Test.Sdk, which today resolves to 18.10.0, and 18.10.0 refuses a net6.0 test project. It is bit rot from the passage of time rather than from any diff — nothing here touches a .csproj or a target framework.

The three jobs that actually build and run the code are green:

SUCCESS  UnitTests (ubuntu-latest)
SUCCESS  UnitTests (windows-latest)
SUCCESS  UnitTests (macos-latest)

Two ways out if you want that job green again: pin the package in the workflow, or set SuppressTfmSupportBuildErrors in UnitTests.csproj. Both are unrelated to this change, so I have deliberately not put either in here — say the word and I will open it separately.