ACEsuit / mace

MACE - Fast and accurate machine learning interatomic potentials with higher order equivariant message passing.
Other
413 stars 157 forks source link

atomic configs with len 1 that are not IsolatedAtom are processed wrong #381

Closed bernstei closed 2 months ago

bernstei commented 2 months ago

The structure of this if statement https://github.com/ACEsuit/mace/blob/f5ab9c5d45e09ac49c7874c44a14ba83c813d5ab/mace/data/utils.py#L223 seems to ignore configs with len(atoms) == 1. It's definitely broken in terms of adding them to theories which is how I discovered it. However, it also doesn't add them to atoms_without_iso, which, if keep_isolated_atoms is False, also excludes them from the main atoms_list, from what I can tell.

I think it would be safer to have the outermost if in that loop be

    if atoms.info.get("config_type") == "IsolatedAtom":
        assert len(atoms) == 1
        # do whatever special needs to be done with IsolatedAtom