docs: Improve docstring for RMSNormalization layer#22503
docs: Improve docstring for RMSNormalization layer#22503xingzihai wants to merge 1 commit intokeras-team:masterfrom
Conversation
- Fixed misleading statement about 'scale' parameter (layer always uses scale) - Added default values for 'axis' and 'epsilon' parameters - Added more detailed description for 'axis' parameter - Added Input shape and Output shape sections - Removed redundant call method docstring for consistency with other normalization layers
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the documentation for the RMSNormalization layer. The changes aim to provide clearer, more accurate, and consistent information for users by refining parameter descriptions, adding default values, and standardizing the documentation structure to align with Keras's established style. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refines the docstrings for the RMSNormalization layer. The changes include clarifying the description of the scale parameter, adding more detailed explanations for the axis and epsilon arguments, and incorporating Input shape and Output shape sections to align with Keras documentation standards. Additionally, a redundant docstring for the call method was removed. The review feedback points out that the axis argument's type hint in the docstring should be updated to reflect its support for a list or tuple of integers, not just a single integer.
| @@ -39,7 +37,18 @@ class RMSNormalization(Layer): | |||
|
|
|||
| Args: | |||
| axis: int. The axis on which to perform the normalization. | |||
There was a problem hiding this comment.
The type hint for the axis argument is not entirely accurate. While it's often an integer, this layer also supports a list or tuple of integers for normalization over multiple axes, as shown in rms_normalization_test.py. To be more precise and align with the implementation, please update the type to reflect this.
| axis: int. The axis on which to perform the normalization. | |
| axis: int or list of ints. The axis or axes on which to perform the normalization. |
There was a problem hiding this comment.
Please address this comment.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #22503 +/- ##
=======================================
Coverage 83.20% 83.20%
=======================================
Files 596 596
Lines 67621 67621
Branches 10531 10531
=======================================
Hits 56266 56266
Misses 8630 8630
Partials 2725 2725
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hertschuh
left a comment
There was a problem hiding this comment.
Please accept the CLA.
| @@ -39,7 +37,18 @@ class RMSNormalization(Layer): | |||
|
|
|||
| Args: | |||
| axis: int. The axis on which to perform the normalization. | |||
There was a problem hiding this comment.
Please address this comment.
Summary
This PR improves the docstring for the
RMSNormalizationlayer to provide better documentation for users.Changes
Fixed misleading statement about scale parameter: The docstring previously said "If
scaleis enabled, the layer will scale the normalized outputs via a learnable scaling factor." This was misleading because the layer always uses a scale parameter (created inbuild()). Updated to clarify that the layer always scales the normalized outputs.Added default values for parameters: Added default values (
-1foraxis,1e-6forepsilon) to the Args section for clarity.Improved axis parameter description: Added more detail about the axis parameter, mentioning that it is typically the features axis and
-1is the last dimension.Added Input shape and Output shape sections: Added these sections to match the documentation style of other normalization layers (e.g., GroupNormalization).
Removed redundant call method docstring: The call method docstring was inconsistent with other normalization layers (LayerNormalization, GroupNormalization) which do not have call method docstrings. The shape description in the docstring was also inaccurate for arbitrary input shapes.
Testing
Related
This follows the Keras documentation style as described in CONTRIBUTING.md.