NX endorses spec7 - Seeding Pseudo-Random Number Generation

[jarrodmillman]

We are still discussing this SPEC and decided to open this PR to have a place where we can make comments and vote. We will leave this PR open for a week or two.

[dschult]

I approved this PR for NetworkX endorsement of SPEC7 understanding that it applies only to the API for NumPy random number generators (as stated in the "Scope" part of the SPEC. The first paragraph of the SPEC seems to be much broader in scope than NumPy rngs, but the Scope section does reduce attention to NumPy which is what most of this ecosystem will be using.

I don't think the NetworkX will exactly follow the recommended API because of our use of Python's random.random as well as numpy.random and our tooling to provide a unified RNG interface for both. But I think the project endorses the ideas in this SPEC and will work toward them within our other constraints.

[bsipocz]

We got approvals here, so I think we should go ahead with the merge. Note that netlify is unhappiness is unrelated, so we should just roll with it and fix separately if it's an issue on main, too.

[rossbar]

Thanks for looking at this @MridulS and @bsipocz - it reminded me that this has been on my list to comment on for a while!

I'll just start of by saying that I too endorse SPEC 7 and think it's the right path forward for handling numpy-generated random numbers in the ecosystem.

NetworkX differs from the other libraries in that the infrastructure for supporting random numbers is designed to support both the Python builtin random and numpy.random. The user-facing bits of the machinery (i.e. the "seed" kwarg) doesn't indicate which random implementation is used, and in practice the Python random is more commonly used than numpy. Given the state of things, I feel the recommended API changes in SPEC 7 don't necessarily map cleanly onto NetworkX specifically.

Regardless - I agree that NetworkX endorses SPEC 7 but would expect that there would be more discussion within the NX community about whether the policies (particularly the API parts) would be adopted.

[seberg]

that it applies only to the API for NumPy random number generators

Yeah, I agree it does and the use of Python's rng is fine. I suppose it would be nice if:

• rng= may be a valid spelling in the future to align naming.
• rng= behaves identically for NumPy new Generators and other seeding like inputs. I don't think it is problematic if you prefer using random.Random internally. It may be nice to support SeedSequence() for a seed, but that doesn't have to match strictly IMO. One way may be to use SeedSequence but for seeding the Python rng rather than the NumPy one: seed = np.random.SeedSequence(rng).generate_state(8).tobytes(); rng = random.Random(seed). (The 8 would be 8 uint32s, so 256 bit of entropy.)

[bsipocz]

Oh, I'm sorry @rossbar for merging this prematurely.

Given the state of things, I feel the recommended API changes in SPEC 7 don't necessarily map cleanly onto NetworkX specifically.

Yes, I saw that in Dan's comment above, and that nevertheless there is a will to endorse the SPEC. I believe we are saying it from the beginning that endorsing doesn't mean to exactly do what the spec says.

[rossbar]

Oh, I'm sorry @rossbar for merging this prematurely.

No worries at all, the merge definitely wasn't premature on your end, the comment was extremely delayed on my end 🙃

Yes, I saw that in Dan's comment above, and that nevertheless there is a will to endorse the SPEC. I believe we are saying it from the beginning that endorsing doesn't mean to exactly do what the spec says.

Yes this is my understanding of the SPEC process as well - communities can endorse without necessarily adopting, or perhaps modifying adoption as best fits the need of the individual package. I'm very much +1 on endorsing SPEC 7!