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

Clean up variable vs weight in base layer #521

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

mattdangerw
Copy link
Member

@mattdangerw mattdangerw commented Jul 17, 2023

Clean up docstrings to clarify variables vs weights.

@mattdangerw mattdangerw force-pushed the docstring-fix branch 3 times, most recently from 7c6ece2 to fc8c70c Compare July 17, 2023 21:10
@fchollet
Copy link
Member

Not quite! We still intend for our users to use add_weights, trainable_weights, set_weights, etc. as their primary accessor for weights and their values. Though some users (mainly JAX stateless API users) will resort to variables as their go-to.

There's a (subtle) difference between variables and weights.

Most users won't care about the difference or won't understand it -- so they should generally use weights because it will match their expectations.

Weights are variables that parameterize the forward pass. They are the parameters of the manifold learned by the model. For instance, a Conv kernel, or the BN statistics are weights.

Variables are more general than that. They include weights, but also various side-variables that aren't weights, such as RNG seeds, metric state variables (if any metric is attached), and possibly some metadata variables like counters.

You will use variables if you are referring to the "state" of a layer in general, as you would in JAX.
You will use weights if you are referring to forward pass parameters.

@mattdangerw
Copy link
Member Author

Thanks! I still think there are a few cleanups to be made here, but good to know the distinction! I will rework this.

@mattdangerw mattdangerw force-pushed the docstring-fix branch 2 times, most recently from c9d4558 to 2b2b7d4 Compare July 17, 2023 22:46
@mattdangerw
Copy link
Member Author

OK! Update this to be a pure docstring fix. Tried to clarify a little more, and non_trainable_weights no longer claims to contain metrics and RNG state, which seems false.

Overall, this still feels a little confusing. We refer to weights and variables interchangeably for the most part. layer.add_weight() == layer.add_variable() == layer = backend.Variable(). Then we draw a distinction in one context. I might actually find it easier to grok if we had layer.trainable_state, layer.non_trainable_state and layer.stateless_call, because the API would align around a new term "state". weights is all Variables used by layers and sub-layers, state is all Variables used by layers, metrics and generators. But that is probably too disruptive of a change right now.

Do we want layer.get_variables() and layer.set_variables() as an analogue to layer.get_weights() and layer.set_weights()?

@fchollet
Copy link
Member

Do we want layer.get_variables() and layer.set_variables() as an analogue to layer.get_weights() and layer.set_weights()?

I would say, probably not for now. We can revisit later though.

Copy link
Member

@fchollet fchollet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this pretty confusing. Anyway, thanks for the fixes!

@fchollet fchollet merged commit 0416ab9 into keras-team:main Jul 17, 2023
adi-kmt pushed a commit to adi-kmt/keras-core that referenced this pull request Jul 21, 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.

2 participants