-
Notifications
You must be signed in to change notification settings - Fork 892
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
Split energy corrections into structure/composition-dependent terms in entry.energy_adjustments
#2730
Comments
Hi @janosh , you have definitely found an interesting edge case here. We do not actually have any structure dependent corrections. Rather, some corrections depend on oxidation state, and if there is no oxidation state information available, pymatgen tries to guess based on the composition (see code block). However, entries downloaded from the MP API have oxidation state information appended to the What's happening here is that for whatever reason, the Can you inspect the |
The CSE was published in this paper.
The oxidation states are all empty: cse_mp2020.data={'oxidation_states': {}}
ce_mp2020.data={'oxidation_states': {}}
cse_legacy.data={'oxidation_states': {}}
ce_legacy.data={'oxidation_states': {}} |
Here are the full reprs
|
Thanks @janosh . So in this case the presence of the structure is triggering pymatgen to detect peroxides/superoxides (see code block ) If you were to instantiate either the legacy or MP2020 corrections with As confusing as this all is, it is intended behavior. Basically, the corrections try to utilize as much information to make the best guess they can about the oxidation states of the respective ions. |
@rkingsbury Thanks for taking the time to look into this!
That's what I figured. And you're right, if I pass
So it doesn't make sense to split this structure-dependent part into its own energy adjustment? |
Correct. Our correction scheme is really not "structure based" (with this one small exception) and the correction that is applied is still a composition-based anion correction. The structure just happens to be used in this one case to classify the anion as a superoxide/peroxide/oxide. So I think it could be misleading to separate out a "structure-dependent" energy adjustment. However if you think edits to any of the associated docstrings or documentation are in order to make this clearer, I'd be supportive of that. |
It's also used to classify sulfides vs polysulfides. I'll add a paragraph to the |
I noticed both the old and 2020 MP energy correction schemes are structure-dependent. Here's an example
ComputedStructureEntry
that when converted to ComputedEntry each copy corrected with both old and new correction scheme gives 4 different energies.cse.json.zip
This script prints
Currently the list of energy adjustments only discerns anion corrections from MP advanced corrections.
@rkingsbury @computron Would it be possible to split this further to show structure-dependent corrections and simpler composition-only-corrections separately?
The text was updated successfully, but these errors were encountered: