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

Extend the range of precomputed simple groups from orders up to 10^18 to orders up to about 10^27 #3861

Merged
merged 2 commits into from
Jan 22, 2020

Conversation

hulpke
Copy link
Contributor

@hulpke hulpke commented Jan 18, 2020

(based on work of Stefan Kohl)

This is -- as basic first option -- simply extending the precomputed list of simple groups of "small" orders from 10^18 to about 10^27

Text for release notes

The range of SimpleGroupsIterator has been extended by 9 magnitudes

@hulpke hulpke added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements kind: new feature release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Jan 18, 2020
@coveralls
Copy link

coveralls commented Jan 18, 2020

Coverage Status

Coverage increased (+0.03%) to 84.761% when pulling 734b395 on hulpke:additions into d1516f2 on gap-system:master.

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

In principle this is fine, but the documentation to SimpleGroupsIterator really should be adjusted to match the code change.

Ideally, a few more changes should be made (see my comment), but that's optional for this PR, and can be done by somebody else (e.g. me) in another PR.

grp/simple.gi Show resolved Hide resolved
@fingolfin
Copy link
Member

Also: nice patch, thank you :-)

(based on work of Stefan Kohl)
Changed documentation. Fixed also `One/AllSimpleGroup` (why are
these there?) and introduced REBASE SIMPLE_GROUPS_ITERATOR_RANGE variable
Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Nice! Minor (and optional) remarks added.

grp/simple.gi Outdated Show resolved Hide resolved
pos:=First([1..Length(SIMPLEGPSNONL2)],x->SIMPLEGPSNONL2[x][1]>=start);

if start>=10^18 then LOADSIMPLE2(); fi;
pos:=First([1..Length(SIMPLEGPSNONL2)],x->SIMPLEGPSNONL2[x][1]>=start);
Copy link
Member

Choose a reason for hiding this comment

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

Optional alternative (should be double checked ;-):

Suggested change
pos:=First([1..Length(SIMPLEGPSNONL2)],x->SIMPLEGPSNONL2[x][1]>=start);
pos:=PositionProperty(SIMPLEGPSNONL2,x->x[1]>=start);

Co-Authored-By: Max Horn <[email protected]>
@hulpke hulpke merged commit 92c951d into gap-system:master Jan 22, 2020
@Stefan-Kohl
Copy link
Member

Why 10^27? -- This bound seems a bit arbitrary to me. -- Up to order 10^30, there are 2171 non-PSL2's, up to order 10^40, there are 22709 non-PSL2's, up to order 10^50, there are 298023 non-PSL2's and up to order 10^60, there are 4263813 non-PSL2's. -- Of course listing all the latter in the way you do it now would take hundreds of megabytes of text. -- But that's the reason for the iterator code, instead of using just a list.

@hulpke
Copy link
Contributor Author

hulpke commented Jan 24, 2020

@Stefan-Kohl Yes, 10^27 is arbitary. I don't claim this is the best and end-of-all. If you want to offer your (better) general iterator code as PR, please do so, and I'd be happy to have my code replaced.

@ThomasBreuer ThomasBreuer self-assigned this Feb 17, 2021
@ThomasBreuer ThomasBreuer added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Feb 17, 2021
@ThomasBreuer ThomasBreuer removed their assignment Feb 17, 2021
@fingolfin fingolfin changed the title ENHANCE: Extended range of simple groups Extend the range of precomputed simple groups from orders up to 10^18 to orders up to about 10^27 Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants