dpgen2/flow/dpgen_loop.py (1)

500-503: ⚠️ Potential issue

Fix equality comparisons to True.

Avoid explicit equality comparisons to True as flagged by static analysis.

Apply this diff to fix the style issues:

     steps.outputs.parameters[
         "exploration_scheduler"
     ].value_from_expression = if_expression(
-        _if=(scheduler_step.outputs.parameters["converged"] == True),
+        _if=scheduler_step.outputs.parameters["converged"],
         _then=scheduler_step.outputs.parameters["exploration_scheduler"],
         _else=next_step.outputs.parameters["exploration_scheduler"],
     )

Apply similar changes to lines 513, 521, and 529.

Also applies to: 513-516, 521-524, 529-532

🧰 Tools

🪛 Ruff (0.11.9)

500-500: Avoid equality comparisons to True; use if scheduler_step.outputs.parameters["converged"]: for truth checks

Replace with scheduler_step.outputs.parameters["converged"]

(E712)

dpgen2/exploration/task/lmp/lmp_input.py (2)

53-60: 🛠️ Refactor suggestion

Parameter needs typing + logic guard around electron-temperature check

nvnmd_version is introduced but

1. it is un-typed (Any). Add Optional[str] to make the API explicit.
2. The electron-temperature runtime guard just above (lines 57-63) still fires when nvnmd_version is set, although the ELE_TEMP path is later disabled.
   That prevents users from mixing nvnmd with an old DeepMD build even though DeepMD is not in use.

-    deepmd_version="2.0",
-    nvnmd_version=None,
+    deepmd_version: str = "2.0",
+    nvnmd_version: Optional[str] = None,
 )
@@
-    if (ele_temp_f is not None or ele_temp_a is not None) and Version(
-        deepmd_version
-    ) < Version("1"):
+    # DeepMD-specific restriction – skip when running NVNMD
+    if (
+        nvnmd_version is None
+        and (ele_temp_f is not None or ele_temp_a is not None)
+        and Version(deepmd_version) < Version("1")
+    ):

---

140-149: ⚠️ Potential issue

Dump-file naming is inconsistent with later rerun

When trj_seperate_files is False (default) the dump is written to
dpgen.lammpstrj, but the rerun section looks for dpgen.lammpstrj.0.
This will raise ERROR: Cannot open dump file … in LAMMPS.

-        ret += "dump            1 all custom ${DUMP_FREQ} %s id type x y z fx fy fz\n" \
-            % lmp_traj_file_name
+        # keep the filename consistent with the subsequent `rerun`
+        dump_target = f"{lmp_traj_file_name}.0" if nvnmd_version else lmp_traj_file_name
+        ret += (
+            "dump            1 all custom ${DUMP_FREQ} %s id type x y z fx fy fz\n"
+            % dump_target
+        )

dpgen2/exploration/task/make_task_group_from_config.py (2)

313-318: Same documentation concern as above.

---

661-670: Same hardcoded version issue.

dpgen2/superop/__init__.py (1)

19-21: Suppress or export the newly-imported PrepRunNvNMDTrain to avoid Ruff F401

Ruff flags this symbol as “imported but unused”.
Two quick fixes:

-from .prep_run_nvnmd_train import (
-    PrepRunNvNMDTrain,
-)
+# noqa silences Ruff while keeping the import-side effect
+from .prep_run_nvnmd_train import (
+    PrepRunNvNMDTrain,  # noqa: F401
+)

or explicitly export it:

 __all__ = [
     ...,
+    "PrepRunNvNMDTrain",
 ]

Pick one to keep CI green and signal the public API.

🧰 Tools

🪛 Ruff (0.11.9)

20-20: .prep_run_nvnmd_train.PrepRunNvNMDTrain imported but unused; consider removing, adding to __all__, or using a redundant alias

(F401)

dpgen2/op/__init__.py (1)

25-27: Mark new NVNMD operators as public or silence Ruff F401

PrepNvNMDTrain, RunNvNMD, and RunNvNMDTrain are imported only for side-effects / re-export.
Add them to __all__ or append # noqa: F401 to each line to prevent the unused-import warning.

-from .prep_nvnmd_train import (
-    PrepNvNMDTrain,
-)
+from .prep_nvnmd_train import (
+    PrepNvNMDTrain,  # noqa: F401
+)
 ...
-from .run_nvnmd import (
-    RunNvNMD,
-)
-from .run_nvnmd_train import (
-    RunNvNMDTrain,
-)
+from .run_nvnmd import (
+    RunNvNMD,  # noqa: F401
+)
+from .run_nvnmd_train import (
+    RunNvNMDTrain,  # noqa: F401
+)

Alternatively extend __all__:

__all__.extend(
    ["PrepNvNMDTrain", "RunNvNMD", "RunNvNMDTrain"]
)

Keeps static analysis quiet and clarifies the public surface.

Also applies to: 44-49

🧰 Tools

🪛 Ruff (0.11.9)

26-26: .prep_nvnmd_train.PrepNvNMDTrain imported but unused; consider removing, adding to __all__, or using a redundant alias

(F401)

dpgen2/utils/download_dpgen2_artifacts.py (1)

57-59: Checkpoint files are now downloaded flat – consider optional sub-foldering

The three new ckpt artifacts are added with no suffix, so they will be placed directly under
.../inputs and .../outputs.
If the number of artifacts keeps growing, grouping them (e.g. suffix="ckpts") could keep the
download directory tidy and backward-compatible:

-.add_input("init_models_ckpt_meta")
-.add_input("init_models_ckpt_data")
-.add_input("init_models_ckpt_index")
+ .add_input("init_models_ckpt_meta",  "ckpts")
+ .add_input("init_models_ckpt_data",  "ckpts")
+ .add_input("init_models_ckpt_index", "ckpts")

Same for outputs.
Not blocking, just a maintainability thought.

Also applies to: 64-66

dpgen2/exploration/task/npt_task_group.py (1)

52-54: Parameter nvnmd_version added – update docstring & default propagation

1. The docstring of set_md still omits the new nvnmd_version argument; please add it for
   completeness.
2. self.nvnmd_version is forwarded to make_lmp_input, which expects the same kw-name
   (nvnmd_version=...). Double-check that make_lmp_input indeed accepts this parameter; mismatched
   names will raise TypeError at runtime.
3. No behavioural issues spotted otherwise.

Also applies to: 77-78, 137-138

dpgen2/entrypoint/args.py (3)

133-140: doc_config is unused – delete to silence Ruff F841

-    doc_numb_models = "Number of models trained for evaluating the model deviation"
-    doc_config = "Configuration of training"
+    doc_numb_models = "Number of models trained for evaluating the model deviation"

Removing the dead variable avoids the warning and keeps the doc-block tidy.

🧰 Tools

🪛 Ruff (0.11.9)

135-135: Local variable doc_config is assigned to but never used

Remove assignment to unused variable doc_config

(F841)

---

141-176: nvnmd_train_args() duplicates dp_train_args() – consider factoring

nvnmd_train_args() is almost a verbatim copy of dp_train_args() with only the
RunNvNMDTrain class swapped in. Duplicating ~40 lines now means two places to
patch for every future schema tweak.

