-
Notifications
You must be signed in to change notification settings - Fork 523
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
feat: support seed for pt/dp models #3773
Conversation
WalkthroughWalkthroughThe recent updates in the Changes
Possibly related issues
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (4)
Files skipped from review as they are similar to previous changes (2)
Additional comments not posted (11)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Signed-off-by: Duo <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
Out of diff range and nitpick comments (6)
deepmd/pt/model/descriptor/se_a.py (2)
Line range hint
85-103
: Ensure proper documentation for the newseed
parameter in the constructor.It's important to maintain comprehensive documentation, especially for public APIs. Consider adding a docstring entry for the
seed
parameter explaining its purpose and usage.
Line range hint
333-404
: Ensure proper documentation for the newseed
parameter in the constructor.Similar to the previous class, adding a docstring entry for the
seed
parameter would help users understand its purpose and how it affects the initialization of the descriptor.deepmd/dpmodel/utils/network.py (4)
81-87
: Ensure proper documentation for the newseed
parameter in the constructor.Adding a docstring entry for the
seed
parameter would help users understand its purpose and how it affects the initialization of the layer weights and biases.
Line range hint
317-330
: Ensure proper documentation for the newseed
parameter in the constructor.As with other classes, adding a docstring entry for the
seed
parameter inLayerNorm
would clarify its role in the initialization process.
Line range hint
575-590
: Ensure proper documentation for the newseed
parameter in the constructor.Documentation for the
seed
parameter inEmbeddingNet
is crucial for users to understand its impact on network initialization.
Line range hint
677-697
: Ensure proper documentation for the newseed
parameter in the constructor.Adding a docstring entry for the
seed
parameter inFittingNet
would help users understand its purpose and how it affects the initialization of the network.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #3773 +/- ##
===========================================
- Coverage 82.58% 35.63% -46.95%
===========================================
Files 515 516 +1
Lines 48806 48923 +117
Branches 2982 2982
===========================================
- Hits 40308 17436 -22872
- Misses 7587 30878 +23291
+ Partials 911 609 -302 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Duo <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Out of diff range and nitpick comments (2)
deepmd/pt/model/descriptor/dpa1.py (1)
175-176
: Add documentation for theseed
parameter in the class docstring.It's good practice to document all parameters in the class docstring to improve code maintainability and readability.
deepmd/pt/model/descriptor/dpa2.py (1)
106-106
: Add documentation for theseed
parameter in the class docstring.It's good practice to document all parameters in the class docstring to improve code maintainability and readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approve this PR, but it looks to me that seed
is not passed from descriptors/fittings to the network in dpmodel, which needs to be fixed in the future.
an issue is necessary before this PR can be merged. |
I've opened an issue about this: #3799 |
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added various neural network weight initialization methods: zeros, ones, constants, normal distributions, truncated normal distributions, Kaiming distributions, and Xavier distributions. - **Improvements** - Included optional `seed` parameter in initialization methods and classes. - Implemented a `get_generator` function for random seed management. - **Bug Fixes** - Addressed potential unintended behavior by ensuring proper random seed setting during training processes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Duo <[email protected]>
Summary by CodeRabbit
New Features
Improvements
seed
parameter in initialization methods and classes.get_generator
function for random seed management.Bug Fixes