-
Notifications
You must be signed in to change notification settings - Fork 181
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 MACE-OFF23 models #275
Conversation
Fix selection of device to allow integer
Update version to 0.3.2 for conda installation
Update version for conda release
Update to 0.3.2 to overwrite previous tag
Add small model and warning for float32 accuracy in mace_mp
update the small model to energy model
@davkovacs I think we should also add a paragraph in the readme about the models |
def mace_off( | ||
model: Union[str, Path] = None, | ||
device: str = "", | ||
default_dtype: str = "float32", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default dtype does not match the doc string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, I am updating it.
"Model download failed" | ||
) from exc | ||
|
||
device = device or ("cuda" if torch.cuda.is_available() else "cpu") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe return the raw model here as the print statements are not relevant for users that just want the model.
Ready to be merged. |
Might be good to add a test as well to make sure nothing gets broken in the future? |
You can use the same test as the mace_mp |
Can you test the tests please! @jthorton |
ready to merge? |
@ilyes319 if you are also happy with the tests and everything else, I think it is ready. |
msg = f"Using MACE-OFF23 MODEL for MACECalculator with {model}" | ||
print(msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we missed this print which should only trigger if we don't just want to return the model.
This PR implements the interface to the MACE-OFF23 models.