A minimal refactor could delegate to a helper that injects the operator class:

def _generic_train_args(op_cls):
    base = dp_train_args()
    base[0] = Argument("config", dict, op_cls.training_args(),
                       optional=True,
                       default=op_cls.normalize_config({}),
                       doc="Configuration of training")
    return base

def nvnmd_train_args():
    return _generic_train_args(RunNvNMDTrain)

Keeps behaviour identical while eliminating code drift.

---

504-509: Docstring for "nvnmd" exploration still says “LAMMPS”

doc_lmp is re-used for the new "nvnmd" variant, which may mislead users
reading the generated docs/help. Provide a dedicated doc_nvnmd or amend
doc_lmp to cover both cases.

dpgen2/exploration/render/traj_render_lammps.py (1)

121-122: traj_fmt is now unused after the ASE rewrite

traj_fmt became dead code once the direct dpdata.System call was removed.
Drop it to avoid confusion.

-        traj_fmt = "lammps/dump"

🧰 Tools

🪛 Ruff (0.11.9)

121-121: Local variable traj_fmt is assigned to but never used

Remove assignment to unused variable traj_fmt

(F841)

dpgen2/op/prep_nvnmd_train.py (2)

7-11: Remove unused import Tuple

Tuple isn’t referenced in the module.

-from typing import (
-    List,
-    Tuple,
-    Union,
-)
+from typing import (
+    List,
+    Union,
+)

🧰 Tools

🪛 Ruff (0.11.9)

9-9: typing.Tuple imported but unused

Remove unused import: typing.Tuple

(F401)

---

82-88: Prefer isinstance over type(...) != list

The current check trips static analysers (E721) and fails on subclasses:

-        if type(template) != list:
+        if not isinstance(template, list):

🧰 Tools

🪛 Ruff (0.11.9)

82-82: Use is and is not for type comparisons, or isinstance() for isinstance checks

(E721)

tests/op/test_prep_nvnmd_train.py (1)

8-15: Prune unused imports to keep the test lean

numpy, several dflow.python symbols, and .context.dpgen2 are never used
inside this test module.

-import numpy as np
-from dflow.python import (
-    OP,
-    OPIO,
-    Artifact,
-    OPIOSign,
-)
-from .context import (
-    dpgen2,
-)
+from dflow.python import OPIO

Removing them silences Ruff F401 and speeds import time.

🧰 Tools

🪛 Ruff (0.11.9)

8-8: numpy imported but unused

Remove unused import: numpy

(F401)

---

10-10: dflow.python.OP imported but unused

Remove unused import

(F401)

---

12-12: dflow.python.Artifact imported but unused

Remove unused import

(F401)

---

13-13: dflow.python.OPIOSign imported but unused

Remove unused import

(F401)

dpgen2/exploration/task/make_task_group_from_config.py (1)

300-306: Consider adding NVNMD-specific documentation.

The new variant uses doc_lmp_md documentation, which doesn't mention NVNMD functionality. Consider creating a separate documentation string that explains the NVNMD-specific behavior.

dpgen2/superop/block.py (1)

224-273: Consider refactoring to reduce code duplication.

While the implementation is correct, there's significant duplication between the two branches. The main differences are the step name and the additional checkpoint artifacts for PrepRunNvNMDTrain.

Consider extracting common parameters and artifacts:

+    # Common parameters
+    common_parameters = {
+        "block_id": block_steps.inputs.parameters["block_id"],
+        "train_config": block_steps.inputs.parameters["train_config"],
+        "numb_models": block_steps.inputs.parameters["numb_models"],
+        "template_script": block_steps.inputs.parameters["template_script"],
+        "run_optional_parameter": run_dp_train_optional_parameter,
+    }
+    
+    # Common artifacts
+    common_artifacts = {
+        "init_models": block_steps.inputs.artifacts["init_models"],
+        "init_data": block_steps.inputs.artifacts["init_data"],
+        "iter_data": block_steps.inputs.artifacts["iter_data"],
+    }
+
     if isinstance(prep_run_dp_train_op, PrepRunNvNMDTrain):
+        # Add checkpoint artifacts for NVNMD
+        nvnmd_artifacts = {
+            "init_models_ckpt_index": block_steps.inputs.artifacts["init_models_ckpt_index"],
+            "init_models_ckpt_data": block_steps.inputs.artifacts["init_models_ckpt_data"],
+            "init_models_ckpt_meta": block_steps.inputs.artifacts["init_models_ckpt_meta"],
+        }
         prep_run_dp_train = Step(
             name + "-prep-run-nvnmd-train",
             template=prep_run_dp_train_op,
-            parameters={
-                "block_id": block_steps.inputs.parameters["block_id"],
-                "train_config": block_steps.inputs.parameters["train_config"],
-                "numb_models": block_steps.inputs.parameters["numb_models"],
-                "template_script": block_steps.inputs.parameters["template_script"],
-                "run_optional_parameter": run_dp_train_optional_parameter,
-            },
-            artifacts={
-                "init_models": block_steps.inputs.artifacts["init_models"],
-                "init_models_ckpt_index": block_steps.inputs.artifacts[
-                    "init_models_ckpt_index"
-                ],
-                "init_models_ckpt_data": block_steps.inputs.artifacts[
-                    "init_models_ckpt_data"
-                ],
-                "init_models_ckpt_meta": block_steps.inputs.artifacts[
-                    "init_models_ckpt_meta"
-                ],
-                "init_data": block_steps.inputs.artifacts["init_data"],
-                "iter_data": block_steps.inputs.artifacts["iter_data"],
-            },
+            parameters=common_parameters,
+            artifacts={**common_artifacts, **nvnmd_artifacts},
             key="--".join(
                 ["%s" % block_steps.inputs.parameters["block_id"], "prep-run-train"]
             ),
         )
     else:
         prep_run_dp_train = Step(
             name + "-prep-run-dp-train",
             template=prep_run_dp_train_op,
-            parameters={
-                "block_id": block_steps.inputs.parameters["block_id"],
-                "train_config": block_steps.inputs.parameters["train_config"],
-                "numb_models": block_steps.inputs.parameters["numb_models"],
-                "template_script": block_steps.inputs.parameters["template_script"],
-                "run_optional_parameter": run_dp_train_optional_parameter,
-            },
-            artifacts={
-                "init_models": block_steps.inputs.artifacts["init_models"],
-                "init_data": block_steps.inputs.artifacts["init_data"],
-                "iter_data": block_steps.inputs.artifacts["iter_data"],
-            },
+            parameters=common_parameters,
+            artifacts=common_artifacts,
             key="--".join(
                 ["%s" % block_steps.inputs.parameters["block_id"], "prep-run-train"]
             ),
         )

tests/test_prep_run_nvnmd_train.py (1)

101-101: Convert lambda to function definition.

Replace the lambda expression with a proper function definition for better readability.

-    revised_fname = lambda ff: Path(ff).name if only_check_name else Path(ff)
+    def revised_fname(ff):
+        return Path(ff).name if only_check_name else Path(ff)

🧰 Tools

🪛 Ruff (0.11.9)

101-101: Do not assign a lambda expression, use a def

