SE-0202 Amendment Proposal: Rename Random to DefaultRandomNumberGenerator

Personally I like:

public enum RandomNumberGenerators {}
public extension RandomNumberGenerators {
    private struct Default: RandomNumberGenerator { ... }
    static defaultSharedBaseGenerator = Default()
}

Then I could write:

struct MyType {
    func random<R>(using rng: inout R = &RandomNumberGenerators.defaultSharedBaseGenerator) -> MyType where R: RandomNumberGenerator { ... }
}

Which I think:

  1. Reads well.
  2. Allows other generators too be added, to RandomNumberGenerators, in the future.
  3. Will discourage people from calling the generator directly.

I’m still strongly in favour of the Random namespace. I don’t understand why people think that using Random.default directly is a rare use case — the proposal was designed to be extensible and usable on your own types.

If you still decide to go with this unnecessarily long name, please at least drop the Default, and name it RandomNumberGenerator.default.

2 Likes

Note however that when using a PRNG, you'd probably want to create your own seeded instance and use that:

// Somewhere in your code you have something like this:
var prng = MersenneTwister(seed: 1234) // Or you might leave out seed and it will choose a random one.

// And you'd use it like this:
let a = Int.random(in: 0 ... 100, using: &prng)
let b = Float.random(in: 0 ..< 1, using: &prng)

