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

Semigroups test crashes if all packages are loaded. #543

Closed
olexandr-konovalov opened this issue Sep 28, 2018 · 16 comments
Closed

Semigroups test crashes if all packages are loaded. #543

olexandr-konovalov opened this issue Sep 28, 2018 · 16 comments
Labels
bug Label for issues or PR which report or fix bugs resolved-pending-release A label for issues that are resolved pending a release.

Comments

@olexandr-konovalov
Copy link
Contributor

As can be seen at https://travis-ci.org/gap-system/gap-docker-pkg-tests-stable-4.10/jobs/433451928, it first goes into some errors, and then suddenly terminates.

This is the test using stable-4.10 branch and http://www.gap-system.org/pub/gap/gap4pkgs/packages-required-stable-4.10.tar.gz packages archive.

Note that this is a new test for Semigroups - previously the package had a single .tst file, and it recently switched to a more comprehensive standard tests, so we had no chance to observe this before.

testing: /home/gap/inst/gap-stable-4.10/pkg/semigroups-3.0.19/tst/standard/fre\
einverse.tst
######## > Diff in:
/home/gap/inst/gap-stable-4.10/pkg/semigroups-3.0.19/tst/standard/freeinverse.\
tst:91
# Input is:
for i in [1 .. 10] do
NextIterator(iter);
od;
# Expected output:
# But found:
Error, Record Element: '<rec>.word' must have an assigned value
########
######## > Diff in:
/home/gap/inst/gap-stable-4.10/pkg/semigroups-3.0.19/tst/standard/freeinverse.\
tst:94
# Input is:
NextIterator(iter);
# Expected output:
a*b
# But found:
Error, Record Element: '<rec>.word' must have an assigned value
########
######## > Diff in:
/home/gap/inst/gap-stable-4.10/pkg/semigroups-3.0.19/tst/standard/freeinverse.\
tst:101
# Input is:
for i in [1 .. 1000] do 
NextIterator(iter);
od;
# Expected output:
# But found:
Error, Record Element: '<rec>.word' must have an assigned value
########
######## > Diff in:
/home/gap/inst/gap-stable-4.10/pkg/semigroups-3.0.19/tst/standard/freeinverse.\
tst:106
# Input is:
for i in [1 .. 100] do 
Add(list1, NextIterator(iter));
od;
# Expected output:
# But found:
Error, Record Element: '<rec>.word' must have an assigned value
########
######## > Diff in:
/home/gap/inst/gap-stable-4.10/pkg/semigroups-3.0.19/tst/standard/freeinverse.\
tst:111
# Input is:
for i in [1 .. 100] do 
Add(list2, NextIterator(iter));
od;
# Expected output:
# But found:
Error, Record Element: '<rec>.word' must have an assigned value
########
######## > Diff in:
/home/gap/inst/gap-stable-4.10/pkg/semigroups-3.0.19/tst/standard/freeinverse.\
tst:148
# Input is:
x := NextIterator(iter);;
# Expected output:
# But found:
Error, Record Element: '<rec>.word' must have an assigned value
########
######## > Diff in:
/home/gap/inst/gap-stable-4.10/pkg/semigroups-3.0.19/tst/standard/freeinverse.\
tst:149
# Input is:
for i in [1 .. 10] do
y := x;
x := NextIterator(iter);
MinimalWord(x * y);
od; 
# Expected output:
# But found:
Error, Record Element: '<rec>.word' must have an assigned value
########
#I  Errors detected while testing
@wilfwilson
Copy link
Collaborator

Thank you @alex-konovalov, this was reported in #532 and was fixed in #540, and so these diffs should not occur once the next release of Semigroups is made.

(However, once these diffs are fixed, I think it is possible that diffs will appear in tst/standard/prop.tst: this file is not currently being run in your system, because the testing aborts after the file tst/standard/freeinverse.tst produces the diffs above, and fails).

@james-d-mitchell
Copy link
Collaborator

Thanks @alex-konovalov, I think the "sudden termination" is a feature, not a bug, that the tests stop running when an error is encountered.

