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

Activation histograms #17624

Conversation

abinthomasonline
Copy link

@abinthomasonline abinthomasonline commented Mar 3, 2023

keras-team/tf-keras#118

design based on:- https://stackoverflow.com/a/70948849

does not support subclassed models

@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Mar 3, 2023
@gbaned gbaned requested a review from rchao March 3, 2023 14:31
@google-ml-butler google-ml-butler bot added the keras-team-review-pending Pending review by a Keras team member. label Mar 3, 2023
@fchollet
Copy link
Member

fchollet commented Mar 5, 2023

Thanks for the PR! It's a neat design, but it would involve adding quite a bit of complexity (overriding call methods on all layers in particular) and I'm not sure the value of the feature justifies it. It would also represent a fair amount of overhead on accelerator, especially TPU, due to the need to transfer activation values back to host memory.

I would recommend that you factor this as a standalone easily-reusable callback, rather than adding it to the TensorBoard callback (ActivationHistograms(Callback)) and that you surface it via a 3rd party distribution channel like StackOverflow or your own block post.

@fchollet fchollet closed this Mar 5, 2023
PR Queue automation moved this from Assigned Reviewer to Closed/Rejected Mar 5, 2023
@stellarpower
Copy link

Admittedly I am hotpatching the version of Keras I have installed, but, the early return here meant that just NaNs were logged:

https://github.com/abinthomasonline/keras/blob/c5a4d842a057433fb7793e5216f34da1a76953fd/keras/callbacks.py#L2850

I simply commented that out should anyone benefit from it.

I'm not sure the value of the feature justifies it.

Agree that it is a bit convoluted however it appears people are asking for this feature. It has proved valuable for me to be able to verify if the activation is becoming saturated or not when the model does not converge, and I don't think it should be difficult for the end user to do this. For me, any performance concerns are secondary to having a working model, given that this is a feature primarily used for debugging. I think adding it as an opt-in feature that won't harm the performance for any users who don't want it would be a viable idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keras-team-review-pending Pending review by a Keras team member. size:M
Projects
PR Queue
Closed/Rejected
Development

Successfully merging this pull request may close these issues.

None yet

4 participants