FEFFDictSet write_input appears to be bugged in some cases

[matthewcarbone]

Description

I've found what appears to be a strange edge case when using the FEFFDictSet, which does not appear in older versions of Pymatgen.

print(struct)  # this is mp-2664

Full Formula (Ti1 O1)
Reduced Formula: TiO
abc   :   3.033043   3.033043   3.033043
angles:  60.000000  60.000000  60.000000
pbc   :       True       True       True
Sites (2)
  #  SP      a    b    c    magmom
---  ----  ---  ---  ---  --------
  0  Ti    0    0    0          -0
  1  O     0.5  0.5  0.5         0

Note: there are clearly Ti and O atoms in this structure. Here's a boilerplate FEFF input config.

dict_set = FEFFDictSet(
    absorbing_atom="Ti",
    structure=struct,
    radius=10.0,
    config_dict={
        "S02": "0",
        "COREHOLE": "regular",
        "CONTROL": "1 1 1 1 1 1",
        "XANES": "4 0.04 0.1",
        "SCF": "7.0 0 100 0.2 3",
        "FMS": "9.0 0",
        "EXCHANGE": "0 0.0 0.0 2",
        "RPATH": "-1",
    }
)

But when trying to write the input file, I get this error.

dict_set.write_input("my_test")

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
Cell In[74], line 1
----> 1 dict_set.write_input("my_test")

File ~/miniforge3/envs/Lightshow/lib/python3.9/site-packages/pymatgen/io/feff/sets.py:99, in AbstractFeffInputSet.write_input(self, output_dir, make_dir_if_not_present)
     95     os.makedirs(output_dir)
     97 feff = self.all_input()
---> 99 feff_input = "\n\n".join(str(feff[k]) for k in ["HEADER", "PARAMETERS", "POTENTIALS", "ATOMS"] if k in feff)
    101 for k, v in feff.items():
    102     with open(os.path.join(output_dir, k), "w") as f:

File ~/miniforge3/envs/Lightshow/lib/python3.9/site-packages/pymatgen/io/feff/sets.py:99, in <genexpr>(.0)
     95     os.makedirs(output_dir)
     97 feff = self.all_input()
---> 99 feff_input = "\n\n".join(str(feff[k]) for k in ["HEADER", "PARAMETERS", "POTENTIALS", "ATOMS"] if k in feff)
    101 for k, v in feff.items():
    102     with open(os.path.join(output_dir, k), "w") as f:

File ~/miniforge3/envs/Lightshow/lib/python3.9/site-packages/pymatgen/io/feff/inputs.py:535, in Atoms.__str__(self)
    533 def __str__(self):
    534     """String representation of Atoms file."""
--> 535     lines_sorted = self.get_lines()
    536     # TODO: remove the formatting and update the unit tests
    537     lines_formatted = str(
    538         tabulate(
    539             lines_sorted,
    540             headers=["*       x", "y", "z", "ipot", "Atom", "Distance", "Number"],
    541         )
    542     )

File ~/miniforge3/envs/Lightshow/lib/python3.9/site-packages/pymatgen/io/feff/inputs.py:517, in Atoms.get_lines(self)
    515 for i, site in enumerate(self._cluster[1:]):
    516     site_symbol = re.sub(r"[^aA-zZ]+", "", site.species_string)
--> 517     ipot = self.pot_dict[site_symbol]
    518     lines.append(
    519         [
    520             f"{site.x:f}",
   (...)
    527         ]
    528     )
    530 # sort by distance from absorbing atom

KeyError: 'Ti'

It appears that somehow Ti is not found in this self.pot_dict object buried in pymatgen.io.feff.inputs.Atoms.get_lines. I have no idea why and have not run into this before (this has worked in previous versions of Pymatgen). Any idea what's going on?

Repro

See above. I'm using Pymatgen version 2023.7.20.

Expected behavior

I don't think this should error... Clearly Ti is in this structure!

Environment

MacOS M1 Ventura 13.4.1

[janosh]

@stefsmeets Could this be caused by the recent changes to Site.label?

[stefsmeets]

@matthewcarbone what is the last known version that this is working?

I did a git bisect to way before we added labels, and it seems to be broken since a couple of months from what I can tell.

c62f8a05f4607d0d75f34c9f18315bd0598503b6 is the first bad commit
commit c62f8a05f4607d0d75f34c9f18315bd0598503b6
Author: Ryan Kingsbury <[email protected]>
Date:   Mon May 2 16:40:28 2022 -0700

    FEFF: bugfix for structures with 1 abs atom

 pymatgen/io/feff/inputs.py | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

This commit introduces the bug: c62f8a0

[matthewcarbone]

@stefsmeets I don't quite recall, no (though looks like you did find it). I do know that the old version of pymatgen does seem to work (from 2022).

[janosh]

@stefsmeets thanks a lot for digging into this!

@rkingsbury looks like the commit fixed one case, but broke another. I'm not very familiar with this part of pymatgen. Do you see an easy way to fix both?

[matthewcarbone]

@janosh I've managed to pin down a version of PMG that works: pymatgen<=2022.5.26. This is what I've version-locked my own code to for now. It would be wonderful though to get the most recent API working. I would like to port my own software to the new API.

[shyuep]

I just pushed a fix. Pls check.

[rkingsbury]

Thanks for reporting @matthewcarbone and bisecting @stefsmeets and suggesting a fix @shyuep . Apologies @janosh I didn't see your ping last week.

As part of the PR that introduced the regression, I added a test for the case in which there is only a single absorbing atom in the structure (which is what motivated the change in the first place). The basic issue was that that single atom needs to be excluded from the pot_dict section. If the test still passes with @shyuep fix and if it works for you @matthewcarbone then I think we're good.

[matthewcarbone]

@shyuep @rkingsbury sorry for the late reply but this appears to work now. v2 API (2023.9.10) now passes our testing suite (AI-multimodal/Lightshow#190) and as such I'm happy. I'll keep you posted if I find any other bugs 😁