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

FS-1135 - Random functions for collections #732

Merged
merged 18 commits into from
May 29, 2024
Merged
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
206 changes: 206 additions & 0 deletions RFCs/FS-1135-random-functions-for-collections.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
# F# RFC FS-1135 - Random functions for collections (List, Array, Seq)

The design suggestion [Add shuffle, sample etc. methods for lists, arrays etc](https://github.com/fsharp/fslang-suggestions/issues/508) has been marked "approved in principle".

This RFC covers the detailed proposal for this suggestion.

- [x] [Suggestion](https://github.com/fsharp/fslang-suggestions/issues/508)
- [x] Approved in principle
- [ ] [Implementation]() (no implementation yet)
- [ ] Design Review Meeting(s) with @dsyme and others invitees
- [Discussion](https://github.com/fsharp/fslang-design/discussions/731)

# Summary

This feature extends collections apis with functions for random sampling and shuffling built-in fsharp collections.
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

# Motivation

This feature is motivated by the following use cases:
- Using F# for data science and machine learning (like building a neural network), where data shuffling plays important role
- Building games, where random sampling is used for generating random levels, random decks, etc.
- Building simulations, where random sampling is used for generating random input data

# Detailed design

### General

The following general rules are applied to all functions
- New functions should be implemented in List, Array, Seq modules
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved
- All functions should have a variant with a [Random](https://learn.microsoft.com/en-us/dotnet/api/system.random) parameter
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved
- Shared thread-safe Random instance should be used for all basic functions
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

### Shuffle

Shuffle function should returned a new shuffled collection of the same collection type.
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

Two functions should be added to each module.
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

```fsharp
// Array module
val randomShuffle: array:'T[] -> 'T[]
val randomShuffleWith: random:Random -> array:'T[] -> 'T[]
val randomShuffleInPlace: array:'T[] -> 'T[]
val randomShuffleInPlaceWith: random:Random -> array:'T[] -> 'T[]
// List module
val randomShuffle: list:'T list -> 'T list
val randomShuffleWith: random:Random -> list:'T list -> 'T list
// Seq module
val randomShuffle: source:'T seq -> 'T seq
val randomShuffleWith: random:Random -> source:'T seq -> 'T seq
```
[ArgumentNullException](https://learn.microsoft.com/en-us/dotnet/api/system.argumentnullexception) should be raised if collection is null
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

Example:
```fsharp
let allPlayers = [ "Alice"; "Bob"; "Charlie"; "Dave" ]
let round1Order = allPlayers |> List.randomShuffle // [ "Charlie"; "Dave"; "Alice"; "Bob" ]
```

### Choice

Choice function should returned a random element from the collection.
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

Two functions should be added to each module.
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

```fsharp
// Array module
val randomChoice: array:'T[] -> 'T
val randomChoiceWith: random:Random -> array:'T[] -> 'T
// List module
val randomChoice: list:'T list -> 'T
val randomChoiceWith: random:Random -> list:'T list -> 'T
// Seq module
val randomChoice: source:'T seq -> 'T
val randomChoiceWith: random:Random -> source:'T seq -> 'T
```
[ArgumentNullException](https://learn.microsoft.com/en-us/dotnet/api/system.argumentnullexception) should be raised if collection is null
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

[ArgumentException](https://learn.microsoft.com/en-us/dotnet/api/system.argumentexception) should be raised if collection is empty

Example:
```fsharp
let allPlayers = [ "Alice"; "Bob"; "Charlie"; "Dave" ]
let round1Order = allPlayers |> List.randomChoice // "Charlie"
```

### Choices

Choices should select N elements from input collection in random order, once element is taken it can be selected again.

Two functions should be added to each module.

```fsharp
// Array module
val randomChoices: count:int -> array:'T[] -> 'T[]
val randomChoicesWith: random:Random -> count:int -> array:'T[] -> 'T[]
// List module
val randomChoices: count:int -> list:'T list -> 'T list
val randomChoicesWith: random:Random -> count:int -> list:'T list -> 'T list
// Seq module
val randomChoices: count:int -> source:'T seq -> 'T seq
val randomChoicesWith: random:Random -> count:int -> source:'T seq -> 'T seq
```
[ArgumentNullException](https://learn.microsoft.com/en-us/dotnet/api/system.argumentnullexception) should be raised if collection is null

abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved
[ArgumentException](https://learn.microsoft.com/en-us/dotnet/api/system.argumentoutofrangeexception) should be raised if N is negative

[ArgumentException](https://learn.microsoft.com/en-us/dotnet/api/system.argumentexception) should be raised if collection is empty

Example:
```fsharp
let allPlayers = [ "Alice"; "Bob"; "Charlie"; "Dave" ]
let round1Order = allPlayers |> List.randomChoices 3 // ["Bob", "Dave", "Bob"]
```

### Sample

Sample should select N elements from input collection in random order, once element is taken it won't be selected again. N can't be greater than collection length

Two functions should be added to each module.

```fsharp
// Array module
val randomSample: count:int -> array:'T[] -> 'T[]
val randomSampleWith: random:Random -> count:int -> array:'T[] -> 'T[]
// List module
val randomSample: count:int -> list:'T list -> 'T list
val randomSampleWith: random:Random -> count:int -> list:'T list -> 'T list
// Seq module
val randomSample: count:int -> source:'T seq -> 'T seq
val randomSampleWith: random:Random -> count:int -> source:'T seq -> 'T seq
```
[ArgumentNullException](https://learn.microsoft.com/en-us/dotnet/api/system.argumentnullexception) should be raised if collection is null

abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved
[ArgumentException](https://learn.microsoft.com/en-us/dotnet/api/system.argumentoutofrangeexception) should be raised if N is greater than collection length or is negative

[ArgumentException](https://learn.microsoft.com/en-us/dotnet/api/system.argumentexception) should be raised if collection is empty

Example:
```fsharp
let allPlayers = [ "Alice"; "Bob"; "Charlie"; "Dave" ]
let round1Order = allPlayers |> List.randomSample 3 // ["Charlie", "Dave", "Alice"]
```

# Drawbacks

System.Random interface has added some new methods in [.NET 8](https://learn.microsoft.com/en-us/dotnet/core/whats-new/dotnet-8#methods-for-working-with-randomness), where naming is a bit different. More new methods can eventually be added in future .NET versions.
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

# Alternatives

Use online snippets, or provide a nuget package.
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

# Compatibility

Please address all necessary compatibility questions:
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

* Is this a breaking change? **No, unless user defined those extensions themselves**
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved
* What happens when previous versions of the F# compiler encounter this design addition as source code? **Should be fine**
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved
* What happens when previous versions of the F# compiler encounter this design addition in compiled binaries? **Should be fine**
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved
* If this is a change or extension to FSharp.Core, what happens when previous versions of the F# compiler encounter this construct? **Will work as usual**

# Pragmatics

## Diagnostics

Please list the reasonable expectations for diagnostics for misuse of this feature. **I don't see a way to misuse it**
Copy link
Member

Choose a reason for hiding this comment

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

Not sure that goes in here, but any random implementation can be valid even if it always returns the exact same value. In that case, the same item would always be returned. But since this is essentially true of anything that anybody can write in their own code, it's probably not worth mentioning.

Copy link
Contributor Author

@Lanayx Lanayx May 8, 2024

Choose a reason for hiding this comment

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

I don't know what to write here either :) So left it as is

Copy link
Member

@abelbraaksma abelbraaksma May 19, 2024

Choose a reason for hiding this comment

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

Hey @Lanayx, it appears we both forgot about this line. We shouldn't leave the remnants of the template hanging around (I know, it happens, but we shouldn't ;) ).

This is my suggestion:

Suggested change
Please list the reasonable expectations for diagnostics for misuse of this feature. **I don't see a way to misuse it**
There are no known diagnostics on any abuse or misuse of this feature.

EDIT: this is resolved (but GH doesn't let me)


## Tooling

Please list the reasonable expectations for tooling for this feature, including any of these:

* Debugging
* Breakpoints/stepping
* Expression evaluator
* Data displays for locals and hover tips
* Auto-complete
* Tooltips
* Navigation and Go To Definition
* Colorization
* Brace/parenthesis matching

**Should work just like for other collections functions**
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

## Performance

Please list any notable concerns for impact on the performance of compilation and/or generated code
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

* For existing code **Existing code should not be affected**
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved
* For the new features **Performance should be respected when implementing this feature, since it can be used in performance-sensitive scenaris**
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

## Scaling

Algorithmic complexity of the new features should be O(n) or less.
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

## Culture-aware formatting/parsing

N/A
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

# Unresolved questions

It's unclear, if more sophisticated overloads should be added:
- Weights parameter for randomChoices function
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved
- Counts parameter for randomSample function
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved

~~In .NET 6 and higher [Random.Shared](https://learn.microsoft.com/en-us/dotnet/api/system.random.shared) is available, but as soon as F# Core only targets standard, it can't use it. Is targeting higher .NET possible (not just for this feature, maybe some others need it)?
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved
Same question about new [.NET 8 apis](https://learn.microsoft.com/en-us/dotnet/core/whats-new/dotnet-8#methods-for-working-with-randomness), they could be reused in theory~~
abelbraaksma marked this conversation as resolved.
Show resolved Hide resolved