Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear stored inverse when calling TrimPerm #3575

Merged
merged 1 commit into from
Jul 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/permutat.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2238,6 +2238,7 @@ Obj Array2Perm (

void TrimPerm(Obj perm, UInt m)
{
CLEAR_STOREDINV_PERM(perm);
if (TNUM_OBJ(perm) == T_PERM2) {
GAP_ASSERT(m <= DEG_PERM2(perm));
ResizeBag(perm, SIZEBAG_PERM2(m));
Expand Down
19 changes: 18 additions & 1 deletion src/permutat.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,12 @@ EXPORT_INLINE const UInt4 * CONST_ADDR_PERM4(Obj perm)

EXPORT_INLINE Obj STOREDINV_PERM(Obj perm)
{
return ADDR_OBJ(perm)[0];
Obj inv = ADDR_OBJ(perm)[0];
/* Check inv has the same TNAM as perm
This is checked below in SET_STOREDINV_PERM, but could
be invalidated if either perm or inv is changed in-place */
GAP_ASSERT(!inv || (TNUM_OBJ(perm) == TNUM_OBJ(inv)));
return inv;
}

/* SET_STOREDINV_PERM should only be used in neither perm, nor inv has
Expand All @@ -98,6 +103,18 @@ EXPORT_INLINE void SET_STOREDINV_PERM(Obj perm, Obj inv)
}
}

/* Clear the stored inverse. This is required if 'perm' changes TNUM.
Also clears the stored inverse of the stored inverse (which should be
perm). */
EXPORT_INLINE void CLEAR_STOREDINV_PERM(Obj perm)
{
Obj inv = ADDR_OBJ(perm)[0];
if (inv) {
ADDR_OBJ(inv)[0] = 0;
ADDR_OBJ(perm)[0] = 0;
}
}


#define IMAGE(i,pt,dg) (((i) < (dg)) ? (pt)[(i)] : (i))

Expand Down
40 changes: 40 additions & 0 deletions tst/testbugfix/2019-07-15-StoredInv.tst
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# When TRIM_PERM causes a permutation 'p' to change TNAM, the stored inverse
# must be cleared, as the stored inverse of 'p' must have the
# same TNAM as 'p'

# Make p a Perm4
gap> p := (1,2,3,4)*(2^16,2^16+1)*(2^16,2^16+1);
(1,2,3,4)
gap> IsPerm4Rep(p);
true

# Force inverse calculation
gap> q := p^-1;
(1,4,3,2)
gap> IsPerm4Rep(q);
true

# Now trim
gap> TRIM_PERM(p, 4);
gap> IsPerm2Rep(p);
true

# Check inverse is also a perm2
gap> IsPerm2Rep(p^-1);
true

# But this has not changed q
gap> IsPerm4Rep(q);
true

# and it's inverse is the correct type
gap> IsPerm4Rep(q^-1);
true

# Check some calculations that use the inverse
gap> List([1..5], x -> x/p);
[ 4, 1, 2, 3, 5 ]

# And on q (to ensure it's inverse is not still 'p')
gap> List([1..5], x -> x/q);
[ 2, 3, 4, 1, 5 ]