Skip to content

Conversation

@HydrogenSulfate
Copy link
Collaborator

@HydrogenSulfate HydrogenSulfate commented Nov 25, 2024

Summary of this PR:

  1. upload DPA-1 related code
  2. merge much develop code
  3. add all eager composite operators except softmax_grad, p_norm_grad, split_grad, and concat_grad to the composite operator blacklist(https://github.com/deepmodeling/deepmd-kit/pull/4414/files#diff-e678abb052b278f8a479f8d13b839a9ec0effd9923478a850bc13758f918e1e9R134-R148) to significantly improve model execution speed (reducing the time taken from 100% more than PyTorch to about 10% to 15% more).

related PR: lanpa/tensorboardX#728

Training curve:

training_curves_comparison_eager_opt

Accuracy test(left: paddle, right: torch):

image

Ralated optimization of Paddle framework:

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced several new classes for molecular descriptors, including DescrptDPA1, DescrptBlockSeAtten, and LayerNorm, enhancing the modeling capabilities for molecular simulations.
    • Added new JSON configuration files for model parameters and multitask models related to water simulations.
    • Implemented new test classes for validating the functionality of the DPAtomicModel and various descriptor classes.
    • Added new test classes for evaluating denoising models, including TestDenoiseModelDPA1 and TestDenoiseModelDPA2.
    • Enhanced the ModelWrapper class to clarify the handling of model parameters and state management.
  • Bug Fixes

    • Improved internal logic for handling model state saving and loading, ensuring consistency in outputs.
  • Documentation

    • Enhanced type hints and return annotations across various classes and methods for better clarity.
  • Tests

    • Expanded the testing framework with new test cases for denoising models and descriptor functionalities, ensuring robust validation of features.
    • Activated previously skipped tests for energy models, improving test coverage.
    • Enhanced multitask training tests with new configuration handling and test classes.

HydrogenSulfate and others added 30 commits November 2, 2024 11:14
@njzjz njzjz requested a review from iProzd November 30, 2024 20:31
@HydrogenSulfate
Copy link
Collaborator Author

@iProzd PR of pd: DPA-1 and pd: DPA-2 is ready for review.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (3)
source/tests/pd/model/water/multitask_sharefit.json (2)

11-15: Document the rationale behind selection parameters

The selection parameters [46, 92, 4] seem arbitrary. Consider adding a comment explaining why these specific values were chosen and their significance in the context of water molecule simulations.


56-61: Consider adjusting the minimum learning rate

The stop_lr value of 3.51e-08 is extremely small. Consider using a larger value (e.g., 1e-6) as training might not benefit from such small learning rates and could unnecessarily extend training time.

source/tests/pd/test_multitask.py (1)

71-76: Document the conditions for parameter sharing

The conditions for parameter sharing in fitting networks exclude certain parameters (bias_atom_e and case_embd). Consider adding a comment explaining why these specific parameters are excluded from sharing.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 95ac41d and 63ccdbd.

📒 Files selected for processing (2)
  • source/tests/pd/model/water/multitask_sharefit.json (1 hunks)
  • source/tests/pd/test_multitask.py (6 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeQL
source/tests/pd/test_multitask.py

[notice] 48-48: Unused global variable
The global variable 'multitask_sharefit_template' is not used.

🔇 Additional comments (2)
source/tests/pd/model/water/multitask_sharefit.json (1)

64-82: LGTM! Well-structured loss weighting strategy

The loss configuration implements a good strategy by:

  1. Initially prioritizing force accuracy (pref_f: 1000 → 1)
  2. Gradually increasing energy contribution (pref_e: 0.02 → 1)
  3. Maintaining identical configurations for both models
source/tests/pd/test_multitask.py (1)

43-48: ⚠️ Potential issue

Remove or utilize the unused template variable

The multitask_sharefit_template variable is loaded but never used in the code. Either:

  1. Remove it if it's not needed, or
  2. Update the tests to utilize this shared fitting configuration
🧰 Tools
🪛 GitHub Check: CodeQL

[notice] 48-48: Unused global variable
The global variable 'multitask_sharefit_template' is not used.

@njzjz
Copy link
Member

njzjz commented Dec 13, 2024

@coderabbitai resolve

@njzjz njzjz requested a review from iProzd December 13, 2024 20:38
@njzjz njzjz enabled auto-merge December 17, 2024 22:27
@njzjz njzjz added this pull request to the merge queue Dec 17, 2024
Merged via the queue into deepmodeling:devel with commit e8167ce Dec 18, 2024
60 checks passed
HydrogenSulfate added a commit to HydrogenSulfate/deepmd-kit that referenced this pull request Dec 18, 2024
@HydrogenSulfate HydrogenSulfate deleted the add_dpa1 branch December 19, 2024 07:56
github-merge-queue bot pushed a commit that referenced this pull request Dec 25, 2024
Support DPA-2 in paddle backend. This PR will be updated after #4414 is
merged.

### Training curve:


![training_curves_comparison_dpa2](https://github.com/user-attachments/assets/29bdeffa-cf2d-4586-afcf-7df0569997c3)



### Accuracy test(left: paddle, right: torch):


![image](https://github.com/user-attachments/assets/5bff55f3-1c39-4b95-93f0-68783e794716)


Ralated optimization of Paddle framework:
- [x] PaddlePaddle/Paddle#69349
- [x] PaddlePaddle/Paddle#69333
- [x] PaddlePaddle/Paddle#69479
- [x] PaddlePaddle/Paddle#69515
- [x] PaddlePaddle/Paddle#69487
- [x] PaddlePaddle/Paddle#69661
- [x] PaddlePaddle/Paddle#69660
- [x] PaddlePaddle/Paddle#69596
- [x] PaddlePaddle/Paddle#69556

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Introduced new classes for molecular descriptors: `DescrptDPA2`,
`DescrptBlockRepformers`, `DescrptSeTTebd`, and `DescrptBlockSeTTebd`.
- Added new functions for tensor operations and descriptor management,
enhancing the capabilities of the module.
- Updated JSON configurations for multitask models to refine selection
criteria and data paths.

- **Bug Fixes**
- Improved error handling and parameter validation across various
descriptor classes.

- **Documentation**
- Enhanced test coverage for new descriptor functionalities and
configurations.

- **Tests**
- Added new test classes to validate the functionality of `DescrptDPA2`
and multitask training scenarios.
- Expanded test capabilities for descriptor classes based on installed
dependencies.
- Updated existing tests to support new configurations and
functionalities.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants