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.

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 possibility to use your own M3GNet potential #911

Merged
merged 6 commits into from
Jul 10, 2024

Conversation

QuantumChemist
Copy link
Contributor

@QuantumChemist QuantumChemist commented Jul 2, 2024

allow the possibility to use your own M3GNet potential, instead of the pretrained model only.

I want to run the M3GNetRelaxMaker and the M3GNetStaticMaker for the forcefield PhononMaker

class M3GNetRelaxMaker(ForceFieldRelaxMaker):

with my own trained potential (passed in the calculator_kwargs to the PhononMaker).
Therefore I added the possibility to use your own potential, instead of the pretrained model only.

allow the possibility to use your own M3GNet potential, instead of the pretrained model only.
@QuantumChemist
Copy link
Contributor Author

Hi @utf , this PR is ready for review and merging. Thanks 😄

import matgl
from matgl.ext.ase import PESCalculator

m3gnet_calculator = ase_calculator(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually want to implement it as

    m3gnet_calculator = ase_calculator(
        calculator_meta="MLFF.M3GNet",
        **{"path": "M3GNet-MP-2021.2.8-DIRECT-PES", "stress_weight": 2.0},
    )

    m3gnet_default = ase_calculator(  # uses "M3GNet-MP-2021.2.8-PES" per default
        calculator_meta="MLFF.M3GNet", **{"stress_weight": 2.0},
    )

and the test passes, but ruff-format changes it....

Copy link
Member

Choose a reason for hiding this comment

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

Just define the two dicts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup. Just noticed it after pushing 🤣 so I didn't want you to merge it like that

@JaGeo
Copy link
Member

JaGeo commented Jul 10, 2024

@QuantumChemist will merge once the tests pass

@JaGeo JaGeo enabled auto-merge (squash) July 10, 2024 15:01
@JaGeo JaGeo merged commit d30890f into materialsproject:main Jul 10, 2024
6 checks passed
BryantLi-BLI pushed a commit to BryantLi-BLI/atomate2 that referenced this pull request Jul 29, 2024
* allow the possibility to use your own M3GNet potential

allow the possibility to use your own M3GNet potential, instead of the pretrained model only.

* added a unit test

* added a unit test

* test_dir not needed

* change kwargs passing
@utf utf added the enhancement Improvements to existing features label Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants