gap-packages / sotgrps

https://gap-packages.github.io/sotgrps/
GNU General Public License v2.0
2 stars 1 forks source link

Make IdSOTGroup much faster in many cases #26

Closed fingolfin closed 1 year ago

fingolfin commented 1 year ago

Before this, I observed this:

gap> grps:=AllSOTGroups(102576253);;time;
197
gap> for i in [1..Length(grps)] do Print(i, ": \c"); t:=Runtime(); IdSOTGroup(grps[i]); Display(Runtime()-t); od;
1: 3
2: 1
3: 1
4: 1
5: 1
6: 2
7: 2
8: 2
9: 2
10: 4
11: 2
12: 4
13: 4
14: 3
15: 3
16: 3
17: 3
18: 2
19: 14929
20: 16019
21: 16817
22: 16408
23: 16127
24: 15365
...

As a result, the CI tests timed out, because this test took many minutes:

List(AllSOTGroups(102576253),x->IdSOTGroup(x)[2]) = [1..NumberOfSOTGroups(102576253)];

With this PR, that line takes just 2.5 seconds on my machine.

fingolfin commented 1 year ago

I basically got rid of all uses of Elements. BTW that function is a synonym for AsSet. And you also use AsSet, so perhaps some of those could be inspected as well

fingolfin commented 1 year ago

oops, that was a bit of a hasty merge! The CI tests failed, i.e. something was wrong with this PR. I'll look into fixing it now but I recommend not merging changes while their cause CI failures :-)

fingolfin commented 1 year ago

Aha but the failure actually seems to not be caused by this PR, but by some other prior changes, so all is good, maybe