Rewrite revised_fname as a def

(E731)

tests/op/test_run_nvnmd_train.py (1)

609-625: Remove unused variable assignments in exception handling.

The variables ee and out are assigned but never used.

         ptrain = RunNvNMDTrain()
-        with self.assertRaises(FatalError) as ee:
-            out = ptrain.execute(
+        with self.assertRaises(FatalError):
+            ptrain.execute(
                 OPIO(
                     {
                         "config": config,
                         "task_name": task_name,
                         "task_path": Path(task_path),
                         "init_model": Path(self.init_model),
                         "init_model_ckpt_meta": Path(self.init_model_ckpt_meta),
                         "init_model_ckpt_data": Path(self.init_model_ckpt_data),
                         "init_model_ckpt_index": Path(self.init_model_ckpt_index),
                         "init_data": [Path(ii) for ii in self.init_data],
                         "iter_data": [Path(ii) for ii in self.iter_data],
                     }
                 )
             )

🧰 Tools

🪛 Ruff (0.11.9)

610-610: Local variable ee is assigned to but never used

Remove assignment to unused variable ee

(F841)

---

611-611: Local variable out is assigned to but never used

Remove assignment to unused variable out

(F841)

dpgen2/flow/dpgen_loop.py (1)

458-495: Consider extracting checkpoint artifact checking logic.

The conditional logic for checking checkpoint artifacts is correct but verbose. Consider extracting this pattern into a helper function for better readability.

Add a helper function at the module level:

def _has_checkpoint_artifacts(artifacts):
    """Check if all checkpoint artifacts have path attributes."""
    return (
        hasattr(artifacts["models_ckpt_meta"]._from, "path")
        and hasattr(artifacts["models_ckpt_data"]._from, "path")
        and hasattr(artifacts["models_ckpt_index"]._from, "path")
    )

Then simplify the conditional:

-    if (
-        hasattr(block_step.outputs.artifacts["models_ckpt_meta"]._from, "path")
-        and hasattr(block_step.outputs.artifacts["models_ckpt_data"]._from, "path")
-        and hasattr(block_step.outputs.artifacts["models_ckpt_index"]._from, "path")
-    ):
+    if _has_checkpoint_artifacts(block_step.outputs.artifacts):

dpgen2/op/run_nvnmd.py (2)

14-15: Remove unused imports.

Several imports are not used in the code and should be removed to keep the module clean.

Remove these unused imports:

 from typing import (
     List,
     Optional,
-    Set,
-    Tuple,
 )

 from dargs import (
     Argument,
-    ArgumentEncoder,
-    Variant,
-    dargs,
 )

 from dflow.python import (
     OP,
     OPIO,
     Artifact,
     BigParameter,
-    FatalError,
-    HDF5Datasets,
     OPIOSign,
     TransientError,
 )

 from dpgen2.constants import (
-    lmp_conf_name,
     lmp_input_name,
     lmp_log_name,
     lmp_model_devi_name,

Also applies to: 24-26, 33-34, 40-40

🧰 Tools

🪛 Ruff (0.11.9)

14-14: typing.Set imported but unused

Remove unused import

(F401)

---

15-15: typing.Tuple imported but unused

Remove unused import

(F401)

---

345-345: Remove unnecessary f-string prefix.

The string doesn't contain any placeholders.

-            raise RuntimeError(f"last matching index should not be -1, terribly wrong ")
+            raise RuntimeError("last matching index should not be -1, terribly wrong ")

🧰 Tools

🪛 Ruff (0.11.9)

345-345: f-string without any placeholders

Remove extraneous f prefix

(F541)

dpgen2/superop/prep_run_nvnmd_train.py (1)

1-1: Remove unused imports.

Multiple imports are not used and should be removed.

Remove these unused imports:

-import json
 import os
 from copy import (
     deepcopy,
 )
-from pathlib import (
-    Path,
-)
 from typing import (
     List,
     Optional,
-    Set,
     Type,
 )

 from dflow import (
     InputArtifact,
     InputParameter,
     Inputs,
     OutputArtifact,
     OutputParameter,
     Outputs,
     S3Artifact,
     Step,
     Steps,
-    Workflow,
     argo_len,
-    argo_range,
     argo_sequence,
-    download_artifact,
     upload_artifact,
 )
 from dflow.python import (
     OP,
-    OPIO,
-    Artifact,
-    BigParameter,
-    OPIOSign,
     PythonOPTemplate,
     Slices,
 )

 from dpgen2.constants import (
     train_index_pattern,
-    train_script_name,
-    train_task_pattern,
 )

Also applies to: 7-7, 12-12, 26-26, 28-28, 30-30, 35-38, 45-46

🧰 Tools

🪛 Ruff (0.11.9)

1-1: json imported but unused

Remove unused import: json

(F401)

dpgen2/entrypoint/submit.py (1)

313-318: Simplify conditional by removing unnecessary elif.

The elif is unnecessary after a return statement.

Apply this diff:

     if explore_style in ("lmp", "nvnmd"):
         return make_lmp_naive_exploration_scheduler(config)
-    elif "calypso" in explore_style or explore_style == "diffcsp":
+    if "calypso" in explore_style or explore_style == "diffcsp":
         return make_naive_exploration_scheduler_without_conf(config, explore_style)
     else:
         raise KeyError(f"Unknown explore_style `{explore_style}`")

🧰 Tools

🪛 Pylint (3.3.7)

[refactor] 313-318: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

tests/mocked_ops.py (2)

413-546: Good implementation with minor cleanup opportunities.

The mocked NvNMD training operator correctly handles the specific directory structure and checkpoint files. However, there are some unused variables that could be cleaned up.

Remove or use the unused variables flagged by static analysis:

-        task_id = int(ip["task_name"].split(".")[1])
         assert ip["task_name"] in str(ip["task_path"])
         assert "model" in str(ip["init_model"])
         assert ".pb" in str(ip["init_model"])

And at line 448:

         init_model = init_model.resolve()
-        init_model_str = str(init_model)

🧰 Tools

🪛 Ruff (0.11.9)

436-436: Local variable task_id is assigned to but never used

Remove assignment to unused variable task_id

(F841)

---

448-448: Local variable init_model_str is assigned to but never used

Remove assignment to unused variable init_model_str

(F841)

🪛 Pylint (3.3.7)

[refactor] 415-415: Too many local variables (32/15)

(R0914)

---

[refactor] 415-415: Too many statements (100/50)

(R0915)

---

753-828: Good implementation with minor cleanup opportunities.

The mocked NvNMD operator correctly simulates LAMMPS execution with neural network models.

Remove unused variables:

-        task_id = int(ip["task_name"].split(".")[1])
         assert task_path.is_dir()
         assert ip["task_name"] in str(ip["task_path"])

And at line 780:

         models = [ii.resolve() for ii in models]
-        models_str = [str(ii) for ii in models]

🧰 Tools

🪛 Ruff (0.11.9)

765-765: Local variable task_id is assigned to but never used

Remove assignment to unused variable task_id

(F841)

---

780-780: Local variable models_str is assigned to but never used

Remove assignment to unused variable models_str

(F841)

🪛 Pylint (3.3.7)

[refactor] 755-755: Too many local variables (17/15)

(R0914)

tests/test_prep_run_nvnmd.py (2)

1-69: Clean up unused imports.

This file has many unused imports that should be removed for cleaner code.

Remove the following unused imports:

-import json
 import os
-import pickle
 import shutil
 import time
 import unittest
 from pathlib import (
     Path,
 )
 from typing import (
     List,
-    Set,
 )

-import jsonpickle
-import numpy as np
 from dflow import (
-    InputArtifact,
-    InputParameter,
-    Inputs,
-    OutputArtifact,
-    OutputParameter,
-    Outputs,
-    S3Artifact,
     Step,
-    Steps,
     Workflow,
-    argo_range,
     download_artifact,
     upload_artifact,
 )
 from dflow.python import (
-    OP,
     OPIO,
-    Artifact,
-    OPIOSign,
-    PythonOPTemplate,
 )

Also remove unused constants imports:

 from dpgen2.constants import (
     lmp_conf_name,
     lmp_input_name,
     lmp_log_name,
     lmp_model_devi_name,
     lmp_task_pattern,
     lmp_traj_name,
-    model_name_pattern,
-    train_log_name,
-    train_script_name,
-    train_task_pattern,
 )

🧰 Tools

🪛 Ruff (0.11.9)

1-1: json imported but unused

Remove unused import: json

(F401)

---

3-3: pickle imported but unused

Remove unused import: pickle

(F401)

---

12-12: typing.Set imported but unused

Remove unused import: typing.Set

(F401)

---

15-15: jsonpickle imported but unused

Remove unused import: jsonpickle

(F401)

---

16-16: numpy imported but unused

Remove unused import: numpy

(F401)

---

18-18: dflow.InputArtifact imported but unused

Remove unused import

(F401)

---

19-19: dflow.InputParameter imported but unused

Remove unused import

(F401)

---

20-20: dflow.Inputs imported but unused

Remove unused import

(F401)

---

21-21: dflow.OutputArtifact imported but unused

Remove unused import

(F401)

---

22-22: dflow.OutputParameter imported but unused

Remove unused import

(F401)

---

23-23: dflow.Outputs imported but unused

Remove unused import

(F401)

---

24-24: dflow.S3Artifact imported but unused

Remove unused import

(F401)

---

26-26: dflow.Steps imported but unused

Remove unused import

(F401)

---

28-28: dflow.argo_range imported but unused

Remove unused import

(F401)

---

33-33: dflow.python.OP imported but unused

Remove unused import

(F401)

---

35-35: dflow.python.Artifact imported but unused

Remove unused import

(F401)

---

36-36: dflow.python.OPIOSign imported but unused

Remove unused import

(F401)

---

37-37: dflow.python.PythonOPTemplate imported but unused

Remove unused import

(F401)

---

40-46: Use contextlib.suppress(ModuleNotFoundError) instead of try-except-pass

(SIM105)

---

42-42: context.dpgen2 imported but unused; consider using importlib.util.find_spec to test for availability

(F401)

---

66-66: dpgen2.constants.model_name_pattern imported but unused

Remove unused import

(F401)

---

67-67: dpgen2.constants.train_log_name imported but unused

Remove unused import

(F401)

---

68-68: dpgen2.constants.train_script_name imported but unused

Remove unused import

(F401)

---

69-69: dpgen2.constants.train_task_pattern imported but unused

Remove unused import

(F401)

---

40-46: Use contextlib.suppress for cleaner exception handling.

Replace the try-except-pass pattern:

-try:
-    from context import (
-        dpgen2,
-    )
-except ModuleNotFoundError:
-    # case of upload everything to argo, no context needed
-    pass
+from contextlib import suppress
+
+with suppress(ModuleNotFoundError):
+    from context import dpgen2

🧰 Tools

🪛 Ruff (0.11.9)

40-46: Use contextlib.suppress(ModuleNotFoundError) instead of try-except-pass

(SIM105)

---

42-42: context.dpgen2 imported but unused; consider using importlib.util.find_spec to test for availability

(F401)

tests/op/test_run_nvnmd.py (4)

1-26: Clean up unused imports.

Remove unused imports:

-import json
 import os
 import shutil
 import unittest
 from pathlib import (
     Path,
 )

 import dpdata
 import numpy as np
 from dflow.python import (
-    OP,
     OPIO,
-    Artifact,
-    OPIOSign,
     TransientError,
 )
 from mock import (
     call,
-    mock,
     patch,
 )

 # isort: off
 from .context import (
-    dpgen2,
 )

🧰 Tools

🪛 Ruff (0.11.9)

1-1: json imported but unused

Remove unused import: json

(F401)

---

12-12: dflow.python.OP imported but unused

Remove unused import

(F401)

---

14-14: dflow.python.Artifact imported but unused

Remove unused import

(F401)

---

15-15: dflow.python.OPIOSign imported but unused

Remove unused import

(F401)

---

20-20: mock.mock imported but unused

Remove unused import: mock.mock

(F401)

---

26-26: .context.dpgen2 imported but unused

Remove unused import: .context.dpgen2

(F401)

---

130-145: Remove unused variables in exception handling.

     def test_error(self, mocked_run):
         mocked_run.side_effect = [(1, "foo\n", "")]
         op = RunNvNMD()
-        with self.assertRaises(TransientError) as ee:
-            out = op.execute(
+        with self.assertRaises(TransientError):
+            op.execute(
                 OPIO(
                     {
                         "config": {"command": "mylmp"},
                         "task_name": self.task_name,
                         "task_path": self.task_path,
                         "models": self.models,
                     }
                 )
             )

🧰 Tools

🪛 Ruff (0.11.9)

134-134: Local variable ee is assigned to but never used

Remove assignment to unused variable ee

(F841)

---

135-135: Local variable out is assigned to but never used

Remove assignment to unused variable out

(F841)

---

270-270: Remove unnecessary f-string.

-            (work_dir / (model_name_pattern % ii)).read_text(), f"teacher model"
+            (work_dir / (model_name_pattern % ii)).read_text(), "teacher model"

🧰 Tools

🪛 Ruff (0.11.9)

270-270: f-string without any placeholders

Remove extraneous f prefix

(F541)

---

304-319: Remove unused exception variables.

In both test methods:

-        with self.assertRaises(RuntimeError) as re:
+        with self.assertRaises(RuntimeError):
             set_models(input_name, self.model_names)

🧰 Tools

🪛 Ruff (0.11.9)

309-309: Local variable re is assigned to but never used

Remove assignment to unused variable re

(F841)

---

317-317: Local variable re is assigned to but never used

Remove assignment to unused variable re

(F841)

dpgen2/op/run_nvnmd_train.py (4)

1-49: Clean up unused imports.

Remove unused imports:

 from typing import (
     Dict,
     List,
     Optional,
-    Tuple,
     Union,
 )

 import dpdata
 from dargs import (
     Argument,
-    ArgumentEncoder,
-    Variant,
-    dargs,
 )
 from dflow.python import (
     OP,
     OPIO,
     Artifact,
     BigParameter,
     FatalError,
     NestedDict,
     OPIOSign,
     Parameter,
-    TransientError,
 )

 from dpgen2.constants import (
     train_cnn_script_name,
     train_qnn_script_name,
     train_script_name,
-    train_task_pattern,
 )

🧰 Tools

🪛 Ruff (0.11.9)

14-14: typing.Tuple imported but unused

Remove unused import: typing.Tuple

(F401)

---

21-21: dargs.ArgumentEncoder imported but unused

Remove unused import

(F401)

---

22-22: dargs.Variant imported but unused

Remove unused import

(F401)

---

23-23: dargs.dargs imported but unused

Remove unused import

(F401)

---

34-34: dflow.python.TransientError imported but unused

Remove unused import: dflow.python.TransientError

(F401)

---

41-41: dpgen2.constants.train_task_pattern imported but unused

Remove unused import: dpgen2.constants.train_task_pattern

(F401)

---

196-199: Remove unused variable.

         config = ip["config"] if ip["config"] is not None else {}
         dp_command = ip["config"].get("command", "dp").split()
-        train_args = config.get("train_args", "")
         config = RunNvNMDTrain.normalize_config(config)

🧰 Tools

🪛 Ruff (0.11.9)

199-199: Local variable train_args is assigned to but never used

Remove assignment to unused variable train_args

(F841)

---

219-223: Use ternary operator for cleaner code.

-        if "systems" in train_dict["training"]:
-            major_version = "1"
-        else:
-            major_version = "2"
+        major_version = "1" if "systems" in train_dict["training"] else "2"

🧰 Tools

🪛 Ruff (0.11.9)

219-222: Use ternary operator major_version = "1" if "systems" in train_dict["training"] else "2" instead of if-else-block

Replace if-else-block with major_version = "1" if "systems" in train_dict["training"] else "2"

(SIM108)

---

155-388: Consider refactoring to reduce complexity.

The execute method has high complexity (45 local variables, 83 statements). While this might be necessary for the two-stage training process, consider extracting some logic into helper methods.

Would you like me to help refactor this method by extracting the CNN and QNN training logic into separate methods? This could improve readability and maintainability.

🧰 Tools

🪛 Ruff (0.11.9)

199-199: Local variable train_args is assigned to but never used

Remove assignment to unused variable train_args

(F841)

---

219-222: Use ternary operator major_version = "1" if "systems" in train_dict["training"] else "2" instead of if-else-block

Replace if-else-block with major_version = "1" if "systems" in train_dict["training"] else "2"

(SIM108)

---

272-272: Use a context manager for opening files

(SIM115)

🪛 Pylint (3.3.7)

[refactor] 155-155: Too many local variables (45/15)

(R0914)

---

[refactor] 272-272: Consider using 'with' for resource-allocating operations

(R1732)

---

[refactor] 155-155: Too many statements (83/50)

(R0915)

dpgen2/exploration/task/lmp/lmp_input.py (2)

110-118: nvnmd_version value is ignored – either use it or demote it to a boolean

The string passed via nvnmd_version is never interpolated; the code only checks for non-None.
If different NVNMD formats (e.g. pair_style nvnmd${nvnmd_version}) are ever needed, the value will be lost.
If versioning is not required, rename the parameter to use_nvnmd: bool to avoid confusion.

-elif nvnmd_version is not None:
-    ret += "pair_style      nvnmd %s\n" % ("model.pb")
+elif nvnmd_version is not None:
+    # honour explicit version if supplied, fallback to latest
+    model_file = f"model_{nvnmd_version}.pb" if nvnmd_version else "model.pb"
+    ret += f"pair_style      nvnmd {model_file}\n"

---

201-205: Execution flow: unreachable rerun block when rerun==0

The unconditional jump SELF end right after run guarantees the rerun
label is skipped when rerun==0, which is correct, but it also prevents
fall-through when jump SELF rerun is taken earlier (the script re-reads
from the top). Make this explicit to aid maintenance, e.g. comment or:

-    if nvnmd_version is not None:
-        ret += "jump SELF end\n"
+    if nvnmd_version is not None:
+        # ensure we land in the correct section after the MAIN run
+        ret += "jump SELF end\n"

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

dpgen2/exploration/task/lmp/lmp_input.py (1)

31-56: 🛠️ Refactor suggestion

Add documentation for the new nvnmd_version parameter.

The nvnmd_version parameter was added but not documented in the function signature or docstring. Please add appropriate documentation explaining its purpose and usage.

Would you like me to help add comprehensive documentation for this parameter, including its expected values and effects on the generated LAMMPS input?

🧰 Tools

🪛 Pylint (3.3.7)

[refactor] 31-31: Too many arguments (24/5)

(R0913)

---

[refactor] 31-31: Too many positional arguments (24/5)

(R0917)

---

[refactor] 31-31: Too many local variables (36/15)

(R0914)

---

[refactor] 31-31: Too many branches (35/12)

(R0912)

---

[refactor] 31-31: Too many statements (111/50)

(R0915)

dpgen2/op/prep_nvnmd_train.py (2)

7-11: Remove unused import Tuple.

Tuple is imported but never used in the file – dead code that increases maintenance cost and trips linters (cf. Ruff F401).

-from typing import (
-    List,
-    Tuple,
-    Union,
-)
+from typing import (
+    List,
+    Union,
+)

🧰 Tools

🪛 Ruff (0.11.9)

9-9: typing.Tuple imported but unused

Remove unused import: typing.Tuple

(F401)

---

82-88: Prefer isinstance over direct type comparison.

type(template) != list bypasses inheritance and is flagged by linters (E721).

-        if type(template) != list:
-            template = [template for ii in range(numb_models)]
+        if not isinstance(template, list):
+            template = [template] * numb_models

🧰 Tools

🪛 Ruff (0.11.9)

82-82: Use is and is not for type comparisons, or isinstance() for isinstance checks

(E721)

dpgen2/entrypoint/args.py (1)

133-176: doc_config defined but never used; wrong docstring passed to config argument.

Ruff flags doc_config unused. The intention is clearly to attach doc_config to the "config" argument, but doc_numb_models is supplied instead.

-        Argument(
-            "config",
-            dict,
-            RunNvNMDTrain.training_args(),
-            optional=True,
-            default=RunNvNMDTrain.normalize_config({}),
-            doc=doc_numb_models,
-        ),
+        Argument(
+            "config",
+            dict,
+            RunNvNMDTrain.training_args(),
+            optional=True,
+            default=RunNvNMDTrain.normalize_config({}),
+            doc=doc_config,
+        ),

If you decide to keep the current docstring, simply drop the unused doc_config variable to silence the linter.

🧰 Tools

🪛 Ruff (0.11.9)

135-135: Local variable doc_config is assigned to but never used

Remove assignment to unused variable doc_config

(F841)

dpgen2/exploration/task/make_task_group_from_config.py (1)

643-651: Code duplication – common branch between MD and NvNMD.

"lmp-md" and "lmp-nvnmd" share identical logic except for inserting nvnmd_version. Consider extracting a helper to reduce duplication.

tests/op/test_prep_nvnmd_train.py (2)

8-16: Prune unused imports to keep the test lean.

numpy, several dflow.python symbols and dpgen2 from .context aren’t referenced. Removing them speeds import time and silences lint warnings (F401).

-import numpy as np
-from dflow.python import (
-    OP,
-    OPIO,
-    Artifact,
-    OPIOSign,
-)
-from .context import (
-    dpgen2,
-)
+from dflow.python import OPIO

🧰 Tools

🪛 Ruff (0.11.9)

8-8: numpy imported but unused

Remove unused import: numpy

(F401)

---

10-10: dflow.python.OP imported but unused

Remove unused import

(F401)

---

12-12: dflow.python.Artifact imported but unused

Remove unused import

(F401)

---

13-13: dflow.python.OPIOSign imported but unused

Remove unused import

(F401)

---

165-169: Variable op unused – remove or assert something with it.

The execute result is stored but not consulted; either assert on it or drop the assignment.

-                op = self.ptrain.execute(ip)
+                self.ptrain.execute(ip)

🧰 Tools

🪛 Ruff (0.11.9)

167-167: Local variable op is assigned to but never used

Remove assignment to unused variable op

(F841)

dpgen2/superop/block.py (2)

224-272: Consider refactoring to reduce code duplication.

The step creation logic is mostly duplicated between the two branches. Consider extracting common parameters and artifacts to reduce duplication:

-    if isinstance(prep_run_dp_train_op, PrepRunNvNMDTrain):
-        prep_run_dp_train = Step(
-            name + "-prep-run-nvnmd-train",
-            template=prep_run_dp_train_op,
-            parameters={
-                "block_id": block_steps.inputs.parameters["block_id"],
-                "train_config": block_steps.inputs.parameters["train_config"],
-                "numb_models": block_steps.inputs.parameters["numb_models"],
-                "template_script": block_steps.inputs.parameters["template_script"],
-                "run_optional_parameter": run_dp_train_optional_parameter,
-            },
-            artifacts={
-                "init_models": block_steps.inputs.artifacts["init_models"],
-                "init_models_ckpt_index": block_steps.inputs.artifacts[
-                    "init_models_ckpt_index"
-                ],
-                "init_models_ckpt_data": block_steps.inputs.artifacts[
-                    "init_models_ckpt_data"
-                ],
-                "init_models_ckpt_meta": block_steps.inputs.artifacts[
-                    "init_models_ckpt_meta"
-                ],
-                "init_data": block_steps.inputs.artifacts["init_data"],
-                "iter_data": block_steps.inputs.artifacts["iter_data"],
-            },
-            key="--".join(
-                ["%s" % block_steps.inputs.parameters["block_id"], "prep-run-train"]
-            ),
-        )
-    else:
-        prep_run_dp_train = Step(
-            name + "-prep-run-dp-train",
-            template=prep_run_dp_train_op,
-            parameters={
-                "block_id": block_steps.inputs.parameters["block_id"],
-                "train_config": block_steps.inputs.parameters["train_config"],
-                "numb_models": block_steps.inputs.parameters["numb_models"],
-                "template_script": block_steps.inputs.parameters["template_script"],
-                "run_optional_parameter": run_dp_train_optional_parameter,
-            },
-            artifacts={
-                "init_models": block_steps.inputs.artifacts["init_models"],
-                "init_data": block_steps.inputs.artifacts["init_data"],
-                "iter_data": block_steps.inputs.artifacts["iter_data"],
-            },
-            key="--".join(
-                ["%s" % block_steps.inputs.parameters["block_id"], "prep-run-train"]
-            ),
-        )
+    # Common parameters for both training types
+    common_parameters = {
+        "block_id": block_steps.inputs.parameters["block_id"],
+        "train_config": block_steps.inputs.parameters["train_config"],
+        "numb_models": block_steps.inputs.parameters["numb_models"],
+        "template_script": block_steps.inputs.parameters["template_script"],
+        "run_optional_parameter": run_dp_train_optional_parameter,
+    }
+    
+    # Common artifacts
+    common_artifacts = {
+        "init_models": block_steps.inputs.artifacts["init_models"],
+        "init_data": block_steps.inputs.artifacts["init_data"],
+        "iter_data": block_steps.inputs.artifacts["iter_data"],
+    }
+    
+    # Determine step name and add checkpoint artifacts if needed
+    if isinstance(prep_run_dp_train_op, PrepRunNvNMDTrain):
+        step_name = name + "-prep-run-nvnmd-train"
+        common_artifacts.update({
+            "init_models_ckpt_index": block_steps.inputs.artifacts["init_models_ckpt_index"],
+            "init_models_ckpt_data": block_steps.inputs.artifacts["init_models_ckpt_data"],
+            "init_models_ckpt_meta": block_steps.inputs.artifacts["init_models_ckpt_meta"],
+        })
+    else:
+        step_name = name + "-prep-run-dp-train"
+    
+    prep_run_dp_train = Step(
+        step_name,
+        template=prep_run_dp_train_op,
+        parameters=common_parameters,
+        artifacts=common_artifacts,
+        key="--".join(["%s" % block_steps.inputs.parameters["block_id"], "prep-run-train"]),
+    )

---

224-272: Consider refactoring to reduce code duplication.

The branching logic is functionally correct but contains significant duplication. Consider extracting common elements:

+    # Common parameters for both training types
+    common_parameters = {
+        "block_id": block_steps.inputs.parameters["block_id"],
+        "train_config": block_steps.inputs.parameters["train_config"],
+        "numb_models": block_steps.inputs.parameters["numb_models"],
+        "template_script": block_steps.inputs.parameters["template_script"],
+        "run_optional_parameter": run_dp_train_optional_parameter,
+    }
+    
+    # Common artifacts
+    common_artifacts = {
+        "init_models": block_steps.inputs.artifacts["init_models"],
+        "init_data": block_steps.inputs.artifacts["init_data"],
+        "iter_data": block_steps.inputs.artifacts["iter_data"],
+    }
+    
+    # Common key
+    step_key = "--".join(["%s" % block_steps.inputs.parameters["block_id"], "prep-run-train"])
+
     if isinstance(prep_run_dp_train_op, PrepRunNvNMDTrain):
+        # Add NvNMD-specific artifacts
+        nvnmd_artifacts = {
+            "init_models_ckpt_index": block_steps.inputs.artifacts["init_models_ckpt_index"],
+            "init_models_ckpt_data": block_steps.inputs.artifacts["init_models_ckpt_data"],
+            "init_models_ckpt_meta": block_steps.inputs.artifacts["init_models_ckpt_meta"],
+        }
         prep_run_dp_train = Step(
             name + "-prep-run-nvnmd-train",
             template=prep_run_dp_train_op,
-            parameters={
-                "block_id": block_steps.inputs.parameters["block_id"],
-                "train_config": block_steps.inputs.parameters["train_config"],
-                "numb_models": block_steps.inputs.parameters["numb_models"],
-                "template_script": block_steps.inputs.parameters["template_script"],
-                "run_optional_parameter": run_dp_train_optional_parameter,
-            },
-            artifacts={
-                "init_models": block_steps.inputs.artifacts["init_models"],
-                "init_models_ckpt_index": block_steps.inputs.artifacts[
-                    "init_models_ckpt_index"
-                ],
-                "init_models_ckpt_data": block_steps.inputs.artifacts[
-                    "init_models_ckpt_data"
-                ],
-                "init_models_ckpt_meta": block_steps.inputs.artifacts[
-                    "init_models_ckpt_meta"
-                ],
-                "init_data": block_steps.inputs.artifacts["init_data"],
-                "iter_data": block_steps.inputs.artifacts["iter_data"],
-            },
-            key="--".join(
-                ["%s" % block_steps.inputs.parameters["block_id"], "prep-run-train"]
-            ),
+            parameters=common_parameters,
+            artifacts={**common_artifacts, **nvnmd_artifacts},
+            key=step_key,
         )
     else:
         prep_run_dp_train = Step(
             name + "-prep-run-dp-train",
             template=prep_run_dp_train_op,
-            parameters={
-                "block_id": block_steps.inputs.parameters["block_id"],
-                "train_config": block_steps.inputs.parameters["train_config"],
-                "numb_models": block_steps.inputs.parameters["numb_models"],
-                "template_script": block_steps.inputs.parameters["template_script"],
-                "run_optional_parameter": run_dp_train_optional_parameter,
-            },
-            artifacts={
-                "init_models": block_steps.inputs.artifacts["init_models"],
-                "init_data": block_steps.inputs.artifacts["init_data"],
-                "iter_data": block_steps.inputs.artifacts["iter_data"],
-            },
-            key="--".join(
-                ["%s" % block_steps.inputs.parameters["block_id"], "prep-run-train"]
-            ),
+            parameters=common_parameters,
+            artifacts=common_artifacts,
+            key=step_key,
         )

dpgen2/flow/dpgen_loop.py (1)

500-500: Fix equality comparisons to True.

Direct comparisons to True are not Pythonic and flagged by static analysis:

     steps.outputs.parameters[
         "exploration_scheduler"
     ].value_from_expression = if_expression(
-        _if=(scheduler_step.outputs.parameters["converged"] == True),
+        _if=scheduler_step.outputs.parameters["converged"],
         _then=scheduler_step.outputs.parameters["exploration_scheduler"],
         _else=next_step.outputs.parameters["exploration_scheduler"],
     )

Apply similar changes to lines 513, 521, and 529.

Also applies to: 513-513, 521-521, 529-529

🧰 Tools

🪛 Ruff (0.11.9)

500-500: Avoid equality comparisons to True; use if scheduler_step.outputs.parameters["converged"]: for truth checks

Replace with scheduler_step.outputs.parameters["converged"]

(E712)

tests/mocked_ops.py (4)

436-436: Remove unused variables.

The variables task_id and init_model_str are assigned but never used.

-        task_id = int(ip["task_name"].split(".")[1])
         assert ip["task_name"] in str(ip["task_path"])
         assert "model" in str(ip["init_model"])
         assert ".pb" in str(ip["init_model"])

         script = Path(script).resolve()
         init_model = init_model.resolve()
-        init_model_str = str(init_model)
         init_model_ckpt_meta = init_model_ckpt_meta.resolve()

Also applies to: 448-448

🧰 Tools

🪛 Ruff (0.11.9)

436-436: Local variable task_id is assigned to but never used

Remove assignment to unused variable task_id

(F841)

---

588-588: Remove unused variable.

-        task_id = int(ip["task_name"].split(".")[1])
         assert ip["task_name"] in str(ip["task_path"])

🧰 Tools

🪛 Ruff (0.11.9)

588-588: Local variable task_id is assigned to but never used

Remove assignment to unused variable task_id

(F841)

---

765-765: Remove unused variables.

-        task_id = int(ip["task_name"].split(".")[1])
         assert task_path.is_dir()

         task_path = task_path.resolve()
         models = [ii.resolve() for ii in models]
-        models_str = [str(ii) for ii in models]

Also applies to: 780-780

🧰 Tools

🪛 Ruff (0.11.9)

765-765: Local variable task_id is assigned to but never used

Remove assignment to unused variable task_id

(F841)

---

753-828: Consider refactoring to reduce code duplication.

The MockedRunNvNMD class is nearly identical to MockedRunLmp. Consider extracting common logic into a base class or shared method to improve maintainability.

Would you like me to help create a base class that both MockedRunLmp and MockedRunNvNMD can inherit from to eliminate the code duplication?

🧰 Tools

🪛 Ruff (0.11.9)

765-765: Local variable task_id is assigned to but never used

Remove assignment to unused variable task_id

(F841)

---

780-780: Local variable models_str is assigned to but never used

Remove assignment to unused variable models_str

(F841)

🪛 Pylint (3.3.7)

[refactor] 755-755: Too many local variables (17/15)

(R0914)

dpgen2/superop/prep_run_nvnmd_train.py (1)

1-1: Remove unused imports.

The following imports are not used in the code:

• json
• Path
• Set
• Workflow
• argo_range
• download_artifact
• OPIO, Artifact, BigParameter, OPIOSign
• train_script_name, train_task_pattern

-import json
 import os
 from copy import (
     deepcopy,
 )
-from pathlib import (
-    Path,
-)
 from typing import (
     List,
     Optional,
-    Set,
     Type,
 )

 from dflow import (
     InputArtifact,
     InputParameter,
     Inputs,
     OutputArtifact,
     OutputParameter,
     Outputs,
     S3Artifact,
     Step,
     Steps,
-    Workflow,
     argo_len,
-    argo_range,
     argo_sequence,
-    download_artifact,
     upload_artifact,
 )
 from dflow.python import (
     OP,
-    OPIO,
-    Artifact,
-    BigParameter,
-    OPIOSign,
     PythonOPTemplate,
     Slices,
 )

 from dpgen2.constants import (
     train_index_pattern,
-    train_script_name,
-    train_task_pattern,
 )

Also applies to: 7-7, 12-12, 26-26, 28-28, 30-30, 35-38, 45-46

🧰 Tools

🪛 Ruff (0.11.9)

1-1: json imported but unused

Remove unused import: json

(F401)

dpgen2/op/run_nvnmd.py (3)

14-15: Remove unused imports.

The following imports are not used:

• Set, Tuple from typing
• ArgumentEncoder, Variant, dargs from dargs
• FatalError, HDF5Datasets from dflow.python
• lmp_conf_name from dpgen2.constants

 from typing import (
     List,
     Optional,
-    Set,
-    Tuple,
 )

 from dargs import (
     Argument,
-    ArgumentEncoder,
-    Variant,
-    dargs,
 )
 from dflow.python import (
     OP,
     OPIO,
     Artifact,
     BigParameter,
-    FatalError,
-    HDF5Datasets,
     OPIOSign,
     TransientError,
 )

 from dpgen2.constants import (
-    lmp_conf_name,
     lmp_input_name,
     lmp_log_name,
     lmp_model_devi_name,

Also applies to: 24-26, 33-34, 40-40

🧰 Tools

🪛 Ruff (0.11.9)

14-14: typing.Set imported but unused

Remove unused import

(F401)

---

15-15: typing.Tuple imported but unused

Remove unused import

(F401)

---

345-345: Remove unnecessary f-string prefix.

-        raise RuntimeError(f"last matching index should not be -1, terribly wrong ")
+        raise RuntimeError("last matching index should not be -1, terribly wrong ")

🧰 Tools

🪛 Ruff (0.11.9)

345-345: f-string without any placeholders

Remove extraneous f prefix

(F541)

---

97-263: Consider refactoring the execute method to reduce complexity.

The execute method is handling multiple responsibilities and has become quite complex (29 local variables, 63 statements). Consider extracting helper methods for:

• Setting up input files and models
• Running LAMMPS commands
• Post-processing results

This would improve readability and maintainability.

Would you like me to help refactor this method into smaller, more focused helper methods?

🧰 Tools

🪛 Ruff (0.11.9)

155-155: Do not use bare except

(E722)

---

166-166: Do not use bare except

(E722)

🪛 Pylint (3.3.7)

[refactor] 97-97: Too many local variables (29/15)

(R0914)

---

[refactor] 97-97: Too many branches (14/12)

(R0912)

---

[refactor] 97-97: Too many statements (63/50)

(R0915)

tests/op/test_run_nvnmd.py (5)

1-1: Remove unused imports.

-import json
 import os
 import shutil
 import unittest

 from dflow.python import (
-    OP,
     OPIO,
-    Artifact,
-    OPIOSign,
     TransientError,
 )
 from mock import (
     call,
-    mock,
     patch,
 )

 # isort: off
 from .context import (
-    dpgen2,
 )

Also applies to: 12-12, 14-15, 20-20, 26-26

🧰 Tools

🪛 Ruff (0.11.9)

1-1: json imported but unused

Remove unused import: json

(F401)

---

134-135: Remove unused exception variable.

-        with self.assertRaises(TransientError) as ee:
-            out = op.execute(
+        with self.assertRaises(TransientError):
+            op.execute(

🧰 Tools

🪛 Ruff (0.11.9)

134-134: Local variable ee is assigned to but never used

Remove assignment to unused variable ee

(F841)

---

135-135: Local variable out is assigned to but never used

Remove assignment to unused variable out

(F841)

---

243-243: Remove unused variable.

-        out = op.execute(
+        op.execute(

🧰 Tools

🪛 Ruff (0.11.9)

243-243: Local variable out is assigned to but never used

Remove assignment to unused variable out

(F841)

---

309-309: Remove unused exception variables.

-        with self.assertRaises(RuntimeError) as re:
+        with self.assertRaises(RuntimeError):
             set_models(input_name, self.model_names)

-        with self.assertRaises(RuntimeError) as re:
+        with self.assertRaises(RuntimeError):
             set_models(input_name, self.model_names)

Also applies to: 317-317

🧰 Tools

🪛 Ruff (0.11.9)

309-309: Local variable re is assigned to but never used

Remove assignment to unused variable re

(F841)

---

270-270: Remove unnecessary f-string prefix.

-            (work_dir / (model_name_pattern % ii)).read_text(), f"teacher model"
+            (work_dir / (model_name_pattern % ii)).read_text(), "teacher model"

🧰 Tools

🪛 Ruff (0.11.9)

270-270: f-string without any placeholders

Remove extraneous f prefix

(F541)

tests/test_prep_run_nvnmd_train.py (1)

38-45: Simplify the dpgen2 import handling

The try-except block can be simplified since dpgen2 is not actually used. Consider using contextlib.suppress or removing this block entirely if not needed.

-try:
-    from context import (
-        dpgen2,
-    )
-except ModuleNotFoundError:
-    # case of upload everything to argo, no context needed
-    pass
+# dpgen2 import not needed in this test file

🧰 Tools

🪛 Ruff (0.11.9)

38-44: Use contextlib.suppress(ModuleNotFoundError) instead of try-except-pass

(SIM105)

---

40-40: context.dpgen2 imported but unused; consider using importlib.util.find_spec to test for availability

(F401)

dpgen2/entrypoint/submit.py (1)

211-219: Simplify conditional logic by removing elif after return

Since the previous condition returns, the elif can be replaced with if for cleaner code.

-    elif "nvnmd" in explore_style:
+    if "nvnmd" in explore_style:
         prep_run_explore_op = PrepRunLmp(
             "prep-run-nvnmd",
             PrepLmp,
             RunNvNMD,
             prep_config=prep_explore_config,
             run_config=run_explore_config,
             upload_python_packages=upload_python_packages,
         )

Also applies to: 313-318

dpgen2/op/run_nvnmd_train.py (3)

219-223: Simplify version detection with ternary operator

The if-else block can be simplified using a ternary operator.

-        if "systems" in train_dict["training"]:
-            major_version = "1"
-        else:
-            major_version = "2"
+        major_version = "1" if "systems" in train_dict["training"] else "2"

🧰 Tools

🪛 Ruff (0.11.9)

219-222: Use ternary operator major_version = "1" if "systems" in train_dict["training"] else "2" instead of if-else-block

Replace if-else-block with major_version = "1" if "systems" in train_dict["training"] else "2"

(SIM108)

---

481-494: Remove unnecessary else after return

The else block is not needed since the if block returns.

     @staticmethod
     def skip_training(
         work_dir,
         train_dict,
         init_model,
         iter_data,
     ):
         # we have init model and no iter data, skip training
         if (init_model is not None) and (iter_data is None or len(iter_data) == 0):
             with set_directory(work_dir):
                 with open(train_script_name, "w") as fp:
                     json.dump(train_dict, fp, indent=4)
                 Path("train.log").write_text(
                     f"We have init model {init_model} and "
                     f"no iteration training data. "
                     f"The training is skipped.\n"
                 )
                 Path("lcurve.out").touch()
             return True
-        else:
-            return False
+        return False

🧰 Tools

🪛 Pylint (3.3.7)

[refactor] 482-494: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

---

103-627: Consider refactoring to reduce complexity

The RunNvNMDTrain class implementation is functionally correct but has high complexity:

• The execute method has 45 local variables and 83 statements
• Consider extracting some logic into separate methods (e.g., training script preparation, CNN training, QNN training)
• This would improve maintainability and testability

Would you like me to help refactor this class to reduce complexity by breaking down the execute method into smaller, more focused methods?

🧰 Tools

🪛 Ruff (0.11.9)

199-199: Local variable train_args is assigned to but never used

Remove assignment to unused variable train_args

(F841)

---

219-222: Use ternary operator major_version = "1" if "systems" in train_dict["training"] else "2" instead of if-else-block

Replace if-else-block with major_version = "1" if "systems" in train_dict["training"] else "2"

(SIM108)

---

272-272: Use a context manager for opening files

(SIM115)

---

619-619: Do not use mutable data structures for argument defaults

Replace with None; initialize within function

(B006)

🪛 Pylint (3.3.7)

[refactor] 155-155: Too many local variables (45/15)

(R0914)

---

[refactor] 272-272: Consider using 'with' for resource-allocating operations

(R1732)

---

[refactor] 155-155: Too many statements (83/50)

(R0915)

---

[refactor] 391-391: Too many arguments (7/5)

(R0913)

---

[refactor] 391-391: Too many positional arguments (7/5)

(R0917)

---

[refactor] 430-430: Too many branches (14/12)

(R0912)

---

[refactor] 482-494: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)