feat: support using train/valid data from input.json for dp test#4859
feat: support using train/valid data from input.json for dp test#4859njzjz merged 10 commits intodeepmodeling:develfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for using a validation dataset from a training input JSON file for the dp test command. The implementation adds an --input-json flag that allows users to specify a training configuration file, and the validation systems defined in that file will be used for testing instead of requiring separate system specification.
Key changes:
- Added
--input-jsoncommand line argument to thedp testcommand - Enhanced the test function to handle
rglob_patternswhen using input JSON files - Refactored existing test code to support both traditional and JSON-based testing approaches
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| deepmd/main.py | Added --input-json argument to the test command parser |
| deepmd/entrypoints/test.py | Implemented logic to process validation systems from input JSON with rglob pattern support |
| source/tests/pt/test_dp_test.py | Refactored tests and added new test cases for input JSON functionality |
Comments suppressed due to low confidence (1)
source/tests/pt/test_dp_test.py:33
- [nitpick] The parameter name 'use_input_json' could be more descriptive. Consider 'use_validation_from_json' or 'load_systems_from_json' to better indicate its purpose.
def _run_dp_test(self, use_input_json: bool, numb_test: int = 0) -> None:
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdded optional JSON-based data-source inputs to the test entrypoint and CLI, enabling selection of training or validation systems from a JSON file; updated control flow to prioritize Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI
participant test_fn as test()
participant j_loader
participant process_systems
User->>CLI: run `deepmd test` (opts: --train-data/--valid-data or datafile or system)
CLI->>test_fn: call test(model, train_json?, valid_json?, datafile?, system?, ...)
alt train_json provided
test_fn->>j_loader: load & update JSON (train_json)
j_loader-->>test_fn: parsed JSON
test_fn->>process_systems: resolve training_data systems + apply rglob_patterns
process_systems-->>test_fn: all_sys
else valid_json provided
test_fn->>j_loader: load & update JSON (valid_json)
j_loader-->>test_fn: parsed JSON
test_fn->>process_systems: resolve validation_data systems + apply rglob_patterns
process_systems-->>test_fn: all_sys
else datafile provided
test_fn->>test_fn: read systems from datafile (lines)
test_fn-->>test_fn: all_sys
else system provided
test_fn->>test_fn: expand single system string
test_fn-->>test_fn: all_sys
end
test_fn->>test_fn: execute test flow over all_sys
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #4859 +/- ##
=======================================
Coverage 84.29% 84.29%
=======================================
Files 704 704
Lines 68875 68906 +31
Branches 3572 3572
=======================================
+ Hits 58057 58086 +29
- Misses 9678 9680 +2
Partials 1140 1140 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Chun Cai <amoycaic@gmail.com>
for more information, see https://pre-commit.ci
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
source/tests/pt/test_dp_test.py (1)
263-273: Same tearDown duplication pattern.This tearDown method is identical to the previous ones, reinforcing the need for the refactoring suggested earlier.
🧹 Nitpick comments (1)
source/tests/pt/test_dp_test.py (1)
207-217: Code duplication in tearDown methods.The tearDown logic is duplicated across multiple test classes. Consider extracting this into a base test utility method.
Create a base test mixin or utility method:
class TestCleanupMixin: def cleanup_test_files(self): for f in os.listdir("."): if f.startswith("model") and f.endswith(".pt"): os.remove(f) if f.startswith(self.detail_file): os.remove(f) if f in ["lcurve.out", self.input_json]: os.remove(f) if f in ["stat_files"]: shutil.rmtree(f)Then use
self.cleanup_test_files()in tearDown methods.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
deepmd/entrypoints/test.py(5 hunks)deepmd/main.py(2 hunks)source/tests/pt/test_dp_test.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- deepmd/main.py
- deepmd/entrypoints/test.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: njzjz
PR: deepmodeling/deepmd-kit#4144
File: source/api_cc/tests/test_deeppot_dpa_pt.cc:166-246
Timestamp: 2024-10-08T15:32:11.479Z
Learning: Refactoring between test classes `TestInferDeepPotDpaPt` and `TestInferDeepPotDpaPtNopbc` is addressed in PR #3905.
Learnt from: njzjz
PR: deepmodeling/deepmd-kit#4144
File: source/api_cc/tests/test_deeppot_dpa_pt.cc:166-246
Timestamp: 2024-09-19T04:25:12.408Z
Learning: Refactoring between test classes `TestInferDeepPotDpaPt` and `TestInferDeepPotDpaPtNopbc` is addressed in PR #3905.
📚 Learning: refactoring between test classes `testinferdeeppotdpapt` and `testinferdeeppotdpaptnopbc` is address...
Learnt from: njzjz
PR: deepmodeling/deepmd-kit#4144
File: source/api_cc/tests/test_deeppot_dpa_pt.cc:166-246
Timestamp: 2024-10-08T15:32:11.479Z
Learning: Refactoring between test classes `TestInferDeepPotDpaPt` and `TestInferDeepPotDpaPtNopbc` is addressed in PR #3905.
Applied to files:
source/tests/pt/test_dp_test.py
🪛 Ruff (0.12.2)
source/tests/pt/test_dp_test.py
190-190: Use a context manager for opening files
(SIM115)
245-245: Use a context manager for opening files
(SIM115)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
- GitHub Check: Test Python (2, 3.12)
- GitHub Check: Test Python (4, 3.12)
- GitHub Check: Test Python (6, 3.12)
- GitHub Check: Test Python (4, 3.9)
- GitHub Check: Test Python (5, 3.12)
- GitHub Check: Test Python (3, 3.12)
- GitHub Check: Test Python (6, 3.9)
- GitHub Check: Test Python (3, 3.9)
- GitHub Check: Test Python (5, 3.9)
- GitHub Check: Test Python (1, 3.9)
- GitHub Check: Test Python (1, 3.12)
- GitHub Check: Test Python (2, 3.9)
- GitHub Check: Build wheels for cp310-manylinux_aarch64
- GitHub Check: Build wheels for cp311-macosx_x86_64
- GitHub Check: Build wheels for cp311-win_amd64
- GitHub Check: Build wheels for cp311-macosx_arm64
- GitHub Check: Build wheels for cp311-manylinux_x86_64
- GitHub Check: Build wheels for cp311-manylinux_x86_64
- GitHub Check: Build C library (2.18, libdeepmd_c.tar.gz)
- GitHub Check: Build C library (2.14, >=2.5.0,<2.15, libdeepmd_c_cu11.tar.gz)
- GitHub Check: Test C++ (false)
- GitHub Check: Test C++ (true)
- GitHub Check: Build C++ (rocm, rocm)
- GitHub Check: Build C++ (cuda, cuda)
- GitHub Check: Build C++ (clang, clang)
- GitHub Check: Build C++ (cpu, cpu)
- GitHub Check: Build C++ (cuda120, cuda)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (c-cpp)
🔇 Additional comments (6)
source/tests/pt/test_dp_test.py (6)
33-35: LGTM! Good refactoring to eliminate code duplication.The parameterized helper method effectively consolidates the test logic while supporting different execution modes through boolean flags.
49-51: Validation system extraction logic looks correct.The code properly handles both list and single system configurations by extracting the first system when a list is provided. This matches the existing pattern used elsewhere in the codebase.
56-58: Conditional parameter passing implemented correctly.The use of conditional expressions to pass
input_jsonanduse_trainparameters based on the method arguments is clean and appropriate.
103-115: Good test coverage for the new input JSON functionality.The three test methods provide comprehensive coverage:
test_dp_test_1_frame: Tests original functionality (backward compatibility)test_dp_test_input_json: Tests new JSON input featuretest_dp_test_input_json_train: Tests JSON input with training data fallbackThe test logic in
test_dp_test_input_json_traincorrectly modifies the config to simulate a non-existent validation system, forcing the use of training data.
164-183: New test class setup looks comprehensive.The
TestDPTestSeARglobclass properly configures the test environment for recursive glob pattern testing with appropriaterglob_patternsconfiguration.
219-238: TestDPTestSeARglobTrain setup mirrors the previous class appropriately.The configuration correctly sets up training data with root directory and glob patterns while keeping validation data as a specific file, which is the intended test scenario.
Signed-off-by: Chun Cai <amoycaic@gmail.com>
caic99
changed the title
caic99
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deepmd/entrypoints/test.py (1)
66-80: Restore set_prefix support and align optional-ness with annotations.The function ignores caller-supplied set_prefix (swallowed by kwargs) and forces "set". Also docstring marks system/datafile/detail_file optional but the signature makes the first two required and detail_file non-Optional.
Apply:
def test( *, model: str, - system: Optional[str], - datafile: Optional[str], + system: Optional[str] = None, + datafile: Optional[str] = None, train_json: Optional[str] = None, valid_json: Optional[str] = None, numb_test: int, rand_seed: Optional[int], shuffle_test: bool, - detail_file: str, + detail_file: Optional[str] = None, atomic: bool, head: Optional[str] = None, + set_prefix: str = "set", **kwargs, ) -> None: @@ - data = DeepmdData( - system, - set_prefix="set", + data = DeepmdData( + system, + set_prefix=set_prefix, shuffle_test=shuffle_test, type_map=tmap, sort_atoms=False, )Also update the docstring to add “set_prefix : str, optional” and reflect Optional[str] for detail_file, system, datafile.
Also applies to: 84-107, 171-177
♻️ Duplicate comments (3)
deepmd/entrypoints/test.py (1)
117-145: Make error messages specific and actionable.Prior review requested specificity. Suggest clarifying fields and accepted sources.
- if not systems: - raise RuntimeError("No training data found in input json") + if not systems: + raise RuntimeError( + "No training systems found in training.training_data.systems of input json file" + ) @@ - if not systems: - raise RuntimeError("No validation data found in input json") + if not systems: + raise RuntimeError( + "No validation systems found in training.validation_data.systems of input json file" + ) @@ - raise RuntimeError("No data source specified for testing") + raise RuntimeError( + "No data source specified. Provide one of: --train-data, --valid-data, --datafile, or --system." + )Also applies to: 131-145, 149-151
source/tests/pt/test_dp_test.py (2)
239-260: Apply the same context-manager fix here.Same reasoning as above.
- tmp_model = tempfile.NamedTemporaryFile(delete=False, suffix=".pth") - torch.jit.save(model, tmp_model.name) - dp_test( - model=tmp_model.name, + with tempfile.NamedTemporaryFile(delete=False, suffix=".pth") as tmp_model: + torch.jit.save(model, tmp_model.name) + model_path = tmp_model.name + dp_test( + model=model_path, system=None, datafile=None, train_json=self.input_json, set_prefix="set", numb_test=1, rand_seed=None, shuffle_test=False, detail_file=self.detail_file, atomic=False, ) - os.unlink(tmp_model.name) + os.unlink(model_path)
184-205: Use a context manager for the temp model file to ensure cleanup.Prevents leaks if dp_test raises.
- tmp_model = tempfile.NamedTemporaryFile(delete=False, suffix=".pth") - torch.jit.save(model, tmp_model.name) - dp_test( - model=tmp_model.name, + with tempfile.NamedTemporaryFile(delete=False, suffix=".pth") as tmp_model: + torch.jit.save(model, tmp_model.name) + model_path = tmp_model.name + dp_test( + model=model_path, system=None, datafile=None, valid_json=self.input_json, set_prefix="set", numb_test=1, rand_seed=None, shuffle_test=False, detail_file=self.detail_file, atomic=False, ) - os.unlink(tmp_model.name) + os.unlink(model_path)
🧹 Nitpick comments (2)
deepmd/entrypoints/test.py (1)
117-145: De-duplicate JSON-loading branches with a small helper.Both train_json and valid_json branches differ only by section name; extract a helper to reduce maintenance.
Example (outside this function):
def _load_systems_from_json(json_path: str, section: str) -> list[str]: jdata = update_deepmd_input(j_loader(json_path)) data_params = jdata.get("training", {}).get(section, {}) systems = data_params.get("systems") if not systems: raise RuntimeError(f"No { 'training' if section=='training_data' else 'validation' } systems found in training.{section}.systems of input json file") root = Path(json_path).parent systems = (str((root / Path(systems)).resolve()) if isinstance(systems, str) else [str((root / Path(ss)).resolve()) for ss in systems]) return process_systems(systems, patterns=data_params.get("rglob_patterns"))Then call with section="training_data" or "validation_data".
source/tests/pt/test_dp_test.py (1)
49-51: Minor: isolate validation-system picking.Create a tiny helper (e.g., _get_validation_system()) to avoid in-line branching and keep _run_dp_test focused.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
deepmd/entrypoints/test.py(5 hunks)deepmd/main.py(1 hunks)source/tests/common/test_argument_parser.py(1 hunks)source/tests/pt/test_dp_test.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- deepmd/main.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-08T15:32:11.479Z
Learnt from: njzjz
PR: deepmodeling/deepmd-kit#4144
File: source/api_cc/tests/test_deeppot_dpa_pt.cc:166-246
Timestamp: 2024-10-08T15:32:11.479Z
Learning: Refactoring between test classes `TestInferDeepPotDpaPt` and `TestInferDeepPotDpaPtNopbc` is addressed in PR #3905.
Applied to files:
source/tests/pt/test_dp_test.py
🧬 Code graph analysis (2)
deepmd/entrypoints/test.py (3)
deepmd/common.py (2)
j_loader(162-188)expand_sys_str(191-208)deepmd/utils/compat.py (1)
update_deepmd_input(375-394)deepmd/utils/data_system.py (1)
process_systems(779-806)
source/tests/pt/test_dp_test.py (2)
source/tests/pt/model/test_deeppot.py (3)
setUp(26-48)setUp(120-128)tearDown(50-53)deepmd/pt/train/training.py (1)
get_data(1199-1241)
🪛 Ruff (0.12.2)
source/tests/pt/test_dp_test.py
190-190: Use a context manager for opening files
(SIM115)
245-245: Use a context manager for opening files
(SIM115)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: Test Python (6, 3.9)
- GitHub Check: Test Python (6, 3.12)
- GitHub Check: Test Python (5, 3.9)
- GitHub Check: Test Python (5, 3.12)
- GitHub Check: Test Python (4, 3.12)
- GitHub Check: Test Python (4, 3.9)
- GitHub Check: Test Python (2, 3.9)
- GitHub Check: Test Python (3, 3.12)
- GitHub Check: Test Python (3, 3.9)
- GitHub Check: Test Python (1, 3.9)
- GitHub Check: Test Python (1, 3.12)
- GitHub Check: Test Python (2, 3.12)
- GitHub Check: Analyze (c-cpp)
- GitHub Check: Analyze (python)
- GitHub Check: Build C library (2.18, libdeepmd_c.tar.gz)
- GitHub Check: Build C library (2.14, >=2.5.0,<2.15, libdeepmd_c_cu11.tar.gz)
- GitHub Check: Build C++ (cpu, cpu)
- GitHub Check: Build C++ (clang, clang)
- GitHub Check: Build C++ (rocm, rocm)
- GitHub Check: Build C++ (cuda120, cuda)
- GitHub Check: Build C++ (cuda, cuda)
- GitHub Check: Build wheels for cp311-macosx_x86_64
- GitHub Check: Build wheels for cp311-win_amd64
- GitHub Check: Build wheels for cp311-manylinux_x86_64
- GitHub Check: Build wheels for cp311-manylinux_x86_64
- GitHub Check: Build wheels for cp310-manylinux_aarch64
- GitHub Check: Build wheels for cp311-macosx_arm64
- GitHub Check: Test C++ (false)
- GitHub Check: Test C++ (true)
🔇 Additional comments (4)
source/tests/common/test_argument_parser.py (2)
338-350: Approve: --valid-data dest matches implementation.
Destination (“valid_json”) in tests aligns with the add_argument setup in deepmd/main.py.
325-337: Approve--train-datadest mapping. The CLI definition in deepmd/main.py binds--train-datatodest="train_json", matching the test’s expectations.deepmd/entrypoints/test.py (1)
15-18: Good additions: config loader + compat + system processor are correctly imported and used.Using j_loader + update_deepmd_input prevents legacy-config drift; process_systems centralizes discovery/globbing.
Also applies to: 42-44, 48-50
source/tests/pt/test_dp_test.py (1)
103-116: Good coverage for input_json and train/valid selection.These tests exercise both JSON pathways and precedence.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
deepmd/entrypoints/test.py (1)
133-146: Validation JSON: mirror training fixes; make error actionable.
- Same rglob-for-list handling as above.
- Error should indicate the precise JSON location.
Apply:
- data_params = jdata.get("training", {}).get("validation_data", {}) + data_params = jdata.get("training", {}).get("validation_data", {}) systems = data_params.get("systems") - if not systems: - raise RuntimeError("No validation data found in input json") + if not systems: + raise RuntimeError( + f"No validation systems found at 'training.validation_data.systems' in {valid_json}" + ) root = Path(valid_json).parent if isinstance(systems, str): systems = str((root / Path(systems)).resolve()) else: systems = [str((root / Path(ss)).resolve()) for ss in systems] - patterns = data_params.get("rglob_patterns", None) - all_sys = process_systems(systems, patterns=patterns) + patterns = data_params.get("rglob_patterns", None) + if isinstance(systems, list) and patterns is not None: + expanded: list[str] = [] + for ss in systems: + expanded.extend(process_systems(ss, patterns=patterns)) + all_sys = expanded + else: + all_sys = process_systems(systems, patterns=patterns)
🧹 Nitpick comments (6)
deepmd/entrypoints/test.py (6)
49-51: Prefer using process_systems consistently.You import process_systems; consider also using it for the
systembranch below for consistency and future pattern support.
87-96: Docstring: clarify JSON schema and precedence.Consider documenting the exact JSON paths used:
- train_json: training.training_data.systems[, rglob_patterns]
- valid_json: training.validation_data.systems[, rglob_patterns]
And note precedence: train_json → valid_json → datafile → system.
101-102: Type hint/doc mismatch for detail_file.The docstring says Optional[str], but the signature declares str. Align to Optional[str].
Apply:
- detail_file: str, + detail_file: Optional[str],
118-131: JSON systems: resolve paths correctly; handle rglob for list; improve error text.
- If systems is a list and rglob_patterns is provided, process each entry with rglob; current code passes the list to process_systems, which skips validation/rglob for lists.
- Make the error point to the exact JSON key path and filename for easier debugging.
Apply:
- systems = data_params.get("systems") - if not systems: - raise RuntimeError("No training data found in input json") + systems = data_params.get("systems") + if not systems: + raise RuntimeError( + f"No training systems found at 'training.training_data.systems' in {train_json}" + ) root = Path(train_json).parent if isinstance(systems, str): systems = str((root / Path(systems)).resolve()) else: systems = [str((root / Path(ss)).resolve()) for ss in systems] - patterns = data_params.get("rglob_patterns", None) - all_sys = process_systems(systems, patterns=patterns) + patterns = data_params.get("rglob_patterns", None) + if isinstance(systems, list) and patterns is not None: + expanded: list[str] = [] + for ss in systems: + expanded.extend(process_systems(ss, patterns=patterns)) + all_sys = expanded + else: + all_sys = process_systems(systems, patterns=patterns)
149-151: Use process_systems for the system branch too.Keeps behavior consistent with JSON/datafile flows and centralizes validity checks.
Apply:
- elif system is not None: - all_sys = expand_sys_str(system) + elif system is not None: + all_sys = process_systems(system)
152-152: Static analysis: Ruff TRY003 hints on raise messages.If TRY003 is enforced in CI, append
# noqa: TRY003to these three raise lines (train/valid/no-source) or define project-specific exceptions and move messages into the class docstrings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
deepmd/entrypoints/test.py(5 hunks)deepmd/main.py(1 hunks)source/tests/pt/test_dp_test.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- deepmd/main.py
- source/tests/pt/test_dp_test.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Run ruff check and ruff format on all Python code before committing
Ensure Python code is formatted with ruff’s formatter
Files:
deepmd/entrypoints/test.py
🧬 Code graph analysis (1)
deepmd/entrypoints/test.py (3)
deepmd/common.py (2)
j_loader(162-188)expand_sys_str(191-208)deepmd/utils/compat.py (1)
update_deepmd_input(375-394)deepmd/utils/data_system.py (1)
process_systems(787-814)
🪛 Ruff (0.12.2)
deepmd/entrypoints/test.py
124-124: Avoid specifying long messages outside the exception class
(TRY003)
138-138: Avoid specifying long messages outside the exception class
(TRY003)
152-152: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: Build C++ (cuda, cuda)
- GitHub Check: Build C++ (clang, clang)
- GitHub Check: Build C++ (cuda120, cuda)
- GitHub Check: Build C++ (cpu, cpu)
- GitHub Check: Build C++ (rocm, rocm)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (c-cpp)
- GitHub Check: Test Python (1, 3.12)
- GitHub Check: Test Python (4, 3.9)
- GitHub Check: Test Python (5, 3.12)
- GitHub Check: Test Python (6, 3.9)
- GitHub Check: Test Python (5, 3.9)
- GitHub Check: Test Python (4, 3.12)
- GitHub Check: Test C++ (false)
- GitHub Check: Test Python (6, 3.12)
- GitHub Check: Build C library (2.18, libdeepmd_c.tar.gz)
- GitHub Check: Test Python (2, 3.12)
- GitHub Check: Test Python (3, 3.9)
- GitHub Check: Test Python (1, 3.9)
- GitHub Check: Build C library (2.14, >=2.5.0,<2.15, libdeepmd_c_cu11.tar.gz)
- GitHub Check: Test Python (2, 3.9)
- GitHub Check: Test C++ (true)
- GitHub Check: Test Python (3, 3.12)
- GitHub Check: Build wheels for cp310-manylinux_aarch64
- GitHub Check: Build wheels for cp311-manylinux_x86_64
- GitHub Check: Build wheels for cp311-manylinux_x86_64
- GitHub Check: Build wheels for cp311-macosx_x86_64
- GitHub Check: Build wheels for cp311-win_amd64
- GitHub Check: Build wheels for cp311-macosx_arm64
🔇 Additional comments (3)
deepmd/entrypoints/test.py (3)
18-19: Import of j_loader is correct.Needed for reading input.json/yaml and aligns with common.j_loader. LGTM.
43-45: Good call adding update_deepmd_input.Ensures v0/v1 inputs are normalized before accessing training/validation blocks.
69-73: Verify CLI argument exclusivity for train/valid inputs
Programmatic callers (tests) already use the new optionaltrain_json/valid_jsonparameters correctly. Confirm the actual CLI entrypoint (outsidedeepmd/entrypoints/test.py) defines a mutually-exclusive group for--train-data/--valid-data.
…pmodeling#4859) This pull request extends the testing functionality in DeepMD by allowing users to specify training and validation data directly via input JSON files, in addition to existing system and datafile options. It updates the command-line interface, the main test logic, and adds comprehensive tests to cover these new features, including support for recursive glob patterns when selecting systems from JSON files. ### Feature enhancements to testing data sources * The `test` function in `deepmd/entrypoints/test.py` now accepts `train_json` and `valid_json` arguments, allowing users to specify training or validation systems for testing via input JSON files. It processes these files to extract system paths, including support for recursive glob patterns. The function also raises an error if no valid data source is specified. [[1]](diffhunk://#diff-299c01ed4ee7d0b3f636fe4cb4f0d660a5012b7e95ca0740098b3ace617ab16eL61-R71) [[2]](diffhunk://#diff-299c01ed4ee7d0b3f636fe4cb4f0d660a5012b7e95ca0740098b3ace617ab16eL104-R151) * **The command-line interface in `deepmd/main.py` is updated to add `--train-data` and `--valid-data` arguments for the test subparser, enabling direct specification of input JSON files for training and validation data.** ### Test coverage improvements * New and updated tests in `source/tests/pt/test_dp_test.py` verify the ability to run tests using input JSON files for both training and validation data, including cases with recursive glob patterns. This ensures robust handling of various data source configurations. [[1]](diffhunk://#diff-ce70e95ffdb1996c7887ea3f63b54d1ae0fef98059572ad03875ca36cfef3c34L33-R35) [[2]](diffhunk://#diff-ce70e95ffdb1996c7887ea3f63b54d1ae0fef98059572ad03875ca36cfef3c34R49-R59) [[3]](diffhunk://#diff-ce70e95ffdb1996c7887ea3f63b54d1ae0fef98059572ad03875ca36cfef3c34R103-R116) [[4]](diffhunk://#diff-ce70e95ffdb1996c7887ea3f63b54d1ae0fef98059572ad03875ca36cfef3c34R164-R273) * Additional argument parser tests in `source/tests/common/test_argument_parser.py` confirm correct parsing of the new `--train-data` and `--valid-data` options. ### Internal code improvements * Refactored imports and type annotations in `deepmd/entrypoints/test.py` to support the new functionality and improve code clarity. [[1]](diffhunk://#diff-299c01ed4ee7d0b3f636fe4cb4f0d660a5012b7e95ca0740098b3ace617ab16eR17) [[2]](diffhunk://#diff-299c01ed4ee7d0b3f636fe4cb4f0d660a5012b7e95ca0740098b3ace617ab16eR42-R50) [[3]](diffhunk://#diff-299c01ed4ee7d0b3f636fe4cb4f0d660a5012b7e95ca0740098b3ace617ab16eL77-R95) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Added support for supplying test systems via JSON files, including selecting training or validation data. - Introduced CLI options --train-data and --valid-data for the test command. - Supports resolving relative paths from JSON and optional recursive glob patterns. - Changes - Test command now requires at least one data source (JSON, data file, or system); clearer errors when none or no systems found. - Tests - Expanded test coverage for JSON-driven inputs and recursive glob patterns; refactored helpers for improved readability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Chun Cai <amoycaic@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2 participants
This pull request extends the testing functionality in DeepMD by allowing users to specify training and validation data directly via input JSON files, in addition to existing system and datafile options. It updates the command-line interface, the main test logic, and adds comprehensive tests to cover these new features, including support for recursive glob patterns when selecting systems from JSON files.
Feature enhancements to testing data sources
testfunction indeepmd/entrypoints/test.pynow acceptstrain_jsonandvalid_jsonarguments, allowing users to specify training or validation systems for testing via input JSON files. It processes these files to extract system paths, including support for recursive glob patterns. The function also raises an error if no valid data source is specified. [1] [2]deepmd/main.pyis updated to add--train-dataand--valid-dataarguments for the test subparser, enabling direct specification of input JSON files for training and validation data.Test coverage improvements
source/tests/pt/test_dp_test.pyverify the ability to run tests using input JSON files for both training and validation data, including cases with recursive glob patterns. This ensures robust handling of various data source configurations. [1] [2] [3] [4]source/tests/common/test_argument_parser.pyconfirm correct parsing of the new--train-dataand--valid-dataoptions.Internal code improvements
deepmd/entrypoints/test.pyto support the new functionality and improve code clarity. [1] [2] [3]Summary by CodeRabbit