Conversation
There was a problem hiding this comment.
Pull request overview
This pull request integrates two new machine learning potential models into the MAPLE framework: CHGNet and DPA3 (Deep Potential Architecture 3). CHGNet uses a pretrained charge-informed graph neural network potential, while DPA3 requires a model file to be provided for calculations.
Changes:
- Added "dpa3" and "chgnet" to the list of supported models in command control and calculator setup
- Implemented DPA3Calculator wrapper using DeepMD-Kit's DP calculator with eV to Hartree unit conversion
- Implemented CHGNetCalc wrapper using the pretrained CHGNet model with eV to Hartree unit conversion and stress calculation support
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| maple/function/read/command_control.py | Added "dpa3" and "chgnet" to SUPPORTED_MODELS set |
| maple/function/calculator/set_calculator.py | Added "dpa3" and "chgnet" to model list and implemented calculator instantiation logic for both models |
| maple/function/calculator/deepmd/_dpa3_calculator.py | New calculator implementation for DPA3 with model path resolution, energy/force calculations, and solvent correction support |
| maple/function/calculator/deepmd/init.py | New module initialization file exporting DPA3Calculator |
| maple/function/calculator/chgnet/_chgnet_calculator.py | New calculator implementation for CHGNet with pretrained model loading, energy/force/stress calculations, and solvent correction support |
| maple/function/calculator/chgnet/init.py | New module initialization file exporting CHGNetCalc |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def __init__( | ||
| self, | ||
| device, |
There was a problem hiding this comment.
The device parameter is missing a type hint. For consistency with other calculators in the codebase (e.g., ANICalculator, MACEModelCalculator), consider adding the type hint: device: torch.device.
|
|
||
| def __init__( | ||
| self, | ||
| device, |
There was a problem hiding this comment.
The device parameter is missing a type hint. For consistency with other calculators in the codebase (e.g., ANICalculator, MACEModelCalculator), consider adding the type hint: device: torch.device.
| chgnet = CHGNet.load() | ||
| return CHGNetCalculator(potential=chgnet) |
There was a problem hiding this comment.
The device parameter is accepted but never used when creating the CHGNet calculator. CHGNet.load() and CHGNetCalculator() should be configured with the device parameter to ensure the model runs on the correct device (CPU vs GPU). The device should be passed to CHGNet.load() using the use_device parameter.
| chgnet = CHGNet.load() | |
| return CHGNetCalculator(potential=chgnet) | |
| chgnet = CHGNet.load(use_device=device) | |
| return CHGNetCalculator(potential=chgnet, use_device=device) |
| elif os.path.isfile(model): | ||
| model_path = model | ||
| else: | ||
| model_path = default_path |
There was a problem hiding this comment.
The model path resolution logic has a potential issue. If the default_path does not exist and the model parameter is neither a file nor a directory (e.g., model="dpa3"), the code will still use default_path which doesn't exist, leading to a failure when instantiating the DP calculator. Consider raising a clear error message if no valid model path is found, or ensure the model file exists before attempting to load it.
| model_path = default_path | |
| raise FileNotFoundError( | |
| f"Could not find DeepMD/DPA3 model. " | |
| f"Neither default model file '{default_path}' nor provided " | |
| f"model path '{model}' exists." | |
| ) |
| else: | ||
| model_path = default_path | ||
|
|
||
| self._dp = DP(model=model_path) |
There was a problem hiding this comment.
The device parameter is stored but never passed to the DP calculator during instantiation. DeepMD-Kit calculators typically need to be configured with the target device to ensure the model runs on the correct device (CPU vs GPU). Check if the DP calculator accepts a device parameter and pass it during instantiation, or verify that the model file already contains device information.
| self._dp = DP(model=model_path) | |
| self._dp = DP(model=model_path, device=device) |
New code to integrate CHGNet and DPa3, dpa3 needs a model to be included in order to successfully run in calculations, chgnet has a pretrained one