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

Added dtype to config in Loss. #930

Closed
wants to merge 1 commit into from
Closed

Added dtype to config in Loss. #930

wants to merge 1 commit into from

Conversation

PatReis
Copy link
Contributor

@PatReis PatReis commented Sep 20, 2023

Thank you very much for the update in Loss .

I have following question/suggestion, which I hope is okay and does not clash with the saving/loading behaviour of models: I added the dtype information to the config of Loss such that the default floatx behaviour can be maintained even when created from config. If dtype is specified, then the Loss will set dtype from config.

If the dtype should not be passed on to the config then this pull request should be canceled.

Thank you very much for the changes in `Loss`. I have following suggestion: Added the dtype information to `Loss` such that the default floatx behaviour can be maintained even when created from config. If `dtype` is specified, then the `Loss` will set dtype from config.
@PatReis
Copy link
Contributor Author

PatReis commented Sep 20, 2023

Ah I see, the subclasses and tests do not like dtype in config.

@PatReis PatReis closed this Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant