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.

Sign up for GitHub

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

Add optimised SmallestMovedPoint implementation #3721

Merged
merged 1 commit into from
Nov 13, 2019
Merged

Add optimised SmallestMovedPoint implementation #3721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions lib/permutat.g
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,12 @@ InstallMethod( SmallestMovedPoint,
end );


InstallMethod( SmallestMovedPoint,
"for an internal permutation",
[ IsPerm and IsInternalRep ],
SMALLEST_MOVED_POINT_PERM );


#############################################################################
##
#m LargestMovedPoint( <perm> ) . . . . . . . . for internal permutation
Expand Down
50 changes: 50 additions & 0 deletions src/permutat.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,40 @@ UInt LargestMovedPointPerm(Obj perm)
return LargestMovedPointPerm_<UInt4>(perm);
}

/****************************************************************************
**
*F SmallestMovedPointPerm( <perm> ) smallest point moved by perm
**
** 'SmallestMovedPointPerm' implements 'SmallestMovedPoint' for internal
** permutations.
*/

// Import 'Infinity', as a return value for the identity permutation
static Obj Infinity;

template <typename T>
static inline Obj SmallestMovedPointPerm_(Obj perm)
{
const T * ptPerm = CONST_ADDR_PERM<T>(perm);
UInt deg = DEG_PERM<T>(perm);

for (UInt i = 1; i <= deg; i++) {
if (ptPerm[i - 1] != i - 1)
wilfwilson marked this conversation as resolved.
Show resolved Hide resolved
return INTOBJ_INT(i);
}
return Infinity;
}

static Obj SmallestMovedPointPerm(Obj perm)
{
GAP_ASSERT(TNUM_OBJ(perm) == T_PERM2 || TNUM_OBJ(perm) == T_PERM4);

if (TNUM_OBJ(perm) == T_PERM2)
return SmallestMovedPointPerm_<UInt2>(perm);
else
return SmallestMovedPointPerm_<UInt4>(perm);
}


/****************************************************************************
**
Expand All @@ -1141,6 +1175,18 @@ static Obj FuncLARGEST_MOVED_POINT_PERM(Obj self, Obj perm)
return INTOBJ_INT(LargestMovedPointPerm(perm));
}

/****************************************************************************
**
*F FuncSMALLEST_MOVED_POINT_PERM( <self>, <perm> )
**
** GAP-level wrapper for 'SmallestMovedPointPerm'.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this adds any useful information, as the 2 line implementation makes this info clear anyway. So I'd just remove it.

Suggested change
** GAP-level wrapper for 'SmallestMovedPointPerm'.

*/
static Obj FuncSMALLEST_MOVED_POINT_PERM(Obj self, Obj perm)
{
RequirePermutation("SmallestMovedPointPerm", perm);

return SmallestMovedPointPerm(perm);
}

/****************************************************************************
**
Expand Down Expand Up @@ -2736,6 +2782,7 @@ static StructGVarFunc GVarFuncs [] = {

GVAR_FUNC_1ARGS(PermList, list),
GVAR_FUNC_1ARGS(LARGEST_MOVED_POINT_PERM, perm),
GVAR_FUNC_1ARGS(SMALLEST_MOVED_POINT_PERM, perm),
GVAR_FUNC_2ARGS(CYCLE_LENGTH_PERM_INT, perm, point),
GVAR_FUNC_2ARGS(CYCLE_PERM_INT, perm, point),
GVAR_FUNC_1ARGS(CYCLE_STRUCT_PERM, perm),
Expand Down Expand Up @@ -2784,6 +2831,9 @@ static Int InitKernel (

ImportGVarFromLibrary("PERM_INVERSE_THRESHOLD", &PERM_INVERSE_THRESHOLD);

/* needed for SmallestMovedPoint */
ImportGVarFromLibrary("infinity", &Infinity);

/* install the type functions */
ImportGVarFromLibrary( "TYPE_PERM2", &TYPE_PERM2 );
ImportGVarFromLibrary( "TYPE_PERM4", &TYPE_PERM4 );
Expand Down
40 changes: 33 additions & 7 deletions tst/testinstall/perm.tst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#@local checklens,n,permAll,permBig,permSml,x,y
#@local checklens,n,permAll,permBig,permSml,x,y, moved
gap> START_TEST("perm.tst");

# Permutations come in two flavors in GAP, with two TNUMs: T_PERM2 for
Expand Down Expand Up @@ -314,12 +314,38 @@ gap> PermList(Concatenation([1..70000],[70001,1,70003]));
fail

#
# LARGEST_MOVED_POINT_PERM
#
gap> LARGEST_MOVED_POINT_PERM((2,3));
3
gap> LARGEST_MOVED_POINT_PERM((2,70000));
70000
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for these tests was to make 100% sure we test those kernel function, and not some other installed methods.

I guess these days, I'd move those tests to tst/testinstall/kernel/permutat.tst

# SmallestMovedPoint, LargestMovedPoint, MovedPoints, NrMovedPoints
#
wilfwilson marked this conversation as resolved.
Show resolved Hide resolved
gap> moved := {p} -> [SmallestMovedPoint(p), LargestMovedPoint(p), MovedPoints(p), NrMovedPoints(p)];;
gap> moved((2,3));
[ 2, 3, [ 2, 3 ], 2 ]
gap> moved((2,70000));
[ 2, 70000, [ 2, 70000 ], 2 ]
gap> moved((70000, 70001));
[ 70000, 70001, [ 70000, 70001 ], 2 ]
gap> moved((1,2,3,4));
[ 1, 4, [ 1, 2, 3, 4 ], 4 ]
gap> moved(());
[ infinity, 0, [ ], 0 ]
gap> moved((1,2,3,4)^4);
[ infinity, 0, [ ], 0 ]
gap> moved([]);
[ infinity, 0, [ ], 0 ]
gap> moved([()]);
[ infinity, 0, [ ], 0 ]
gap> moved([(1,2),(5,6)]);
[ 1, 6, [ 1, 2, 5, 6 ], 4 ]
gap> moved(SymmetricGroup([5,7,9]));
[ 5, 9, [ 5, 7 .. 9 ], 3 ]
gap> moved(SymmetricGroup([5,7,9,70000]));
[ 5, 70000, [ 5, 7, 9, 70000 ], 4 ]
gap> moved(SymmetricGroup(1));
[ infinity, 0, [ ], 0 ]
gap> moved(Group((8,9,10),(13,15,19)));
[ 8, 19, [ 8, 9, 10, 13, 15, 19 ], 6 ]
gap> SMALLEST_MOVED_POINT_PERM(fail);
Error, SmallestMovedPointPerm: <perm> must be a permutation (not the value 'fa\
il')
gap> LARGEST_MOVED_POINT_PERM(fail);
Error, LargestMovedPointPerm: <perm> must be a permutation (not the value 'fai\
l')
Expand Down