Skip to content

Commit

Permalink
Fix multiplication of compressed matrix by scalar (#5688)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fingolfin authored Mar 27, 2024
1 parent 22911ef commit cfebe1d
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/vec8bit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
51 changes: 51 additions & 0 deletions tst/testbugfix/2024-03-25-compressed-mat-scalar.tst
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# Multiplying a compressed matrix by a scalar could result
# in corrupt data. See <https://github.com/gap-system/gap/issues/5684>
#
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 .

0 comments on commit cfebe1d

Please sign in to comment.