Nexus: Add Z-valence parsers for UPF, XML, and POTCAR files#6035
Conversation
| f"Could not find Z valence in file: {file!s}\n" | ||
| "You may need to provide the Z valence manually!" | ||
| ) | ||
| elif zval.is_integer(): |
There was a problem hiding this comment.
This is a nice call. What is it's tolerance? If too strict, we won't be able to use it.
There was a problem hiding this comment.
Unusual behavior; even the floating point numbers closest to zero are apparently non-integer:
>>> 4.940656458412465e-324.is_integer()
False
>>> -4.940656458412465e-324.is_integer()
0
This is far too strict.
There was a problem hiding this comment.
I've never seen any pseudopotentials with that small of a Z-valence...
There was a problem hiding this comment.
The point is that parsing floating point numbers is not guaranteed to satisfy is_integer. Those numbers are as close to zero as possible (w/o being zero). The tolerance should be something like 1e-9, >300 orders of magnitude larger.
|
|
||
| def read_xml_z_valence(file: PathLike) -> int | float: | ||
| """Read the Z-valence from a QMCPACK-compatible XML pseudopotential file.""" | ||
| header_pattern = re.compile(r'zval=\"([\d\.]+)\"') |
There was a problem hiding this comment.
This regex is insufficient. XML can have whitespace between zval and =.
| while not found_header_start: | ||
| line = pseudo.readline() | ||
| if "<PP_HEADER" in line: | ||
| header_data.append(line) |
There was a problem hiding this comment.
I think logic is missing here. header_data is collected, but not used. The parse below based on the end of PP_HEADER assumes the zval data resides on that line.
I don't understand how this code passes the upf parsing test, unless it always trips the fallback code.
| break | ||
| elif "/>" in line or "</PP_HEADER>" in line: | ||
| break | ||
| #end if |
prckent
left a comment
prckent
left a comment
There was a problem hiding this comment.
Thanks for the care in not including a complete POTCAR .
|
Test this please |
3 participants
Proposed changes
This PR adds Z-valence parsers for UPF v1, UPF v2, XML, and POTCAR files.
These are currently unused, but will be useful in the future when I eventually get around to refactoring
pseudopotential.py. The idea is to be able to parse most of the pseudopotential formats that a user will have so that they don't need to specify the Z-valences manually. This will mean less chance of user error and generally cleaner scripts in the future.I anticipate allowing users to continue to manually specify Z-valences for creating a
PhysicalSystem, but also adding in a feature where they can provide Z-valences toppset, then providing theppsetlabel toPhysicalSystem, which will either parse the files or use the provided Z-valences to pseudize the system.Ideally this will mean that eventually user's wont need to specify a Z-valence manually anymore, which I personally have found to be a cumbersome task.
Note about POTCAR parser and test
In hopes of not incurring the wrath of a lawyer, I have made sure to only use publicly available information about the format of a POTCAR file. The parser and test for
read_potcar_z_valence()are constructed solely from information found on the VASP Wiki. I modified the mock POTCAR headers slightly to have a Z-valence matching that of the UPF and XML tests, but otherwise it is verbatim from the VASP Wiki.What type(s) of changes does this code introduce?
Does this introduce a breaking change?
What systems has this change been tested on?
Desktop, Fedora Linux 43 (KDE Plasma Desktop Edition)
AMD Ryzen 9 7900X (12 cores, 24 logical processors)
Checklist