search

materialsproject / atomate2

atomate2 is a library of computational materials science workflows
https://materialsproject.github.io/atomate2/
Other
148 stars 83 forks source link

`ElasticMaker` shouldn't use submaker as default factory by design #845

Open chiang-yuan opened 3 months ago

chiang-yuan commented 3 months ago

ElasticMaker shouldn't use submaker as default factory otherwise chgnet will be required to be installed as dependency

https://github.com/materialsproject/atomate2/blob/313f8098d58e3afcebd89a840aa489050afe2477/src/atomate2/forcefields/flows/elastic.py#L62-L70

utf commented 3 months ago

So currently, this doesn't actually require CHGNet to be installed I don't think. I.e., you can import this and overwrite those makers to get a MACE or M3GNet elastic constant workflow. However, it could make sense to create several versions of the workflow, one for each universal force field?

One thing I realised looking at this is that we can remove the prev_calc_dir_argname function from the BaseElasticMaker and subclasses since this has now been standardised across all codes.

JaGeo commented 3 months ago

So currently, this doesn't actually require CHGNet to be installed I don't think. I.e., you can import this and overwrite those makers to get a MACE or M3GNet elastic constant workflow. However, it could make sense to create several versions of the workflow, one for each universal force field?

One thing I realised looking at this is that we can remove the prev_calc_dir_argname function from the BaseElasticMaker and subclasses since this has now been standardised across all codes.

@utf also in the Abinit workflows?

chiang-yuan commented 3 months ago

Yes @utf you are definitely right - one could always replace default factory with the relaxmaker they like to use. However, I think this design is a bit counterintuitive and one might just want to set the forcefield calculator name and the code switch the input generator accordingly like what we did for MDMaker

Not a serious issue so maybe I shouldn't call it bug at the first place (I think it is the template..)