From cfebe1dc6ed2d0d053afa4bc7120ee0db1c0e42f Mon Sep 17 00:00:00 2001 From: Max Horn Date: Wed, 27 Mar 2024 10:38:46 +0100 Subject: [PATCH] Fix multiplication of compressed matrix by scalar (#5688) There was a long-standing bug where multiplying a compressed matrix over a finite field by a scalar could lead to a corrupt result which exhibited weird behavior in subsequent computations. The root cause of this is that in GAP, finite field elements like `Z(2^2)` and `Z(2^4)^5` are equal but (internally) not identical. In this case, a GAP library function had an input validation test that check if a scalar is in the right field (in the new test case, the field is GF(64), and `Z(2^2)` lives in that, hence also `Z(2^4)^5` as it is equal). But the kernel code then used a *different* test which distinguishes between these two elements. For `Z(2^2)` it happily proceeded as before and all was good. But for `Z(2^4)^5` it instead dispatches to a generic function which ends up producing a result over the wrong field. Specifically, before this patch, we had this in the example from the new .tst file: gap> List(a, Q_VEC8BIT); [ 64, 64, 4, 4, 4, 4 ] gap> List(b, Q_VEC8BIT); [ 64, 64, 64, 64, 64, 64 ] With the fix, both matrices give the correct second result. --- src/vec8bit.c | 2 +- .../2024-03-25-compressed-mat-scalar.tst | 51 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 tst/testbugfix/2024-03-25-compressed-mat-scalar.tst diff --git a/src/vec8bit.c b/src/vec8bit.c index d83712c4cb..dcc74b7463 100644 --- a/src/vec8bit.c +++ b/src/vec8bit.c @@ -1409,7 +1409,7 @@ static Obj FuncPROD_VEC8BIT_FFE(Obj self, Obj vec, Obj ffe) GAP_ASSERT(CHAR_FF(FLD_FFE(ffe)) == P_FIELDINFO_8BIT(info)); // check for field compatibility - if (d % DEGR_FF(FLD_FFE(ffe))) { + if (d % DegreeFFE(ffe)) { prod = ProdListScl(vec, ffe); CALL_1ARGS(ConvertToVectorRep, prod); return prod; diff --git a/tst/testbugfix/2024-03-25-compressed-mat-scalar.tst b/tst/testbugfix/2024-03-25-compressed-mat-scalar.tst new file mode 100644 index 0000000000..0df7064a22 --- /dev/null +++ b/tst/testbugfix/2024-03-25-compressed-mat-scalar.tst @@ -0,0 +1,51 @@ +# Multiplying a compressed matrix by a scalar could result +# in corrupt data. See +# +gap> g1 := GO(1,6,64).1; +< immutable compressed matrix 6x6 over GF(64) > +gap> a := g1*Z(2^4)^5; +< immutable compressed matrix 6x6 over GF(64) > +gap> b := g1*Z(2^2); +< immutable compressed matrix 6x6 over GF(64) > +gap> a = b; +true +gap> TransposedMat(a) = TransposedMat(b); +true +gap> List(a, Q_VEC8BIT); +[ 64, 64, 64, 64, 64, 64 ] +gap> List(b, Q_VEC8BIT); +[ 64, 64, 64, 64, 64, 64 ] + +# also verify printing matches +gap> Display(a); +z = Z(64) + z^22 . . . . . + . z^20 . . . . + . . . z^21 . . + . . z^21 . . . + . . . . . z^21 + . . . . z^21 . +gap> Display(b); +z = Z(64) + z^22 . . . . . + . z^20 . . . . + . . . z^21 . . + . . z^21 . . . + . . . . . z^21 + . . . . z^21 . +gap> Display(TransposedMat(a)); +z = Z(64) + z^22 . . . . . + . z^20 . . . . + . . . z^21 . . + . . z^21 . . . + . . . . . z^21 + . . . . z^21 . +gap> Display(TransposedMat(b)); +z = Z(64) + z^22 . . . . . + . z^20 . . . . + . . . z^21 . . + . . z^21 . . . + . . . . . z^21 + . . . . z^21 .