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

Jump to bottom

Better define the result of MappingPermListList #1432

Merged
merged 1 commit into from
Jun 20, 2017
Merged

Better define the result of MappingPermListList #1432

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
9 changes: 5 additions & 4 deletions lib/permutat.g
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ end );
## gap> PermList([1,2,4,5,3]);
## (3,4,5)
## gap> MappingPermListList([2,5,1,6],[7,12,8,2]);
## (1,8,5,12,11,10,9,6,2,7,4,3)
## (1,8,5,12,6,2,7)
## gap> RestrictedPerm((1,2)(3,4),[3..5]);
## (3,4)
## ]]></Example>
Expand Down Expand Up @@ -560,10 +560,11 @@ end);
## <Ref Func="MappingPermListList"/> returns the permutation <C>p</C> from the
## previous sentence, i.e. <A>src</A><C>[</C><M>i</M><C>]^</C><M>p =</M>
## <A>dst</A><C>[</C><M>i</M><C>]</C>.
## The permutation <M>\pi</M> fixes all points larger than the maximum of
## the entries in <A>src</A> and <A>dst</A>.
## The permutation <M>\pi</M> fixes any point which is not in <A>src</A> or
## <A>dst</A>.
## If there are several such permutations, it is not specified which of them
## <Ref Func="MappingPermListList"/> returns.
## <Ref Func="MappingPermListList"/> returns. If there is no such
## permutation, then <Ref Func="MappingPermListList"/> returns <K>fail</K>.
## </Description>
## </ManSection>
## <#/GAPDoc>
Expand Down
98 changes: 80 additions & 18 deletions src/permutat.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4443,34 +4443,63 @@ Obj FuncMappingPermListList(Obj self, Obj src, Obj dst)
Obj out;
Obj tabdst, tabsrc;
Int x;
Obj obj;
Int mytabs[DEGREELIMITONSTACK+1];
Int mytabd[DEGREELIMITONSTACK+1];

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't you check that src and dst are lists here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LEN_LIST will do this -- it throws an Error is passed a non-list.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, but not a very meaningful error, in this context, it'd be easier to find the source of the problem if there is an explicit error here.

