feat(pt): support multitask argcheck #3925
Conversation
WalkthroughWalkthroughThe changes primarily enhance the multi-task functionalities in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TrainFunction as train()
participant ConfigUpdate as update_and_normalize_config()
participant ArgsCheck as training_args()
User->>TrainFunction: Call train(multi_task)
TrainFunction->>ConfigUpdate: Update and normalize config
alt multi_task is True
ConfigUpdate->>TrainFunction: Return updated config
else multi_task is False
ConfigUpdate->>TrainFunction: Return updated config
end
TrainFunction->>ArgsCheck: Call training_args with multi_task
ArgsCheck-->>TrainFunction: Return arguments based on multi_task
In this sequence diagram, the Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (2)
Files skipped from review due to trivial changes (1)
Additional comments not posted (3)
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (5)
deepmd/pt/entrypoints/main.py (2)
Line range hint
117-117: Remove unused variable assignment.- with h5py.File(stat_file_path_single, "w") as f: + with h5py.File(stat_file_path_single, "w"):This change removes the unused variable
f, addressing the static analysis warning.
Line range hint
373-376: Simplify argument parsing with the ternary operator.- if not isinstance(args, argparse.Namespace): - FLAGS = parse_args(args=args) - else: - FLAGS = args + FLAGS = parse_args(args=args) if not isinstance(args, argparse.Namespace) else argsThis change simplifies the code by replacing the
if-elseblock with a ternary operator, making the code more concise and readable.deepmd/utils/argcheck.py (3)
Line range hint
75-75: Specify thestacklevelfor warnings.The function
deprecate_argument_extra_checkuseswarnings.warnwithout specifying thestacklevel. Specifyingstacklevelhelps in identifying where the warning was triggered from, making debugging easier.- warnings.warn(f"{key} has been removed and takes no effect.", FutureWarning) + warnings.warn(f"{key} has been removed and takes no effect.", FutureWarning, stacklevel=2)
Line range hint
1167-1174: Remove unused local variables.Several local variables (
link_lf,link_se_e2_a,link_se_e2_r,link_se_e3,link_se_a_tpe,link_hybrid,link_se_atten,link_se_atten_v2) are defined but never used in the functiondescrpt_variant_type_args. This is likely a code cleanup issue.- link_lf = make_link("loc_frame", "model/descriptor[loc_frame]") - link_se_e2_a = make_link("se_e2_a", "model/descriptor[se_e2_a]") - link_se_e2_r = make_link("se_e2_r", "model/descriptor[se_e2_r]") - link_se_e3 = make_link("se_e3", "model/descriptor[se_e3]") - link_se_a_tpe = make_link("se_a_tpe", "model/descriptor[se_a_tpe]") - link_hybrid = make_link("hybrid", "model/descriptor[hybrid]") - link_se_atten = make_link("se_atten", "model/descriptor[se_atten]") - link_se_atten_v2 = make_link("se_atten_v2", "model/descriptor[se_atten_v2]")
Line range hint
2328-2564: Review the structure and logic oftraining_argsand related functions.The
training_argsfunction is well-structured with clear separation of concerns, handling both single and multi-task scenarios. However, ensure that the documentation for each argument is clear and detailed enough for users to understand without ambiguity, especially for complex configurations likemulti_task. Consider adding more examples or detailed descriptions in the documentation strings.However, there's a potential improvement in how documentation is handled:
# Suggestion to improve documentation readability and maintenance def add_detailed_docs(argument, details): argument.doc += " " + details # Example usage add_detailed_docs(arg_numb_steps, "This determines how many batches the model is trained with each epoch.")
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #3925 +/- ##
==========================================
+ Coverage 82.87% 82.88% +0.01%
==========================================
Files 519 520 +1
Lines 50666 50697 +31
Branches 3015 3015
==========================================
+ Hits 41990 42022 +32
+ Misses 7739 7738 -1
Partials 937 937 ☔ View full report in Codecov by Sentry. |
I checked the dargs API and it is not easy to do so. It uses a Maybe raise an issue in dargs... |
Tracked in deepmodeling/dargs#70 |
Note that: 1. docs for multitask args are not supported, may need help. 2. `trim_pattern="_*"` is not supported for repeat dict Argument, may need to update dargs. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced training configuration to support multi-task mode with additional arguments for data configuration. - Updated example configurations to reflect multi-task mode changes. - **Bug Fixes** - Improved logic for updating and normalizing configuration during training regardless of multi-task mode. - **Dependencies** - Upgraded `dargs` package requirement to version `>= 0.4.7`. - **Tests** - Added new test cases for multi-task scenarios in `TestExamples` class. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Note that:
trim_pattern="_*"is not supported for repeat dict Argument, may need to update dargs.Summary by CodeRabbit
New Features
Bug Fixes
Dependencies
dargspackage requirement to version>= 0.4.7.Tests
TestExamplesclass.