Documentation of random sources

[ThomasBreuer]

When trying to provide a new kind of random sources (see oscar-system/GAP.jl/pull/642), I noticed that the documentation of GAP's random sources leaves some open questions.

1. The documentation says that any GAP random source must provide methods for the operations State, Reset, and Init. It should say that in the case of Reset and Init, methods for the two argument versions must be provided, and that the second argument may be an integer or a State value.
   In fact, the unary version of Init is documented but GAP does not provide any such method. I think the documentation should be adjusted to the code.
2. It is not specified whether Init( rs, state ) makes a copy of state. In fact, I would like to state that this is not the case in the implementation mentioned above. That is, the user must copy state before calling Init( rs, state ) if changing state in later calls Random( rs, ... ) is not wanted. I think the documentation should be adjusted to the code.
3. The code in lib/random.gi shows that Init( rs, seed ) or Init( rs, state ) is admissible also if rs is an already initialized random source, and then has the same effect as Reset with the same arguments (except of course that Init returns rs but Reset returns a copy of the old State value). I think the documentation should be adjusted to the code.
4. In my situation, negative integers aren't admissible as seeds. Perhaps the documentation should say that such restrictions may apply, depending on the type of the random source.

[fingolfin]

Actually, I don't understand why Init works like that -- it seems odd to require the user to provide a previously made object. Shouldn't we instead tell them to use RandomSource(filter, seed) ? Indeed, looking at the code, the only places calling Init in GAP are a RandomSource method and the Reset methods for IsGAPRandomSource and IsMersenneTwister. As such, it seems more like an implementation detail; or perhaps an "internal API" for people who want to implement a random source; but not something intended for end users.

I wonder why not instead RandomSource is a constructor (instead of an operation); then we could do away with Init and instead implement methods for RandomSource. This would break IsRealRandomSource from io, but otherwise I think it'd be fine? (Well, one annoyance would be that we could then not implement the one-argument variant of RandomSource generically (as I recently rediscovered, we can't have "generic" constructor methods). But this could be dealt with by having RandomSource stay as a method and instead add DeclareConstructor("RandomSourceCons", [IsRandomSource, IsObject]);

Regarding your last point, I am not sure we make promises that different seeds lead to different outputs, so one could just use AbsInt(seed)?

[ThomasBreuer]

Concerning the question why RandomSource is not a constructor, I had asked this myself when I looked at the code because of oscar-system/GAP.jl/pull/642 (see the original version of its description).

What I wrote down in this issue arose from the experience with implementing a new kind of random sources:
It would be sufficient to keep the existing code as is, and to adjust/extend the documentation.
Changing the interface would require at least some changes in IO (and in JuliaInterface).
I am not sure whether we should change/improve the interface, but if yes then better now than later.

[fingolfin]

Thomas and me discussed this orally and we agreed that it's not worth the effort to radically change the interface; instead we'll try to improve the documentation and tweak some minor details; Thomas will submit a PR for that.

[ThomasBreuer]

Since the pull request #4438 is now merged, the issue can be closed.