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 a better story for split #97

Closed
treeowl opened this issue Jan 31, 2021 · 3 comments · Fixed by #160
Closed

Make a better story for split #97

treeowl opened this issue Jan 31, 2021 · 3 comments · Fixed by #160

Comments

@treeowl
Copy link

treeowl commented Jan 31, 2021

I think we should look to Data.Bits for inspiration here. Add a class

class RandomGen g => SplittableRandomGen g where
  safeSplit :: g -> (g, g)

safeSplit is the same as split, except it should only be defined for generators that are actually splittable.

Question: can we improve efficiency or flexibility for any mutable generators by offering something like this?

class StatefulGen g m => SplittableStatefulGen g m where
  splitStatefulGenM :: g -> m g

I imagine this might be able to avoid some unnecessary copying in some cases.

@treeowl
Copy link
Author

treeowl commented Jan 31, 2021

It looks like this was discussed in idontgetoutmuch#7 but for reasons I don't know was rejected there? Doesn't make sense to me.

@treeowl
Copy link
Author

treeowl commented Jan 31, 2021

FYI: I want to play with splittable generators specifically for incremental sorting of sequences (for Data.Sequence).

@lehins
Copy link
Contributor

lehins commented Feb 3, 2021

@treeowl Yes we did discuss this approach at lengths when working on random-1.2.0, but we decided not to tackle this problem for that version, since there was already tremendous amount of changes being introduced and we wanted to keep breakage to a minimum.

Here are my comments on your suggestion:

  • SplittableStatefulGen is not useful at all, because only pure PRNGs that have instance of RandomGen are capable of splitting. For that reason I prefer to use RandomGenM class instead and use the approach that is in the works in WIP: Improve RandomGenM interface: #93
  • class RandomGen g => SplittableRandomGen g where:
    • I really don't like the name SplittableRandomGen, I think SplittableGen would be nicer
    • renaming split -> safeSplit would introduce more work on the users side, it is better to migrate split into SplittableGen right away.
    • Introducing new class would mean that we must go through all PRNG libraries and create that instance, which is problematic, since majority of them are unmaintained.

I thought about this issue quite a bit before, because I also share your dislike of partial functions, and I think I came up with slightly less invasive solution to this problem in #94, please let me know what you think.

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

Successfully merging a pull request may close this issue.

2 participants