Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/input.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,19 @@ This section defines how a model is trained.
"numb_models" : 4,
"config" : {},
"template_script" : "/path/to/the/template/input.json",
"init_models_paths" : [
"/path/to/model.000.pb",
"/path/to/model.001.pb",
"/path/to/model.002.pb",
"/path/to/model.003.pb"
],
"_comment" : "all"
}
```
The `"type" : "dp"` tell the traning method is {dargs:argument}`"dp" <train>`, i.e. calling [DeePMD-kit](https://github.com/deepmodeling/deepmd-kit) to train DP models.
The `"config"` key defines the training configs, see {ref}`the full documentation<train[dp]/config>`.
The {dargs:argument}`"template_script" <train[dp]/template_script>` provides the template training script in `json` format.
When {dargs:argument}`"init_models_paths" <train[dp]/init_models_paths>` supplies one model per committee member and those models were already trained on the initial dataset, DPGEN2 automatically skips the iteration-zero training command. It records the generated training script and a skip message, then passes the supplied models directly to exploration. Training resumes after labeling produces iteration data. Finetuning requested with `"do_finetune": true` is never skipped.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"do_finetune" is an inputs argument, not a train one. It is declared in input_args() and read as config["inputs"].get("do_finetune", False) in submit.py. This sentence sits in ### Training, directly under the "train": {...} block, and - unlike "config", "template_script" and "init_models_paths" in the same paragraph - carries no {dargs:argument} role, so a reader has no link to follow to the right section.

The mistake that invites is silent. normalize() calls check_value(..., strict=False), so I put "do_finetune": true under "train" and normalization succeeded, the key was retained verbatim under train, and inputs.do_finetune stayed at its False default. No error, no finetuning - and therefore the iteration-zero skip that this very sentence promises would not happen.

{dargs:argument}`"do_finetune" <inputs/do_finetune>` would both fix the link and make the section obvious.



### Exploration
Expand Down
6 changes: 5 additions & 1 deletion dpgen2/entrypoint/args.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,11 @@ def dp_train_args():
doc_numb_models = "Number of models trained for evaluating the model deviation"
doc_config = "Configuration of training"
doc_template_script = "File names of the template training script. It can be a `List[str]`, the length of which is the same as `numb_models`. Each template script in the list is used to train a model. Can be a `str`, the models share the same template training script. "
doc_init_models_paths = "the paths to initial models"
doc_init_models_paths = (
"Paths to initial models. When these models already represent the "
"initial dataset, DPGEN2 reuses them and skips training in iteration "
"zero because no iteration-generated data exists yet."
)
doc_init_models_uri = "The URI of initial models"
doc_optional_files = "Optional files for training"

Expand Down
4 changes: 3 additions & 1 deletion tests/op/test_run_dp_train.py
Original file line number Diff line number Diff line change
Expand Up @@ -945,7 +945,8 @@ def test_update_input_dict_v2_empty_list(self):
)
self.assertDictEqual(odict, self.expected_odict_v2)

def test_exec_v2_empty_list(self):
@patch("dpgen2.op.run_dp_train.run_command")
def test_exec_v2_empty_list(self, mocked_run):
config = self.config.copy()
config["init_model_policy"] = "no"

Expand Down Expand Up @@ -989,6 +990,7 @@ def test_exec_v2_empty_list(self):
jdata = json.load(fp)
self.assertDictEqual(jdata, self.expected_odict_v2)
self.assertEqual(Path(out["model"]).read_text(), "this is init model")
mocked_run.assert_not_called()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This assertion cannot execute in any scenario where it would fail, so it does not deliver the PR body's "assert the skip path never invokes a DeePMD training or freeze command".

I injected return False at the top of RunDPTrain.skip_training so the skip never fires. The test does fail - but at ret, out, err = run_command(command) with ValueError: not enough values to unpack (expected 3, got 0), because a bare MagicMock yields nothing. Line 993 is never reached. Adding mocked_run.return_value = (0, "", "") moves the failure earlier still, to the pre-existing assertEqual on out["model"]. The reason generalises: the skip branch returns init_model while the training branch always names a different path, so no state exists where run_command was called and the preceding assertions passed.

Running the same mutation against the pre-PR version of this test produces an identical failure set, via FileNotFoundError: No such file or directory: 'dp' - so the new line adds no detection power.

The @patch decorator itself is a real improvement and worth keeping: deepmd-kit is not in the test extra, so dp is absent in CI, but on a developer machine that has it installed the pre-PR test would have shelled out to a real dp train.

Master's sibling test test_exec_v2_fully_empty_training_systems shows the working pattern - it sets mocked_run.side_effect = [(0, "foo\n", ""), (0, "bar\n", "")] before execute() and asserts immediately after, which is why its own assert_not_called() reports properly ("Expected 'run_command' to not have been called. Called 2 times.").


os.remove(self.init_model)

Expand Down