BUG: mismanagement of `MLFF` enum in `ase_calculator`

[gpetretto]

Describe the bug
This is probably a minor issue, since most of the ase calculators are managed through subclasses of the ForceFieldMDMaker, however the ase_calculator does not work as expected from the definition of the API, since passing an MLFF instance is not properly recognized.
The reason is this check:

atomate2/src/atomate2/forcefields/utils.py

Line 45 in 0e50e07

if isinstance(calculator_meta, str | MLFF) and calculator_meta in map(str, MLFF):

Since str(MLFF.MACE) gives 'MLFF.MACE', this check translates in something like MLFF.MACE in ["MLFF.FACE", ...] which resolves to False.

Overall the ForceFieldMDMaker works because it always converts MLFF to a string:

atomate2/src/atomate2/forcefields/md.py

Line 160 in 0e50e07

str(self.force_field_name), # make mypy happy

I am not opening a PR because I am not sure how you would rather address this. In principle changing the code so that it accepts the string "MACE" instead of "MLFF.MACE" may be a backward incompatible change.

To Reproduce

from atomate2.forcefields import MLFF
from atomate2.forcefields.utils import ase_calculator

ase_calculator(MLFF.MACE) # raises an error: ValueError: Could not create ASE calculator for MLFF.MACE.
ase_calculator("MACE") # raises an error: ValueError: Could not create ASE calculator for MLFF.MACE.
ase_calculator(str(MLFF.MACE)) # works
ase_calculator("MLFF.MACE") # works

[JaGeo]

@gpetretto I am also not entirely sure how to solve in the best way.

Can we accept both "MLFF.MACE" and "MACE"?

[gpetretto]

Probably that's a good solution.
In a sense I imagine this can be considered a bug, as I expect that when it was implemented it was thought to be only "MACE". so one can also decide of switching entirely to that. However, I would be more considerate in not breaking previously existing code.

Note also that MLFF have this TODO:

atomate2/src/atomate2/forcefields/__init__.py

Line 13 in 0e50e07

class MLFF(Enum): # TODO inherit from StrEnum when 3.11+

And if switched to StrEnum it will also become that str(MLFF.MACE) will be converted to just "MACE".

[esoteric-ephemera]

@gpetretto thanks for catching this - I'm currently working on some other fixes / changes for the forcefields and will add this in

[gpetretto]

Thanks!

[esoteric-ephemera]

@gpetretto would you mind checking out #1220 and seeing if ase_calculator is working as you expect?