if (!IS_LIST(src) ) {
ErrorMayQuit("first argument must be a list (not a %s)", (Int)TNAM_OBJ(src), 0L);
}
if (!IS_LIST(dst) ) {
ErrorMayQuit("second argument must be a list (not a %s)", (Int)TNAM_OBJ(dst), 0L);
}
l = LEN_LIST(src);
if (l != LEN_LIST(dst)) {
ErrorReturnVoid( "both arguments must be lists of equal length",
0L, 0L, "type 'return;' or 'quit;' to exit break loop" );
return 0L;
ErrorMayQuit( "arguments must be lists of equal length", 0L, 0L);
}
d = 0;
for (i = 1;i <= l;i++) {
x = INT_INTOBJ(ELM_LIST(src,i));
obj = ELM_LIST(src, i);
if (!IS_POS_INTOBJ(obj)) {
ErrorMayQuit("first argument must be a list of positive integers", 0L, 0L);
}
x = INT_INTOBJ(obj);
if (x > d) d = x;
}
for (i = 1;i <= l;i++) {
x = INT_INTOBJ(ELM_LIST(dst,i));
obj = ELM_LIST(dst, i);
if (!IS_POS_INTOBJ(obj)) {
ErrorMayQuit("second argument must be a list of positive integers", 0L, 0L);
}
x = INT_INTOBJ(obj);
if (x > d) d = x;
}
if (d <= DEGREELIMITONSTACK) {
/* Small case where we work on the stack: */
memset(&mytabs,0,sizeof(mytabs));
memset(&mytabd,0,sizeof(mytabd));
for (i = 1;i <= l;i++) {
mytabs[INT_INTOBJ(ELM_LIST(src,i))] = i;
Int val = INT_INTOBJ(ELM_LIST(src, i));
if (mytabs[val]) {
// Already read where this value maps, check it is the same
if (ELM_LIST(dst, mytabs[val]) != ELM_LIST(dst, i)) {
return Fail;
}
}
mytabs[val] = i;
}
for (i = 1;i <= l;i++) {
mytabd[INT_INTOBJ(ELM_LIST(dst,i))] = i;
Int val = INT_INTOBJ(ELM_LIST(dst, i));
if (mytabd[val]) {
// Already read where this value is mapped from, check it is
// the same
if (ELM_LIST(src, mytabd[val]) != ELM_LIST(src, i)) {
return Fail;
}
}
mytabd[val] = i;
}

out = NEW_PLIST(T_PLIST_CYC,d);
SET_LEN_PLIST(out,d);
/* No garbage collection from here ... */
Expand All @@ -4479,28 +4508,52 @@ Obj FuncMappingPermListList(Obj self, Obj src, Obj dst)
if (mytabs[i]) { /* if i is in src */
SET_ELM_PLIST(out,i, ELM_LIST(dst,mytabs[i]));
} else {
/* Skip things in dst: */
while (mytabd[next]) next++;
SET_ELM_PLIST(out,i,INTOBJ_INT(next));
next++;
if (mytabd[i]) {
// Skip things in dst:
while (mytabd[next] ||
(mytabs[next] == 0 && mytabd[next] == 0))
next++;
SET_ELM_PLIST(out, i, INTOBJ_INT(next));
next++;
}
else { // map elements in neither list to themselves
SET_ELM_PLIST(out, i, INTOBJ_INT(i));
}
}
}
/* ... to here! No CHANGED_BAG needed since this is a new object! */
} else {
/* Version with intermediate objects: */

tabsrc = NEW_PLIST(T_PLIST,d);
SET_LEN_PLIST(tabsrc,0);
/* No garbage collection from here ... */
for (i = 1;i <= l;i++) {
SET_ELM_PLIST(tabsrc,INT_INTOBJ(ELM_LIST(src,i)),INTOBJ_INT(i));
Int val = INT_INTOBJ(ELM_LIST(src, i));
if (ELM_PLIST(tabsrc, val)) {
if (ELM_LIST(dst, INT_INTOBJ(ELM_PLIST(tabsrc, val))) !=
ELM_LIST(dst, i)) {
return Fail;
}
}
else {
SET_ELM_PLIST(tabsrc, val, INTOBJ_INT(i));
}
}
/* ... to here! No CHANGED_BAG needed since this is a new object! */
tabdst = NEW_PLIST(T_PLIST,d);
SET_LEN_PLIST(tabdst,0);
/* No garbage collection from here ... */
for (i = 1;i <= l;i++) {
SET_ELM_PLIST(tabdst,INT_INTOBJ(ELM_LIST(dst,i)),INTOBJ_INT(i));
int val = INT_INTOBJ(ELM_LIST(dst, i));
if (ELM_PLIST(tabdst, val)) {
if (ELM_LIST(src, INT_INTOBJ(ELM_PLIST(tabdst, val))) !=
ELM_LIST(src, i)) {
return Fail;
}
}
else {
SET_ELM_PLIST(tabdst, val, INTOBJ_INT(i));
}
}
/* ... to here! No CHANGED_BAG needed since this is a new object! */
out = NEW_PLIST(T_PLIST_CYC,d);
Expand All @@ -4512,10 +4565,19 @@ Obj FuncMappingPermListList(Obj self, Obj src, Obj dst)
SET_ELM_PLIST(out,i,
ELM_LIST(dst,INT_INTOBJ(ELM_PLIST(tabsrc,i))));
} else {
/* Skip things in dst: */
while (ELM_PLIST(tabdst,next)) next++;
SET_ELM_PLIST(out,i,INTOBJ_INT(next));
next++;
if (ELM_PLIST(tabdst, i)) {
// Skip things in dst:
while (ELM_PLIST(tabdst, next) ||
(ELM_PLIST(tabdst, next) == 0 &&
ELM_PLIST(tabsrc, next) == 0)) {
next++;
}
SET_ELM_PLIST(out, i, INTOBJ_INT(next));
next++;
}
else {
SET_ELM_PLIST(out, i, INTOBJ_INT(i));
}
}
}
/* ... to here! No CHANGED_BAG needed since this is a new object! */
Expand Down
10 changes: 10 additions & 0 deletions tst/testbugfix/2017-06-19-repaction.tst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# These functions all worked incorrectly when given symmetric or alternating groups
# Which were not not defined on a domain of the form [1..n]
gap> RepresentativeAction(SymmetricGroup([5,7,11,15]),[7,11],[5,15],OnTuples);
(5,7)(11,15)
gap> RepresentativeAction(AlternatingGroup([5,7,11,15]),[7,11],[5,15],OnTuples);
(5,7)(11,15)
gap> RepresentativeAction(SymmetricGroup([5,7,11,15]),[7,11],[5,15],OnSets);
(5,7)(11,15)
gap> RepresentativeAction(AlternatingGroup([5,7,11,15]),[7,11],[5,15],OnSets);
(5,7)(11,15)
40 changes: 40 additions & 0 deletions tst/testinstall/perm.tst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,12 +309,52 @@ gap> List(permBig, SignPerm);
#
# MappingPermListList
#
gap> MappingPermListList([],[]);
()
gap> MappingPermListList([1,1], [2,2]);
(1,2)
gap> MappingPermListList([1,2], [2,1]);
(1,2)
gap> MappingPermListList([1,2], [3,4]);
(1,3)(2,4)
gap> MappingPermListList([2,4,6], [1,2,3]);
(1,4,2)(3,6)
gap> MappingPermListList([1,1], [1,2]);
fail
gap> MappingPermListList([1,2], [1,1]);
fail
gap> MappingPermListList([1,1000], [1,1000]);
()
gap> MappingPermListList([1,1000], [1,1001]);
(1000,1001)
gap> MappingPermListList([1002,1000], [1,1001]);
(1,1000,1001,1002)
gap> MappingPermListList([1,1], [1000,1000]);
(1,1000)
gap> MappingPermListList([1,1], [1000,1001]);
fail
gap> MappingPermListList([1,2], [1000,1000]);
fail
gap> MappingPermListList((), []);
Error, first argument must be a list (not a permutation (small))
gap> MappingPermListList([], ());
Error, second argument must be a list (not a permutation (small))
gap> MappingPermListList("cheese", "cake");
Error, arguments must be lists of equal length
gap> MappingPermListList("cheese", "cakeba");
Error, first argument must be a list of positive integers
gap> MappingPermListList([1,2], [3,[]]);
Error, second argument must be a list of positive integers
gap> MappingPermListList([1,[]], [3,4]);
Error, first argument must be a list of positive integers
gap> MappingPermListList([1,2], [3,0]);
Error, second argument must be a list of positive integers
gap> MappingPermListList([1,0], [3,4]);
Error, first argument must be a list of positive integers
gap> MappingPermListList([1,-1], [3,4]);
Error, first argument must be a list of positive integers
gap> MappingPermListList([1,2], [3,4]);
(1,3)(2,4)
gap> (1,128000) = ();
false
gap> (1,128000) * (129000, 129002);
Expand Down