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

Make rand_pseudo(::GAPGroup) uniformly usable #1350

Merged
merged 1 commit into from
May 23, 2022

Conversation

fingolfin
Copy link
Member

Now one can always specify radius and it has a default value

This does not address the deeper issues mentioned in #1165 but
at least it makes it a bit more usable for the moment.

Now one can always specify `radius` and it has a default value
Copy link
Member

@ThomasBreuer ThomasBreuer left a comment

Choose a reason for hiding this comment

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

What about describing the behaviour explicitly in the documentation:

    rand_pseudo(G::GAPGroup; radius::Int = 10)

Return a pseudo random element of `G`.

If `G` is an `FPGroup` then the result is a word in the generators of `G`
in the ball of radius `radius`, with equal distribution in the underlying free group.

If `G` is a finite permutation or matrix group,
`rand_pseudo` uses a variant of the product replacement algorithm.
For most inputs, the resulting stream of elements relatively quickly converges
to a uniform distribution.

The idea is that `rand_pseudo` returns elements that are cheap to compute
(cheaper than with `rand`) and somehow random,
but makes no guarantees about their uniform distribution.

It is sometimes necessary to work with finite groups that we cannot
effectively enumerate, e.g. matrix groups over finite fields. We may not even
know the size of these groups. Yet many algorithms need to sample elements
from the group "as randomly as possible", whatever that means; but also they
need this *fast*.

@fieker
Copy link
Contributor

fieker commented May 23, 2022

I would not worry with the exact description at this point, as the random interface, the idomatic julia one at least, is different anyway (see underlying issue as well). Thus I will merge to get it over and done with

@fieker fieker merged commit e5e2f52 into oscar-system:master May 23, 2022
@fingolfin fingolfin deleted the mh/rand_pseudo branch May 26, 2022 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants