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

Random seed not implemented for Orthogonal Initializer #7376

Closed
cosminc98 opened this issue Feb 14, 2023 · 2 comments
Closed

Random seed not implemented for Orthogonal Initializer #7376

cosminc98 opened this issue Feb 14, 2023 · 2 comments
Labels
type:feature New feature or request

Comments

@cosminc98
Copy link
Contributor

System information

  • TensorFlow.js version: 3.17 (but it isn't implemented in any of them)
  • Are you willing to contribute it: Yes

Describe the feature and the current behavior/state.
For reference, if you try to use the Orthogonal initializer with a seed you would get the following error:
Random seed is not implemented for Orthogonal Initializer yet

Current implementations:

The current Orthogonal initializer in TFJS does not use a seed (which is needed to reproduce results across multiple training sessions) to initialize the RNG. Also, the TFJS version uses tf.linalg.gramSchmidt instead of tf.linalg.qr like the Python version (this is mostly a matter of consistency). There are two main modifications to be made:

  1. Add seeded RNG (and tests for it)
  2. Replicate the Python implementation for consistency sake.

Will this change the current api? How?
The current API does not change. The signature of the apply method stays the same. Only the implementation itself and some tests need to change.

Who will benefit with this feature?
People training models with weights initialized by the Orthogonal initializer as the method of generating the values matches the one from python (it should thus be easier to replicate results from python) and is reproducible given a fixed seed (should allow for less variability between training sessions by starting with the same weights which is useful for research / debugging).

@cosminc98 cosminc98 added the type:feature New feature or request label Feb 14, 2023
cosminc98 pushed a commit to cosminc98/tfjs-seeded-orthogonal-initializer that referenced this issue Feb 14, 2023
…d use QR decomposition method instead of gramSchmidt for consistency with the python implementation
cosminc98 added a commit to cosminc98/tfjs-seeded-orthogonal-initializer that referenced this issue Feb 14, 2023
…d use QR decomposition method instead of gramSchmidt for consistency with the python implementation
@cosminc98
Copy link
Contributor Author

A similar issue I have just discovered: #5955

cosminc98 pushed a commit to cosminc98/tfjs-seeded-orthogonal-initializer that referenced this issue Feb 15, 2023
… variables in Orthogonal Initializer apply method
cosminc98 added a commit to cosminc98/tfjs-seeded-orthogonal-initializer that referenced this issue Feb 15, 2023
… variables in Orthogonal Initializer apply method
mattsoulanille pushed a commit that referenced this issue Feb 16, 2023
…gramSchmidt (#7377)

Allow seeding the Orthogonal Initializer and add test. Use QR instead of gramSchmidt for consistency with the python implementation.
@mattsoulanille
Copy link
Member

Fixed by #7377. Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants