Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RunLmpHDF5 #267

Merged
merged 5 commits into from
Oct 21, 2024
Merged

Add RunLmpHDF5 #267

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
899aaba
add RunLmpHDF5
zjgemi Oct 17, 2024
47a551d
fix: pyright and upgrade dflow dependency
zjgemi Oct 18, 2024
b25efde
fix pyright
zjgemi Oct 18, 2024
cae6577
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 18, 2024
5cf58f3
fix pyright
zjgemi Oct 21, 2024
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
3 changes: 2 additions & 1 deletion dpgen2/entrypoint/submit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
RunCalyModelDevi,
RunDPTrain,
RunLmp,
RunLmpHDF5,
RunRelax,
RunRelaxHDF5,
SelectConfs,
Expand Down Expand Up @@ -187,7 +188,7 @@ def make_concurrent_learning_op(
prep_run_explore_op = PrepRunLmp(
"prep-run-lmp",
PrepLmp,
RunLmp,
RunLmpHDF5 if explore_config["use_hdf5"] else RunLmp, # type: ignore
prep_config=prep_explore_config,
run_config=run_explore_config,
upload_python_packages=upload_python_packages,
Expand Down
2 changes: 1 addition & 1 deletion dpgen2/exploration/render/traj_render_lammps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,18 @@

def _load_one_model_devi(self, fname, model_devi):
if isinstance(fname, HDF5Dataset):
dd = fname.get_data()
dd = fname.get_data() # type: ignore

Check warning on line 63 in dpgen2/exploration/render/traj_render_lammps.py

View check run for this annotation

Codecov / codecov/patch

dpgen2/exploration/render/traj_render_lammps.py#L63 

Added line #L63 was not covered by tests
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Investigate and address the root cause of the type ignore comment.

The addition of # type: ignore suggests a potential type mismatch that's being suppressed. While this allows the code to pass type checking, it may hide underlying issues.

Consider investigating the root cause of this type mismatch. If possible, modify the code to resolve the type issue without needing to suppress the type checker. If the suppression is absolutely necessary, add a comment explaining why it's needed and any potential risks.

else:
dd = np.loadtxt(fname)
if len(np.shape(dd)) == 1: # In case model-devi.out is 1-dimensional

Check failure on line 66 in dpgen2/exploration/render/traj_render_lammps.py

View workflow job for this annotation

GitHub Actions / pyright 

Argument of type "Unknown | NDArray[float64] | None" cannot be assigned to parameter "a" of type "ArrayLike" in function "shape" (reportGeneralTypeIssues)
dd = dd.reshape((1, len(dd)))

Check failure on line 67 in dpgen2/exploration/render/traj_render_lammps.py

View workflow job for this annotation

GitHub Actions / pyright 

"reshape" is not a known member of "None" (reportOptionalMemberAccess)

Check failure on line 67 in dpgen2/exploration/render/traj_render_lammps.py

View workflow job for this annotation

GitHub Actions / pyright 

Argument of type "Unknown | NDArray[float64] | None" cannot be assigned to parameter "__obj" of type "Sized" in function "len"
  Type "Unknown | NDArray[float64] | None" cannot be assigned to type "Sized"
    "__len__" is not present (reportGeneralTypeIssues)

model_devi.add(DeviManager.MAX_DEVI_V, dd[:, 1])

Check failure on line 69 in dpgen2/exploration/render/traj_render_lammps.py

View workflow job for this annotation

GitHub Actions / pyright 

Object of type "None" is not subscriptable (reportOptionalSubscript)
model_devi.add(DeviManager.MIN_DEVI_V, dd[:, 2])

Check failure on line 70 in dpgen2/exploration/render/traj_render_lammps.py

View workflow job for this annotation

GitHub Actions / pyright 

Object of type "None" is not subscriptable (reportOptionalSubscript)
model_devi.add(DeviManager.AVG_DEVI_V, dd[:, 3])

Check failure on line 71 in dpgen2/exploration/render/traj_render_lammps.py

View workflow job for this annotation

GitHub Actions / pyright 

Object of type "None" is not subscriptable (reportOptionalSubscript)
model_devi.add(DeviManager.MAX_DEVI_F, dd[:, 4])

Check failure on line 72 in dpgen2/exploration/render/traj_render_lammps.py

View workflow job for this annotation

GitHub Actions / pyright 

Object of type "None" is not subscriptable (reportOptionalSubscript)
model_devi.add(DeviManager.MIN_DEVI_F, dd[:, 5])

Check failure on line 73 in dpgen2/exploration/render/traj_render_lammps.py

View workflow job for this annotation

GitHub Actions / pyright 

Object of type "None" is not subscriptable (reportOptionalSubscript)
model_devi.add(DeviManager.AVG_DEVI_F, dd[:, 6])

Check failure on line 74 in dpgen2/exploration/render/traj_render_lammps.py

View workflow job for this annotation

GitHub Actions / pyright 

Object of type "None" is not subscriptable (reportOptionalSubscript)

def get_ele_temp(self, optional_outputs):
ele_temp = []
Expand Down
1 change: 1 addition & 0 deletions dpgen2/op/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
)
from .run_lmp import (
RunLmp,
RunLmpHDF5,
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Define __all__ in dpgen2/op/__init__.py to include RunLmpHDF5

The __all__ variable is not defined in dpgen2/op/__init__.py. To make RunLmpHDF5 part of the public API and resolve the unused import warning, consider adding an __all__ list that includes RunLmpHDF5 along with other public classes and functions.

Example:

__all__ = [
    "CollectData",
    "CollRunCaly",
    "DiffCSPGen",
    "PrepCalyDPOptim",
    "PrepCalyInput",
    "PrepCalyModelDevi",
    "PrepDPTrain",
    "PrepLmp",
    "PrepRelax",
    "RunCalyDPOptim",
    "RunCalyModelDevi",
    "RunDPTrain",
    "RunLmp",
    "RunLmpHDF5",
    "RunRelax",
    "RunRelaxHDF5",
    "SelectConfs",
]
🔗 Analysis chain

Consider adding RunLmpHDF5 to all if it's part of the public API

The addition of RunLmpHDF5 to the imports is good, as it makes the new functionality available to other parts of the package. However, the static analysis tool has flagged this import as unused within this file.

If RunLmpHDF5 is intended to be part of the public API of this package, consider adding it to the __all__ list (if one exists) to explicitly define what should be imported when using from dpgen2.op import *.

To verify if __all__ is defined in this file and if so, what it contains, you can run the following script:

If __all__ is not defined, consider adding it to explicitly control what's exported:

__all__ = [
    "CollectData",
    "CollRunCaly",
    "DiffCSPGen",
    "PrepCalyDPOptim",
    "PrepCalyInput",
    "PrepCalyModelDevi",
    "PrepDPTrain",
    "PrepLmp",
    "PrepRelax",
    "RunCalyDPOptim",
    "RunCalyModelDevi",
    "RunDPTrain",
    "RunLmp",
    "RunLmpHDF5",
    "RunRelax",
    "RunRelaxHDF5",
    "SelectConfs",
]
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for __all__ definition in __init__.py

# Test: Search for __all__ definition
rg --type python '__all__\s*=' dpgen2/op/__init__.py

# If __all__ is not found, suggest adding it
if [ $? -ne 0 ]; then
    echo "__all__ not found. Consider adding it with RunLmpHDF5 included."
fi

Length of output: 240

Script:

#!/bin/bash
# Description: Check for __all__ definition in __init__.py without specifying file type

# Test: Search for __all__ definition
rg '__all__\s*=' dpgen2/op/__init__.py

# Capture the exit status
if [ $? -ne 0 ]; then
    echo "__all__ not found. Consider adding it with RunLmpHDF5 included."
else
    echo "__all__ is defined."
fi

Length of output: 195

🧰 Tools
🪛 Ruff

39-39: .run_lmp.RunLmpHDF5 imported but unused; consider removing, adding to __all__, or using a redundant alias

(F401)

)
from .run_relax import (
RunRelax,
Expand Down
27 changes: 26 additions & 1 deletion dpgen2/op/run_lmp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
Tuple,
)

import numpy as np
from dargs import (
Argument,
ArgumentEncoder,
Expand All @@ -26,6 +27,7 @@
Artifact,
BigParameter,
FatalError,
HDF5Datasets,
OPIOSign,
TransientError,
)
Expand Down Expand Up @@ -200,7 +202,7 @@
ret_dict = {
"log": work_dir / lmp_log_name,
"traj": work_dir / lmp_traj_name,
"model_devi": work_dir / lmp_model_devi_name,
"model_devi": self.get_model_devi(work_dir / lmp_model_devi_name),
}
plm_output = (
{"plm_output": work_dir / plm_output_name}
Expand All @@ -213,13 +215,17 @@

return OPIO(ret_dict)

def get_model_devi(self, model_devi_file):
return model_devi_file

@staticmethod
def lmp_args():
doc_lmp_cmd = "The command of LAMMPS"
doc_teacher_model = "The teacher model in `Knowledge Distillation`"
doc_shuffle_models = "Randomly pick a model from the group of models to drive theexploration MD simulation"
doc_head = "Select a head from multitask"
doc_use_ele_temp = "Whether to use electronic temperature, 0 for no, 1 for frame temperature, and 2 for atomic temperature"
doc_use_hdf5 = "Use HDF5 to store trajs and model_devis"
return [
Argument("command", str, optional=True, default="lmp", doc=doc_lmp_cmd),
Argument(
Expand All @@ -243,6 +249,13 @@
Argument(
"model_frozen_head", str, optional=True, default=None, doc=doc_head
),
Argument(
"use_hdf5",
bool,
optional=True,
default=False,
doc=doc_use_hdf5,
),
Comment on lines +252 to +258
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Implement logic for use_hdf5 in the execute method.

The use_hdf5 parameter is added, but the execute method does not currently utilize it to alter the behavior of the class. Consider implementing conditional logic in the execute method to handle cases when use_hdf5 is True, such as processing outputs differently or initializing RunLmpHDF5.

]

@staticmethod
Expand Down Expand Up @@ -374,3 +387,15 @@
for model_devi_file in sorted(model_devi_files):
with open(model_devi_file, "r") as f2:
f.write(f2.read())


class RunLmpHDF5(RunLmp):
@classmethod
def get_output_sign(cls):
output_sign = super().get_output_sign()
output_sign["traj"] = Artifact(HDF5Datasets)
output_sign["model_devi"] = Artifact(HDF5Datasets)
return output_sign

Check warning on line 398 in dpgen2/op/run_lmp.py

View check run for this annotation

Codecov / codecov/patch

dpgen2/op/run_lmp.py#L395-L398 

Added lines #L395 - L398 were not covered by tests

def get_model_devi(self, model_devi_file):
return np.loadtxt(model_devi_file)

Check warning on line 401 in dpgen2/op/run_lmp.py

View check run for this annotation

Codecov / codecov/patch

dpgen2/op/run_lmp.py#L401 

Added line #L401 was not covered by tests
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ classifiers = [
dependencies = [
'numpy',
'dpdata>=0.2.20',
'pydflow>=1.8.88',
'pydflow>=1.8.95',
'dargs>=0.3.1',
'scipy',
'lbg',
Expand Down
Loading