@james-d-mitchell james-d-mitchell added duplicate Label for issues or PR that are duplicates of others bug Label for issues or PR which report or fix bugs 3.* labels Sep 28, 2018
@james-d-mitchell
Copy link
Collaborator

This is a duplicate of Issue #532

@wilfwilson
Copy link
Collaborator

wilfwilson commented Sep 28, 2018

Problem: when I reproduce Alex's tests, but with the stable-3.0 branch of Semigroups, I still have one diff (which is different from above):

testing: /Users/Wilf/gap-stable-4.10/pkg/semigroups/tst/standard/freeinverse.t\
st
######## > Diff in:
/Users/Wilf/gap-stable-4.10/pkg/semigroups/tst/standard/freeinverse.tst:94
# Input is:
NextIterator(iter);
# Expected output:
a*b
# But found:
b^-1*a^-1
########
#I  Errors detected while testing

@james-d-mitchell
Copy link
Collaborator

It's this caused by the different order in which RWCA spits out elements of the free group? @wilfwilson

@wilfwilson
Copy link
Collaborator

When running tst/standard/properties.tst, there are also diffs:

testing: /Users/Wilf/gap-stable-4.10/pkg/semigroups/tst/standard/properties.tst
######## > Diff in:
/Users/Wilf/gap-stable-4.10/pkg/semigroups/tst/standard/properties.tst:795
# Input is:
IsInverseSemigroup(S);
# Expected output:
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 2nd choice method found for `CayleyGraphDualSemigroup' on 1 argument\
s
# But found:
Error, resulting list would be too large (length infinity)
########
######## > Diff in:
/Users/Wilf/gap-stable-4.10/pkg/semigroups/tst/standard/properties.tst:1109
# Input is:
IsOrthodoxSemigroup(S);
# Expected output:
true
# But found:
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `IsSemigroupWithClosedIdempotents' on 1 \
arguments
########
######## > Diff in:
/Users/Wilf/gap-stable-4.10/pkg/semigroups/tst/standard/properties.tst:1155
# Input is:
IsOrthodoxSemigroup(S);
# Expected output:
false
# But found:
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `IsSemigroupWithClosedIdempotents' on 1 \
arguments
########
######## > Diff in:
/Users/Wilf/gap-stable-4.10/pkg/semigroups/tst/standard/properties.tst:1159
# Input is:
IsOrthodoxSemigroup(FullTransformationMonoid(3));
# Expected output:
false
# But found:
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `IsSemigroupWithClosedIdempotents' on 1 \
arguments
########
######## > Diff in:
/Users/Wilf/gap-stable-4.10/pkg/semigroups/tst/standard/properties.tst:1802
# Input is:
IsRectangularGroup(S);
# Expected output:
true
# But found:
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `IsSemigroupWithClosedIdempotents' on 1 \
arguments
########
######## > Diff in:
/Users/Wilf/gap-stable-4.10/pkg/semigroups/tst/standard/properties.tst:1805
# Input is:
IsRectangularGroup(S);
# Expected output:
true
# But found:
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `IsSemigroupWithClosedIdempotents' on 1 \
arguments
########
######## > Diff in:
/Users/Wilf/gap-stable-4.10/pkg/semigroups/tst/standard/properties.tst:1807
# Input is:
IsRectangularGroup(PartitionMonoid(3));
# Expected output:
false
# But found:
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `IsSemigroupWithClosedIdempotents' on 1 \
arguments
########
#I  Errors detected while testing

@wilfwilson
Copy link
Collaborator

wilfwilson commented Sep 28, 2018

Using the RCWA iterator:

gap> for i in [1 .. 20] do Print(NextIterator(iter), "\n"); od;
<identity ...>
f1^-1
f1
f2^-1
f2
f1^-2
f1^-1*f2^-1
f1^-1*f2
f1^2
f1*f2^-1
f1*f2
f2^-1*f1^-1
f2^-1*f1
f2^-2
f2*f1^-1
f2*f1
f2^2
f1^-3
f1^-2*f2^-1
f1^-2*f2

Using the GAP iterator:

gap> for i in [1 .. 20] do Print(NextIterator(iter), "\n"); od;
<identity ...>
f1
f1^-1
f2
f2^-1
f1^2
f1^-2
f2*f1
f2^-1*f1
f2*f1^-1
f2^-1*f1^-1
f1*f2
f1^-1*f2
f1*f2^-1
f1^-1*f2^-1
f2^2
f2^-2
f1^3
f1^-3
f2*f1^2

Yes, these are different. So we should not rely on the iterator returning things in a particular fixed order if we're using a free group iterator.

@olexandr-konovalov
Copy link
Contributor Author

@wilfwilson ah, probably I did not see diffs in tst/standard/properties.tst, because the test was terminated earlier. Maybe not so good setting after all - wouldn't it be better to run the whole test?

RCWA is known for iterator returning objects in a different order. Could your test be fixed to not to rely on it?

@wilfwilson
Copy link
Collaborator

wilfwilson commented Sep 28, 2018

All of the problems (except for one) in tst/standard/properties.tst are caused by the following:

gap> S := PartitionMonoid(3);;
gap> ApplicableMethod(IsOrthodoxSemigroup, [S], true);
#I  Searching Method for IsOrthodoxSemigroup with 1 arguments:
#I  Total: 4 entries
#I  Method
1: ``IsOrthodoxSemigroup: system getter'' at /Users/Wilf/gap-stable-4.10/lib/semigrp.gd:
668, value: 2*SUM_FLAGS+18
#I   - 1st argument needs [ "HasIsOrthodoxSemigroup" ]
#I  Method
2
 : ``IsOrthodoxSemigroup: for a semigroup'' at /Users/Wilf/gap-stable-4.10/pkg/smallsemi-0.6.11\
/gap/properties.gi:725, value: 82
#I  Function Body:
function ( s )
    return IsSemigroupWithClosedIdempotents( s ) and IsRegularSemigroup( s );
endfunction( s ) ... end

So, when all packages are loaded in Alex's setup, the smallsemi method for IsOrthodoxSemigroup is the one that applies to a semigroup, rather than the Semigroups package one. Since Semigroups semigroups don't have an applicable method for IsSemigroupWithClosedIdempotents, there is an error. It seems like IsOrthodoxSemigroup is the only method in smallsemi/gap/properties.gi to have the filter IsSemigroup rather than IsSmallSemigroup. So is this perhaps a bug in smallsemi: i.e. should the filter on small/gap/properties.gi:725 be IsSmallSemigroup @james-d-mitchell?

The other diff in properties.tst is a harmless slightly different no-method-found thing being produced, essentially. I'll maybe just remove this test since it's just a code coverage thing.

@james-d-mitchell
Copy link
Collaborator

I can change that

@james-d-mitchell
Copy link
Collaborator

Or we could just implement IsSemigroupWithClosedIdempotents

@james-d-mitchell
Copy link
Collaborator

This is a separate issue maybe?

@wilfwilson
Copy link
Collaborator

I'm not sure it's a separate issue; it's the cause of Semigroups package failing tests when all packages are loaded.

Or we could just implement IsSemigroupWithClosedIdempotents

Note that IsSemigroupWithClosedIdempotents is declared in smallsemi, so we would have to move that declaration to the GAP library if we wanted to avoid:

#I  method installed for IsSemigroupWithClosedIdempotents matches more than one declaration

when loading Semigroups and smallsemi. But that seems like a pain.

I have now avoided this issue completely in #544 by making sure that the Semigroups IsOrthodoxSemigroup method always applies.

@james-d-mitchell
Copy link
Collaborator

Ok, agreed!

@wilfwilson
Copy link
Collaborator

This is resolved by #544, which will appear in the next release of Semigroups

@james-d-mitchell james-d-mitchell removed the duplicate Label for issues or PR that are duplicates of others label Sep 29, 2018
@james-d-mitchell james-d-mitchell added the resolved-pending-release A label for issues that are resolved pending a release. label Oct 1, 2018
@james-d-mitchell
Copy link
Collaborator

Resolved in v3.0.20.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Label for issues or PR which report or fix bugs resolved-pending-release A label for issues that are resolved pending a release.
Projects
None yet
Development

No branches or pull requests

3 participants