Improve handling of Vasprun POTCAR search, expanded fake POTCAR library for VASP I/O tests #3491
Conversation
Signed-off-by: Aaron Kaplan <33381112+esoteric-ephemera@users.noreply.github.com>
There was a problem hiding this comment.
Thanks @esoteric-ephemera, much appreciated! 👍
I think we can rename fake_potcar_library to just fake_potcars to keep the file paths shorter.
There was a problem hiding this comment.
Can you compress this file like the others?
There was a problem hiding this comment.
Done and done!
…, rename directory of fake potcars
…_mock_complete_potcar_summary_stats fixture
…= "PBE_64" in TestMatPESStaticSet
tests/io/vasp/test_sets.py
Outdated
| assert self.set(struct).nelect == 16 | ||
| assert MPRelaxSet(struct).nelect == 22 | ||
| assert self.set(struct).nelect == 18.758 | ||
| assert MPRelaxSet(struct).nelect == 27.166 |
There was a problem hiding this comment.
Why did these values change? Are the fake POTCARs different even in the valence electrons? Could that have downstream implications? Like if we ever tested the same way we do in atomate2, we no longer can use actual VASP output as references files?
There was a problem hiding this comment.
The POTCARs have all non-string fields randomized, including the valence. Wouldn't be too challenging to change this. Another option is to simply change the data in the body of the POTCAR, since the header data is included in vasprun.xml and OUTCAR. And then I'm guessing not copyright protected, but would need confirmation from vasp
There was a problem hiding this comment.
Only randomizing the POTCAR body data would be ideal, I think! Esp. if that means our tests can be more in line with actual VASP output!
OUTCARs are freely shared afaik so doubt there's copyright on the POTCAR headers. Maybe @MichaelWolloch or @sudarshanv01 can confirm?
There was a problem hiding this comment.
Hi @janosh really sorry this took me forever to get back to! I've modified the POTCAR scrambling to only change data that doesn't appear in OUTCAR. That's basically everything after the line "Error from kinetic energy argument (eV)"
Pending tests passing, should be good to go!
There was a problem hiding this comment.
And to just be clear, that means properties like the number of electrons, nuclear charge, core radii, etc. are unaffected. Only the values of the pseudopotential and the kinetic energy corrections (which I don't think are meaningful to most)
janosh
force-pushed
the
master
branch
2 times, most recently
from
3c23114
to
36e289c
Compare
…ed to OUTCAR. Revise test data and fake POTCAR library
…otcarSingle to resolve slow docs loading time
There was a problem hiding this comment.
Thanks a lot @esoteric-ephemera! 👍 Carefully went through the whole PR again. This is excellent work!
io.vasp.outputs, rewrote theVasprun.get_potcarsfunction to not fail in cases where multiple files namedPOTCAR*are found in the cwd, but only one is actually a POTCARtests/filesto stop infringing VASP copyright. Also added a PBE_64 fake POTCAR library to support PR #3484pymatgen.io.vaspdocs page is very slow to load and scroll #3556 , makepotcar_summary_statsa private attr ofpymatgen.io.vasp.inputs.PotcarSingle(rename to_potcar_summary_stats)