Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MP2020Compatibility not applying correct anion correction #3425

Closed
bowen-bd opened this issue Oct 26, 2023 · 4 comments · Fixed by #3427
Closed

MP2020Compatibility not applying correct anion correction #3425

bowen-bd opened this issue Oct 26, 2023 · 4 comments · Fixed by #3427
Labels
bug mixing-schemes About mixing energies from different DFT functionals

Comments

@bowen-bd
Copy link

bowen-bd commented Oct 26, 2023

Version: 2023.10.11

from pymatgen.entries.compatibility import MaterialsProject2020Compatibility
from pymatgen.entries.computed_entries import ComputedStructureEntry

params = {"hubbards": {"Mn": 3.9, "O": 0, "Li": 0}, "run_type": "GGA+U"}

cse = ComputedStructureEntry(lmo, vasp_raw_energy, parameters=params)
MaterialsProject2020Compatibility(check_potcar=False).process_entries(cse)
Screen Shot 2023-10-25 at 9 33 54 PM

The anion correction is missing for these O2-

@bowen-bd
Copy link
Author

Code from here

@shyuep
Copy link
Member

shyuep commented Oct 26, 2023

@janosh @kristinpersson feel free to tag whoever is supposed to be dealing with this in MP.

@janosh
Copy link
Member

janosh commented Oct 26, 2023

@shyuep The person to tag would be @rkingsbury or myself. I thought I fixed this problem in #3184 but looks like there's another bug related to Composition indexing using Elements/str vs. Species. Currently:

comp = Composition({"Li+": 2, "Mn3+": 2, "O2-": 4})
comp["Li"] == 0

I would argue comp["Li"] should return 2 instead, i.e. return all species with matching symbol, regardless of oxidation state. That would fix the problem @BowenD-UCB encountered since currently comp["O"] returns zero and so no oxide correction is applied.

But this would be breaking, and in particular might conflict with user expectations in the (rarely used) case of allow_negative=True since e.g. currently

comp = Composition({"Mg": 1, "Mg2+": -1})
comp["Mg"] == 1

which would change to return 0.

@shyuep
Copy link
Member

shyuep commented Oct 26, 2023

I think it is fine for you to handle it for now. But a more permanent person in charge needs to be defined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mixing-schemes About mixing energies from different DFT functionals
Projects
None yet
3 participants