Having a public static shared instance of a [u]P[/u]RNG would be like promoting the idea of mindlessly using a shared mutable state, with all the problems that comes with that, in a case where is is essential that the user has a clear idea of how they are using the shared state of their PRNG instances. The same is not true for the current default RNG, as it has no state (it's only wrapping a function call) and it'is not seedable (the sequence of bits it generates is not determined by a given seed).

IMO, in the case of PRNGs, where the user wants repeatable results given a specific seed, the API should instead promote the idea that it is important that the user has a clear understanding of their use of various PRNG instances, as these instances will be shared mutable states (no matter if they are instances of classes or structs passed as inout). This is another possible problem of the current API, as the decision to use structs instances passed as inout (instead of class instances), might hide the reference semantics-like behavior we get (and want) by passing them as inout.

5 Likes

For the benefit of people that weigh-in on this amendment review, but didn’t have time to participate in previous discussions, let’s...

Recap:

Suggestion for DefaultRandomNumberGenerator first appears on SE-0202 proposal review:

No reason given.

Second mention down-thread:

No reason given.

Third mention further down-thread:

This is in a context of discussing the implementation details with regards to thread-safety. @Joe_Groff was replying with possible workaround to specific memory exclusivity concerns raised by @nnnnnnnn.

Fourth time down-thread was after the proposal acceptance decision:

The consesus acceptance claim is quite the misrepresentation, given the context I provided above.

Moving on to the amendment pitch phase:

Reason given for renaming: promotion of other than default implementations. (?) That’s non sequitur.
The author of SE-0202, @Alejandro, clarifies the original vision in his reply:

Followed by the amendment proposal author:

@Alejandro follows up by detailed explanation for the current SE-0202 design based on performance considerations and supports the original naming with thorough term-of-art justification from other languages:

It is noted that even though Swift’s in-the-wild practice is to use empty enums as de-facto namespaces (due to them currently missing from the language), it’s just a convention and technically structs can play this role as well.

:man_shrugging: The definition of Bikeshedding?

There seemed to be mild consensus forming on the topic of splitting the default implementation struct and namespace enum. This is not the subject of this amendment proposal.

1 Like

I think the naming-issue is just a symptom of the lack of clear vision for the structure of the Random API, and perhaps some misunderstandings about RNG vs PRNG.

I know that this is written by the author of SE-0202, but in my opinion, the intention expressed in the above quote is confusing and unwise for two reasons:

  1. Is Random meant to be "The default source of random data." as stated in the current documentation of Random, or is it meant to be a "namespace" for various Random API related things such as generators, as the above quote seems to suggest?

  2. See my above post about why having a "globally initialized instance" of a PRNG (which are, by definition, seedable and deterministic) is the same as promoting a mindless use of shared mutable state, and the problems that go with it, instead of signaling that you'd probably want to care very much about how and why you create and use these shared mutable states, ie PRNG instances.

To be clear, I have no qualms about making a namespace like Random which would house properties like default. What I do not like is the default random number generator object being named Random itself. It's just the wrong name. At the very least, it should be called RandomGenerator to actually describe what the object is. Random isn't descriptive of anything, it's at best poor shorthand. It refers to the domain of randomness rather than what this particular type is.

Let's imagine we introduced a Random namespace or submodule (something that was never formally proposed or agreed on in SE-202) but clearly has many people desiring it. We would still need a name for the stdlib generator. You could conceivably imagine:

enum Random { // namespace
    var `default`: DefaultRandomNumberGenerator { ... }
}

I personally would love to hang a default instance accessor off of the RandomNumberGenerator protocol type itself, so you could declare something like func random(using generator: RandomNumberGenerator = .default). Again, this requires a fully-formed name for the type vended.

extension RandomNumberGenerator.Type  { // this is obviously hypothetical syntax of a feature that is nowhere close to implementation currently
      static var `default`: DefaultRandomNumberGenerator { ... }
}

The mere fact that people keep proposing the idea of Random as a 'namespace' is indicative that Random is not the correct name for a RNG source struct. If this proposal was already out in the wild, part of the public Swift language, I would agree it is not so harmful to be worthy of renaming and code churn. But pointedly, it is not yet finalized API. There are no source compatibility concerns at this stage.

2 Likes

I’ve tried to stay as impartial as I could muster in the recap above. More of my opinion here.

It seems one argument behind amendment is that the length of the proposed rename doesn’t matter, because no one should be typing that, which is a result of the core part of SE-0202: to get a number, you should use Int.random(in: 0 ..< 10).

The other argument is that

...which implies that you would have to come across this type somehow, so that it can promote the full breadth of your options by stunning you with it’s impractical length.

If you use logic, you can not have it both ways! One of these two arguments is clearly invalid. But there is a good point about the autocomplete. It’s reasonable to expect that Swift users will discover about the SE-0202 additions this way, which is why maintaining Random in the current form is so important. In addition to being logical future extension point (for users and stdlib alike), it is superior place to house all the relevant documentation than on the RandomNumberGenerator protocol.

I’m assuming that before the release, the :bat::man:—ups, I mean @nnnnnnnn—will come in and fill the Random with examples of rolling you own, well distributed an unbiased, PRNG implementations inside the documentation comment, as is the great Swift tradition. :stuck_out_tongue_winking_eye:

I’m not against the naming discussions per se, but I don’t understand how this amendment made it past the pitch and into the review phase, as I can’t find here any merit beyond bikeshedding. Much more salient naming issue was raised during the amendment pitch:

Learning the lesson from C++, our protocol should be named RandomBitGenerator, to clearly label it as the source of randomness, that is one step removed from the act of generating numbers (done on the numeric types). That should distance the two concepts and address the concern raised earlier:

To reiterate, I see absolutely no “immediate need to rename” Random supported with evidence and logical reasoning. Anyone suggesting this should first demonstrate the harm being done. Next they need to demonstrate how mere renaming avoids said harm. In other words: prove that this is not another bikeshedding session, especially in the light of thorough and very productive pitch phase on SE-0202, where the design evolved quite a bit and cristalized into well though out final accepted proposal! As a personal heuristic, when the “solution” is type name composed of 4 or more words, I suggest you check yourself for unconscious bias... maybe you haven’t yet fully internalized Swift’s naming guidelines (“Omit needles words”) and are just bikeshedding instead of solving a real issue.

2 Likes

The struct Random is a namespace and an extension point. As is any type in Swift. It was introduced to provide a facade that hides the default platform provided source of randomness behind the .default property. It is there to be accessed as default source of randomness for the higher level random number APIs, that use it as the default value for the full method signature with the inout using: argument. There is no mandated implementation inside the struct Random. Is this the source of your confusion?

EDIT: If I understand the implementation correctly, we could swap struct for enum today without it changing anything. See below.

Random conforms to the RandomNumberGenerator protocol. Random.default is an instance of Random. It's not a dumb namespace, it's a type.

1 Like

Yes. I’m assuming this is an implementation detail motivated by performance considerations, that were raised as the design evolved during the pitch. I’m speaking about the type’s semantics. Any type in Swift is a namespace, because nesting types inside other types is the only namespacing technique we currently have in Swift.

The current Random struct is the type of the "default random number generator", the current static default property is the "default instance" of this type. What do you mean that there is "no implementation" inside the struct Random? From the implementation:

/// The default source of random data.
/// ...
public struct Random : RandomNumberGenerator {
  /// The default instance of the `Random` random number generator.
  public static var `default`: Random {
    get { return Random() }
    set { /* Discard */ }
  }

  private init() {}

  /// Returns a value from a uniform, independent distribution of binary data.
  ///
  /// - Returns: An unsigned 64-bit random value.
  @effects(releasenone)
  public mutating func next() -> UInt64 {
    var random: UInt64 = 0
    _stdlib_random(&random, MemoryLayout<UInt64>.size)
    return random
  }

  public mutating func _fill(bytes buffer: UnsafeMutableRawBufferPointer) {
    if !buffer.isEmpty {
      _stdlib_random(buffer.baseAddress!, buffer.count)
    }
  }
}

If changing Random to an enum, how would you implement the static default property, the "default instance of the Random random number generator"?

Sorry for the confusing formulation. I mean SE-0202 does not specify the implementation beyond the declaration:
public mutating func next() -> UInt64
and the rest is platform specific implementation detail, not a part of SE-0202. I was trying to limit the discussion to the design part, that is appropriate for Swift Evolution, to keep it separate from the implementation discussion that happens in GitHub during pull request review.

From SE-0202, section "Proposed Solution", subsection "Random API" (not eg "Detailed Design"):

Then for the stdlib's default RNG implementation, introduce a new struct named Random. This struct contains a singleton called default which provides access to the methods of RandomNumberGenerator.

That is, the Random struct is clearly described as "the default random number generator implementation".

I see your point that it isn’t possible to simply swap struct for enum because than there is no way to create an instance. Point taken.

I think the SE-0202 part you are quoting clearly describes Random.default as a facade for hiding the platform specific implementation. The need to provide the instance for default argument, leads to this being a struct.

This seems orthogonal to the namespacing role this type can play. Analogous to the customary use of enum for namespacing, this struct can serve this role as well. There is no technical reason for splitting the implementation into enum Random and DefaultRandomNumberGenerator to get the namespacing benefits.

The proposed amendment entirely removes this role, because the new name is unwieldy and purposefully not general, but doesn’t explain how the original, simple and generic name Random leads to harm. How can this be misused?

It boils down to this:

How does the fact that Random serves 2 roles, facade for default randomness and a namespace, lead to problems that having it split into 2 would avoid? Please note that the amendment was not about doing that, but only about renaming the type, destroying its second use as a namespace.

I can see this being a minor quibble about the elegance of the implementation details. But Random.default is just the facade for the platform-specific, system provided, source of randomness. The struct Random itself doesn’t contain this implementation, it merely forwards to it. How does renaming it solve anything?

We can argue about SE-0202 not outlining the vision for using Random as namespace in the Future Directions section in more detail. We can quibble about it over-specifying minor implementation details (Random implementing RandomNumberGenerator itself). Let these be lessons learned for future proposal authors.

This doesn’t change the fact that Random is a namespace, it currently contains single facade property that provides access to the default source of randomness: Random.default (and this combined name is as clear as it gets). Random can be extended in the future by standard library to provide further built-in functionality and provides an extension point for users to surface parts of their custom (P)RNGs. I believe this should become the main documentation node for all the goodies introduced by the SE-0202.

1 Like

Just to clarify for myself. This means that using next() on the Iterator of a 'normal' Sequence like this [...] is a completely legitimate pattern

It really depends on what you mean by "legitimate".

If I saw someone writing that exact code, as you have it, in a code review, I would tag it as needing to be changed to a for...in loop before merging. It is clearly incorrect from a code style POV, even though the code is "correct" in that it will produce the right output.

Calling Random.default.next() is the same. It is possible the user wants UInt64.random(in: .min ... .max), and this would be a technically correct way to achieve the same thing. But the latter form is explicit about this being the intent. The former invites a suspicion that the user really wants something else (say, they really wanted an Int, or they subsequently narrow the range). As ever, encouraging clarity at the point of use is our primary goal in naming things. The API as it currently stands invites lack of clarity.

Worse, it encourages misuse. I fully expect that if Random.default remains in the language, then you will immediately start seeing something like the following appear in code:

let i = Int(Random.default.next() % n)

Unlike the other examples, this code is explicitly incorrect. It has a bug – a subtle one regarding modulo bias that most users will never know about.

Just search GitHub for occurrences of "arc4random %" to see this play out in practice. Autocomplete encourages people to use the first thing they stumble over. Despite arc4random_uniform existing, people discover arc4random first, and then improvise what they want based on that. With this API, people will type Random, spot the default property (hey, the default is supposed to be what you use, like with other APIs, right?), then roll with it, despite the higher-level APIs being a better choice.

As I mentioned earlier, the extent to which people are objecting to the longer name in this thread being something that will look bad in code really does lead me to think that the name Random.default is highly misleading, in that it suggests this is something you should use actively in code.

Brent's comments about how the longer name will make already-verbose definitions of random generation of user-defined types is a good one, however this is still to my mind a small problem compared to the great risk of encouraging thousands of programmers to use the API incorrectly in client code.

6 Likes

This problem could be corrected by DOCUMENTING THE API. We really should stop this madness of "people will know what to use" - they don't if we don't tell them how an API is supposed to be used. I propose a forced documentation part for any proposal to the language and stdlib. Anything missing this documentation would automatically not be approved. So docs first.

3 Likes

I have to agree with this sentiment. I am even more inclined to believe that we should just not provide a default in the standard library at all then. Let’s just rip it out and have people roll their own. Adding a longer name will not stop people from using the ways you describe. I for one would YOLO use what ever is provided to me by the standard library. I am not trolling here. If the concern of misuse is real then lets leave up to the user.

Sorry for the offtopic question, but as a newbie, I have to ask: what's the purpose of all this discussion? Who makes the ultimate implementation decision, and is that person participating in this thread? (If not, what are we doing here?)

2 Likes

@sbromberger, you can read about the Swift evolution process and the core team, and note that @Ben_Cohen is also the review manager of SE-0202.

1 Like

Thanks, @benrimmington - those links are very helpful. Again, speaking from inexperience with the Swift process, I'm surprised at the amount of bikeshedding I see here (not just this thread, but the one on trim() as well). In other environments I'm familiar with, the bias is towards implementation and refinement as opposed to refinement first.

I don't know whether one approach is inherently better than another, but I note that the reason I'm using Swift in the first place is because Go has been debating generics for so long without implementing something that it's impacted their ability to be a candidate for some groundbreaking research (TensorFlow) developed within the same company. I worry about this when I see lengthy discussion of what should be easily-reversed design decisions that appears to delay actual implementation that can benefit end users like me.

Sorry if this comes off as a complaint; it's not really – just the musings of a newcomer. I'm done derailing this thread and hope we can get to implementation soon.

1 Like