[Breaking] Fix valence electron configuration parsing for PotcarSingle.electron_configuration

[DanielYang59]

Summary

• Fix valence electron configuration parsing in PotcarSingle.electron_configuration, to fix Possibly incorrect electron_configuration returned by vasp.inputs.PotcarSingle #4269
• Likely a follow up PR of Element/Species: order full_electron_structure by energy #3944
• Update unit tests
• Test other versions of PSP?

[DanielYang59]

I have a feeling that we might need to rewrite the electron_configuration property to parse the "Atomic configuration" section of POTCAR instead of generating from Madelung energy ordering rule, as electron configuration from POTCAR may or may not agree with periodic table (now that it's a property of PotcarSingle instead of Element).

For example for the PAW_PBE Fe 06Sep2000 POTCAR, the valence electron configuration is "3d7 4s1" instead of "3d6 4s2":

pymatgen/tests/io/vasp/test_inputs.py

Line 1386 in 5b997f7

assert self.psingle_Fe.electron_configuration == [(3, "d", 6), (4, "s", 2)]

   Atomic configuration
    9 entries
     n  l   j            E        occ.
     1  0  0.50        xxx   2.0000
     2  0  0.50        xxx   2.0000
     2  1  1.50        xxx   6.0000
     3  0  0.50        xxx   2.0000
     3  1  1.50        xxx   6.0000
     3  2  2.50        xxx   7.0000
     4  0  0.50        xxx   1.0000
     4  1  1.50        xxx   0.0000
     4  3  2.50        xxx   0.0000

Can you please give me some advice on this? @rkingsbury Thanks in advance

[JaGeo]

@DanielYang59
I agree.
Maybe, take a look at #4262
It partially parses this info already (the code is poorly written but ...)

I also defines a cutoff for an occupied orbital which might help for non-integer cases

[DanielYang59]

Hi @JaGeo good day and thanks for the comment

Maybe, take a look at #4262 It partially parses this info already

Beautiful! Looks like the dev_scripts.get_lobster_basis_from_potcars.get_valence function could replace electron_configuration property? In this case do you want me to take over the valence electron property reader part of PotcarSingle (perhaps copy and refactor your get_valence code) and then you work on top of this for the LOBSTER part? Or do you want to work on both?

the code is poorly written but

I guess no code is perfectly written without repeated refactor? :)

pymatgen/src/pymatgen/dao.py

Line 18 in 5b997f7

10. Constant refactoring is the hallmark of an open platform.

[JaGeo]

@DanielYang59 yes, it would be great if you take over and make this code a method for the potcar. Then, I can adapt the developer code.

[DanielYang59]

it would be great if you take over and make this code a method for the potcar

Great! I guess we could either:

• Simply change (fix) the implement of electron_configuration property (this PR would be breaking then)
• Or go the safer way, deprecate current electron_configuration with a corrected property (something like val_elec_config or something, naming could be discussed later of course)

Which do you suggest? I might personally prefer the former now that its already giving the wrong results so I would consider this a fix?

---

I also defines a cutoff for an occupied orbital which might help for non-integer cases

I'm not really aware of non-integer occupancies from POTCARs, can you perhaps point me to some such POTCARs such that I could put more thinking on them?

[JaGeo]

@DanielYang59 i also think it should be the former as it currently is implemented wrong in my opinion.

Li, Be, maybe? Some have 0.01 occupancy, I think. If you have some potcars available, you can use the dev script to spot them.

[DanielYang59]

i also think it should be the former as it currently is implemented wrong in my opinion.

Cool I would just replace for now but should be easy to switch anytime

Li, Be, maybe? Some have 0.01 occupancy, I think. If you have some potcars available, you can use the dev script to spot them.

Thanks I would have a look, I thought it's H.* (H.25, H.35, ...)

[DanielYang59]

We might need to rethink about this cutoff.

For the Be electron configuration, it's:

     1  0  0.50        xxx   2.0000
     2  0  0.50        xxx   1.9900
     2  1  0.50        xxx   0.0100
     3  2  1.50        xxx   0.0000

If we choose cutoff as > 0.01, the 1s orbital would also be counted as valence (as 1.99 < ZVAL).

---

Perhaps use a "tolerance" instead? which serves two purposes:

• Below the tolerance, an orbital would be considered empty (ignored)
• If already accumulated number of electrons is close to ZVAL by "tolerance", stop counting electrons from deeper shells? (i.e. the 2s orbital of 1.99 electrons is close enough to ZVAL as 2 defined under tolerance of 0.1, stop going further and not counting 1s orbital in?)

[JaGeo]

You could round to the next integer using this cutoff to get the energy config and if not possible raise an error

[JaGeo]

But happy to hear other opinions

[DanielYang59]

You could round to the next integer using this cutoff to get the energy config and if not possible raise an error

If sounds pretty similar to above "tolerance" method if I understand correctly? I.e. when current number of electrons is "close enough" to ZVAL (closeness defined by tolerance), stop searching further.

In this case, if we apply tolerance = 0.1, 2p orbital (0.01 electrons) would be ignored, first counted orbital would be 2s (1.99 electrons), and as it's close enough to ZVAL=2, search would stop and 1s wouldn't be counted.

Am I understanding this right?

[JaGeo]

Yes, exactly.

The Lobster basis function search is slightly different. As I just need the orbitals that might be relevant for a projection from the plane waves. Here, we look for the exact valence electrons (and only integer numbers might be allowed).

@shyuep @mkhorton any opinions here as this would be a breaking change?

[DanielYang59]

Great thanks for confirming. I would implement the change and everyone please feel free to comment (certainly there would be edge cases that I'm not aware of), also ping @esoteric-ephemera maybe

[rkingsbury]

It looks to me like this is the only substantive change to core.periodic_table.py, which I support. Can you please make sure to mirror these changes in Species.electronic_structure as well? Unfortunately the method doesn't inherit; it had to be re-implemented.

[DanielYang59]

Thanks for looking into this! Sure I would port changes to Species.electronic_structure.

This change was made to make mypy happy otherwise it would report: tuple don't have replace method (as members of data could be either tuple or str):

pymatgen/src/pymatgen/core/periodic_table.py

Lines 477 to 478 in 5b997f7

	if data[0][0] == "[":
	sym = data[0].replace("[", "").replace("]", "")

[rkingsbury]

@DanielYang59 thanks for flagging. I'm not qualified to comment on the best way to connect this to POTCAR output; my changes in #3944 were guided only by trying to follow the canonical Madelung's rule. But in general it seems parsing information specific to the calculation is always better than assuming a standard configuration.

I made one comment above to please mirror changes here in Species, because it actually does not inherit from Element.