Determinant and Inverse return wrong values when called from more than one thread
Evaluación
- Dificultad
- 4/5
- Tiempo estimado
- 3-5 días
- Aptitud para principiantes
- 35/100
- Tipo de issue
- Error
- Claridad
- Bien especificado
- Estado de actividad
- Estancado
- Stack tecnológico
- csharp
- Área
- performance
Línea de trabajo
Comienza con SquareMatrixFactory<T, TWrapper>.GetMatrix y ExpressionCompiler<T, TWrapper>.storage; después reproduce los fallos de determinante/inversa indicados con Parallel.For, incluyendo una caché fría. Se considera terminado cuando las operaciones concurrentes de matrices devuelven los valores verificados secuencialmente sin excepciones de colección ni corrupción de búferes compartidos.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
Two process-wide statics are read and written with no synchronisation, so matrix operations called from more than one thread return wrong numbers. Nothing throws, nothing is malformed, and no result looks suspicious.
I found this from the other side: AngouriMath uses GenTensor for its symbolic matrices, and a determinant computed on a background thread was coming back numerically wrong. AngouriMath's issue is #1219.
1. The scratch-matrix pool
SquareMatrixFactory<T, TWrapper>.GetMatrix hands every caller the same GenTensor for a given size, and the caller then writes into it — Inversion.GetCofactorMatrix(t, temp, ...) fills the matrix it is given. Two threads taking a determinant or an inverse of the same size get the same buffer and overwrite each other's minors between the write and the read.
Sixty 5×5 int matrices, each computed once sequentially for truth and then rebuilt and recomputed under Parallel.For:
DeterminantLaplace 53 of 60 disagree
Adjoint 60 of 60 disagree
Around 10 ms, every run. This is not a rare interleaving, it is the normal case.
DeterminantGaussianSafeDivision is unaffected — it works on its own copy and never asks the pool for anything.
The lock inside GetMatrix does not help and is itself incomplete: the list is indexed after the lock is released, so a concurrent Add can reallocate the backing array underneath the reader.
2. The compiled-operation cache
ExpressionCompiler<T, TWrapper>.storage is a plain Dictionary, written by whichever thread first asks for a given (operation, rank, parallel). Dictionary<,> is documented as safe for concurrent readers only while nobody is writing.
Reached cold from several threads it fails with the runtime's own diagnostic:
System.InvalidOperationException: Operations that change non-concurrent collections must
have exclusive access. A concurrent update was performed on this collection and corrupted
its state. The collection's state is no longer correct.
...
The given key '(Subtraction, 1, False)' was not present in the dictionary.
Worth noting that this one hides easily. My first test computed its expected values sequentially and only then went parallel, which populated every key before the threads started — the cache holds a handful of entries, so a warm-up makes the defect invisible and the test passes against the broken code.
Fix
I have opened a PR. Both are small: [ThreadStatic] for the pool, since a scratch buffer has nothing to share between threads, and ConcurrentDictionary for 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 every one of its n! nodes.
- Lenguaje dominante
- C#
- Estrellas
- 52
- Forks
- 6
- Métricas de merge de PR
- Sin PR fusionados en 30 d
Preparar el entorno
Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de ASC-Community/GenericTensor
-
Null reference warnings to fixAbiertogood first issue
Dificultad 4/5 3-5 días Aptitud para principiantes 25/100
-
proposal
Dificultad 4/5 3-5 días Aptitud para principiantes 35/100
ASC-Community/GenericTensor#36 · 1 comentario ·
-
Opinions wanted proposal
Dificultad 5/5 Más de una semana Aptitud para principiantes 25/100
ASC-Community/GenericTensor#31 · 2 comentarios ·
-
Will this package support MKL or OpenBlas as backend to accelerate matrix inverse computing speedAbierto
Dificultad 5/5 Más de una semana Aptitud para principiantes 25/100
ASC-Community/GenericTensor#30 · 1 comentario ·
-
Dificultad 5/5 Más de una semana Aptitud para principiantes 20/100
ASC-Community/GenericTensor#29 · 1 comentario ·
Todos los issues de ASC-Community/GenericTensor
Issues similares
-
bug next version
Dificultad 2/5 1-3 horas Aptitud para principiantes 68/100
Rem0o/FanControl.Releases#4220 · 1 comentario ·
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
nkdAgility/azure-devops-migration-tools#3217 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
skoruba/Duende.IdentityServer.Admin#335 · 1 comentario ·
Los mantenedores suelen responder en 1 día
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 86/100
Los mantenedores suelen responder en 1 día
-
question
Dificultad 2/5 1-3 horas Aptitud para principiantes 68/100