Skip to content

Add GRACE - #797

Open
ElliottKasoar wants to merge 8 commits into
ddmms:mainfrom
ElliottKasoar:add-grace
Open

Add GRACE#797
ElliottKasoar wants to merge 8 commits into
ddmms:mainfrom
ElliottKasoar:add-grace

Conversation

@ElliottKasoar

Copy link
Copy Markdown
Collaborator

Pre-review checklist for PR author

PR author must check the checkboxes below when creating the PR.

  • I've confirmed the contribution guidelines.
  • I have reviewed and understand all AI-generated code in this PR.
  • I have added human-written tests for the new logic.
  • I have properly cited any upstream algorithms or libraries the AI utilized.
  • I have disclosed significant AI tool usage in the PR description.

Summary

Updates GRACE in models.yml to use the foundational model interface, allowing models to be downloaded by name, and updates the GRACE dependency.

One problem seems to be that based on their docs, float32 is the default and float64 seems to only be accessible via different model downloads. I've modified the get_calculator function to try to deal with this, which seems to work from testing the two models I've added (one of which doesn't have a fp64 variant).

Model Reference

https://github.com/ICAMS/grace-tensorpotential

Progress

  • Model configuration
  • Documentation

Testing

Tested against X23

@ElliottKasoar ElliottKasoar added the new model Proposals and suggestions for new models to test label Aug 5, 2026
Comment thread ml_peg/models/models.py
Comment on lines +508 to +509
if self.default_dtype is not None:
suffix = self.default_dtype

@joehart2001 joehart2001 Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks good apart from this. i think thsi would result in e.g. float32 rather than fp32 due to the precision map in the generic ase calc.

@ElliottKasoar ElliottKasoar Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I may be missing something but why? default_dtype defaults to None for GenericASECalc, and the precision_map is only defined within the get_calculator functions, which GRACE has its own implementation of.

I've admittedly forgotten all of the discussions about how default_dtype works, but I believe the intent is it's only usually set by an overwrite_dtype, which comes from the model-specific part of the yaml (I just fixed passing this to GRACE).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new model Proposals and suggestions for new models to test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants