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

finalize MP2020 correction values #2106

Merged
merged 14 commits into from
Apr 20, 2021

Conversation

rkingsbury
Copy link
Contributor

@rkingsbury rkingsbury commented Apr 15, 2021

Summary

Final correction fitting for MaterialsProject2020Compatibility

@rkingsbury rkingsbury marked this pull request as ready for review April 16, 2021 18:08
@rkingsbury
Copy link
Contributor Author

@mkhorton there seem to be some problems with the test environment - it complains about enumlib and openbabel not being installed / available

@mkhorton
Copy link
Member

That message is just saying those tests have been skipped -- the actual error is that PMG_MAPI_KEY isn't set. I can look into it.

@rkingsbury rkingsbury changed the title [WIP] finalize MP2020 correction values finalize MP2020 correction values Apr 16, 2021

# Test S, which has Na in reference solids
pbx_entries = self.rester.get_pourbaix_entries(["S"])
so4_two_minus = pbx_entries[9]
self.assertAlmostEqual(so4_two_minus.energy, 0.0644980568750011, places=2)
self.assertAlmostEqual(so4_two_minus.energy, 0.301511, places=3)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rkingsbury before merging, can you share verification that the new Pourbaix numbers are decent? have you compared the resulting Pourbaix diagrams? These differences seem like they may be significant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mkhorton thanks for flagging this; I should have said something about it. I saw it too and had the same reaction, but I checked many Pourbaix diagrams with the correction values in this MPR and they compare well with the existing ones. I'll email you a slide deck so you can see the comparisons. Bottom line I don't think we need to be concerned, and being able to do a full DB build will give us even more confidence.

@mkhorton mkhorton merged commit 2ce8725 into materialsproject:master Apr 20, 2021
@mkhorton
Copy link
Member

Thanks @rkingsbury :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

latexify_ion is broken
2 participants