Use RandomState instead of global random seed in Orthogonal initializer

[vikua]

Summary

np.random.seed modifies global seed that other packages or programs where Keras is being imported into have access to. This may lead to confusion and errors.

Related Issues

PR Overview

It looks like seeding random globally is not required in Orthogonal initializer.
Using np.random.RandomState instead of global np.random.seed to generate normal distribution.

• This PR requires new unit tests [y/n] (make sure tests are included)
• This PR requires to update the documentation [y/n] (make sure the docs are up-to-date)
• [y] This PR is backwards compatible [y/n]
• This PR changes the current API [y/n] (all API changes need to be approved by fchollet)

[gabrieldemarmiesse]

np.random.seed modifies global seed that other packages or programs where Keras is being imported into have access to. This may lead to confusion and errors.

I'm not sure I understand.

If we follow your reasonning, which packages should use np.random.seed and which packages shouldn't?

[fchollet]

LGTM, thanks for the PR!

[fchollet]

I'm not sure I understand.

Ideally, calling black-box Keras functions shouldn't modify the randomness behavior of downstream calls to Numpy that are unrelated to Keras. So we should avoid changing the global seed if possible.

[gabrieldemarmiesse]

Ok that makes sense.

[vikua]

@fchollet PR is green now.
The change in accuracy in test_temporal_classification_functional can be explained by the fact that previously orthogonal initializer was using random state after it has been modified by get_test_data (and maybe something else), but now it uses fresh random state.

[fchollet]

If I'm not mistaken, this PR would have the unintended consequence that people using np.random.seed at the start of their program for determinism would no longer have deterministic initializers, right?

[vikua]

Yes, I'm afraid this is true.

[fchollet]

I think we should prioritize this use case over "Keras should not modify the global random state".

I think it might be possible to have both, if we use the global random state to generate a local one?

[vikua]

Interesting idea, let me try that out.

[vikua]

Yeah, the original test passes as before with global random seed. Also added another test to verify initializer with local random seed.

[YuriyGuts]

I have a very similar PR that also removes global seeding from the dataset readers, as well as the orthogonal initializer. Sorry I haven't discovered this PR earlier because I looked for this issue in the tickets rather than PRs. #12259

[vikua]

Closing, reason is -> #12259 (comment)