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 calculator lists #346

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

ElliottKasoar
Copy link
Member

Starting point for one potential solution to #240

  • Converts results from Atoms.get_property to list, with a value for each calculator
  • Wraps calculations, to ensure torch data type is set between MLIPs
    • This would need generalising to use the correct dtype, and ideally, not to be called for non-MultiCalcs, e.g. for MD with one calculator, this become a huge number of unnecessary calls, although maybe extra operations have a minimal impact?

The main issue this approach has is some built in functions, such as get_stress, check the shape of the value returned by the calculator, which means currently this throws an error.

Without overwriting this, I'm not sure this is possible to avoid.

The main other alternative I can think of is to create copies of the Atoms objects, each of which has its own calculator.

The disadvantage of this would be duplication of potentially quite large amounts of data, but it would ensure all results from calculators are returned in their expected forms, so remain useable by ASE functions.

This implementation would also make it much more readily available for distributed calculations using any of the calculations e.g. MD for MACE and CHGNet with the same settings, if we wanted to offer that, eventually.

In terms of singlepoint, we'd want to recombine the two Atoms objects when writing out, but it would allow geomopt structures to diverge, for example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant