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

Add option to as_permutation_group to initialize the seed for pseudo random results via libgap #27143

Closed
soehms opened this issue Jan 27, 2019 · 12 comments

Comments

@soehms
Copy link
Member

soehms commented Jan 27, 2019

This is a follow up ticket to #26903. The aim is to realize the suggestion given there in comment 9.

Furthermore, as an example the missing doctest of that ticket should be added!

CC: @embray @tscrim

Component: group theory

Keywords: as_permutation_group, seed

Author: Sebastian Oehms

Branch/Commit: 3b3599d

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/27143

@soehms soehms added this to the sage-8.7 milestone Jan 27, 2019
@soehms
Copy link
Member Author

soehms commented Feb 6, 2019

@soehms
Copy link
Member Author

soehms commented Feb 6, 2019

comment:2

I use the new option to modify existing doctests and add a note citing the GAP documentation. Furthermore, I add the doctest concerning #26903.


New commits:

51cb6fc27143: initial version

@soehms
Copy link
Member Author

soehms commented Feb 6, 2019

Commit: 51cb6fc

@soehms
Copy link
Member Author

soehms commented Feb 6, 2019

Author: Sebastian Oehms

@tscrim
Copy link
Collaborator

tscrim commented Feb 7, 2019

comment:3

I would just pass seed to libgap.set_seed or explicitly cast it to an int or Integer (whichever is more appropriate and let the error fall out from that. IMO, the traceback would provide sufficient information and this would allow things that to convert to ZZ to work (well, at least in the latter case).

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 7, 2019

Changed commit from 51cb6fc to 3b3599d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 7, 2019

Branch pushed to git repo; I updated commit sha1. New commits:

3b3599d27143: following the reviewers suggestion

@soehms
Copy link
Member Author

soehms commented Feb 7, 2019

comment:5

The reason, why I didn't just pass it, is:

gap> Reset(GlobalRandomSource, "something");
[ 45, [ 66318732, 86395905, 22233618, 21989103, 237245480, 264566285, 240037038, 264902875, 9274660, 180361945, 94688010, 24032135, 106293216, 27264613, 126456102, 243761907, 80312412, 2522186,
      59575208, 70682510, 228947516, 173992210, 175178224, 250788150, 73030390, 210575942, 128491926, 194508966, 201311350, 63569414, 185485910, 62786150, 213986102, 88913350, 94904086,
      252860454, 247700982, 233113990, 75685846, 196780518, 74570934, 7958751, 130274620, 247708693, 183364378, 82600777, 28385464, 184547675, 20423483, 75041763, 235736203, 54265107, 49075195,
      100648387, 114539755 ] ]
gap> State(GlobalRandomSource);
"so"
gap>

I have no idea, what this should mean (this use of the argument is not described in the GAP-documentation).

But I agree to your suggestion to cast the argument!

@tscrim
Copy link
Collaborator

tscrim commented Feb 7, 2019

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Feb 7, 2019

comment:6

Thank you.

@soehms
Copy link
Member Author

soehms commented Feb 7, 2019

comment:7

Thanks, as well!

@vbraun
Copy link
Member

vbraun commented Feb 8, 2019

Changed branch from u/soehms/seed_arg_as_permutation_group_27143 to 3b3599d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants