-
Notifications
You must be signed in to change notification settings - Fork 525
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
chore(pt): update multitask example #4419
chore(pt): update multitask example #4419
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request involve updates to the JSON configuration for a multi-task model, specifically modifying the descriptors used in the model's definition. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Model
participant Descriptor
participant SharedDict
User->>Model: Request model initialization
Model->>SharedDict: Retrieve dpa2_descriptor
SharedDict->>Descriptor: Provide dpa2_descriptor parameters
Descriptor-->>Model: Return descriptor details
Model-->>User: Initialize model with dpa2_descriptor
Warning Rate limit exceeded@iProzd has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 4 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
🧹 Outside diff range and nitpick comments (1)
examples/water_multi_task/pytorch_example/input_torch.json (1)
78-78
: Consider descriptor parameter specialization for different tasks.Both water_1 and water_2 now share identical descriptor configurations. While this might be intentional for parameter sharing in multi-task learning, consider whether task-specific descriptor parameters might be beneficial for each water model's unique characteristics.
You might want to experiment with task-specific parameters in the descriptor configuration if the two water models have different physical characteristics or accuracy requirements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
examples/water_multi_task/pytorch_example/input_torch.json
(2 hunks)
🔇 Additional comments (3)
examples/water_multi_task/pytorch_example/input_torch.json (3)
9-57
: Verify the descriptor parameters for optimal performance.
The dpa2_descriptor configuration appears well-structured with consistent cutoff parameters and dimensions. However, a few parameters might need verification:
- The
tebd_dim
(8) seems relatively small compared tonsel
(120). This might limit the model's representational capacity. - The attention configuration uses different hidden dimensions for different levels (attn1_hidden: 128, attn2_hidden: 32). Ensure this intentional asymmetry aligns with your model design.
✅ Verification successful
The dpa2_descriptor configuration parameters are consistent with established examples
After examining multiple dpa2 configurations in the codebase, I can confirm that:
- The
tebd_dim
value of 8 is the standard setting used across all examples, including small, medium, and large configurations. - The
nsel
values (120 for repinit, 48 for repformer) match the configurations used in other water simulations. - The attention dimensions (attn1_hidden: 128, attn2_hidden: 32) are consistent with all other dpa2 examples in the water directory.
The asymmetric attention configuration is a deliberate design choice present in all production examples, not a concern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar dpa2 configurations in the codebase for parameter validation
rg -g '*.json' -A 10 '"dpa2_descriptor":\s*{' | grep -E '("tebd_dim"|"nsel"|"attn.*hidden")'
Length of output: 202
Script:
#!/bin/bash
# Search for similar dpa2 configurations in the codebase with fixed regex
rg -g '*.json' '"dpa2_descriptor"'
# If found, let's look for specific parameter patterns
rg -g '*.json' -A 2 '"tebd_dim"'
rg -g '*.json' -A 2 '"nsel"'
rg -g '*.json' -A 2 '"attn.*hidden"'
# Also search for any documentation or examples
fd -e md -e rst | xargs rg -l "dpa2"
Length of output: 6910
Line range hint 9-78
: Verify training hyperparameters with new descriptor.
The switch to dpa2_descriptor introduces more complex representation learning. The current learning rate configuration (exp decay from 0.0002 to 3.51e-08) and training steps (100000) might need adjustment for optimal convergence with the new descriptor.
64-64
: Verify fitting network compatibility with new descriptor.
The switch to dpa2_descriptor might require adjustments to the fitting network dimensions. The current neuron configuration [240, 240, 240] should be validated against the descriptor's output dimension (repinit.neuron[100] + potential repformer contributions).
Consider validating the input dimension compatibility between the descriptor output and the fitting network input:
✅ Verification successful
Network dimensions are compatible with dpa2_descriptor
The dpa2_descriptor configuration shows that the output dimension would be 100 (final layer of repinit.neuron) plus potential contributions from the repformer (g1_dim: 128, g2_dim: 32). The fitting network's input layer of 240 neurons is sufficient to handle this descriptor's output dimension.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar configurations to verify dimension compatibility
rg -g '*.json' -B 10 -A 10 '"fitting_net":\s*{.*"neuron":\s*\[240' | grep -B 5 -A 5 '"descriptor":\s*"dpa2_descriptor"'
Length of output: 259
Script:
#!/bin/bash
# Search for dpa2_descriptor configurations to understand typical network dimensions
rg -g '*.json' -B 10 -A 10 '"descriptor":\s*"dpa2_descriptor"'
Length of output: 2928
Script:
#!/bin/bash
# Search for fitting network configurations with 240 neurons
rg -g '*.json' -B 10 -A 10 '"fitting_net".*"neuron".*240'
Length of output: 59
Script:
#!/bin/bash
# Find any documentation or examples related to dpa2_descriptor
fd -g '*dpa2*.md' -g '*dpa2*.rst' -g '*dpa2*.txt'
Length of output: 206
Script:
#!/bin/bash
# Search for descriptor configuration in the same file
rg -g 'input_torch.json' -A 20 '"dpa2_descriptor":\s*{' examples/water_multi_task/pytorch_example/
Length of output: 206
Script:
#!/bin/bash
# Look for any files containing dpa2_descriptor configuration
fd . -t f -x grep -l "dpa2_descriptor" {}
Length of output: 104
Script:
#!/bin/bash
# Get the full content of input_torch.json to understand the descriptor configuration
cat examples/water_multi_task/pytorch_example/input_torch.json
Length of output: 3896
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #4419 +/- ##
==========================================
- Coverage 84.64% 84.64% -0.01%
==========================================
Files 614 614
Lines 57135 57135
Branches 3486 3487 +1
==========================================
- Hits 48364 48363 -1
Misses 7645 7645
- Partials 1126 1127 +1 ☔ View full report in Codecov by Sentry. |
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Updated multi-task model configuration with a new descriptor for enhanced representation learning. - Introduced additional parameters for model initialization and attention mechanisms. - **Bug Fixes** - Replaced outdated descriptor references in model configurations to ensure compatibility with new settings. <!-- end of auto-generated comment: release notes by coderabbit.ai --> (cherry picked from commit e7ad8dc)
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Updated multi-task model configuration with a new descriptor for enhanced representation learning. - Introduced additional parameters for model initialization and attention mechanisms. - **Bug Fixes** - Replaced outdated descriptor references in model configurations to ensure compatibility with new settings. <!-- end of auto-generated comment: release notes by coderabbit.ai --> (cherry picked from commit e7ad8dc)
Summary by CodeRabbit
New Features
Bug Fixes