Conversation
|
@ladinesa Should I add the test referenced in the original issue? |
- Tie each `kpoints` to `eigenvalues` on a `n_calc` base
TODO: distinguish for the case of single-point calculations
…e the symmetry is not updated
5595e1f to
3906e81
Compare
electronicparsers/vasp/parser.py
Outdated
| self._kpoints_info = [] | ||
| for kpts_occs in self.parser.get('kpoints'): | ||
| if kpts_occs is not None: | ||
| self._kpoints_info.append({}) |
There was a problem hiding this comment.
probably define info = {} then append this later. Is there no info about the iter #? I am just worried that the kpoint does not match the step. If there is none maybe put a check that len(kpoint_info) == len(calcs)
There was a problem hiding this comment.
I've only seen 2 cases up to now: the k-points are only updated once, or they are updated at each ionic update.
I am storing the index now, and will consider using it for safety checks.
There was a problem hiding this comment.
Btw, in vasprun.xml, it's rather the eigenvalues that are out of sync with the no. steps.
There was a problem hiding this comment.
Pls, take a look at my comment below. Personally, I think we can go without any iteration index.
Or would you rather than a list, prefer a dict with a clearer index?
Instead, I'd rather have a way of moving k-points under calculation, but this isn't self-evident for OUTCAR.
electronicparsers/vasp/parser.py
Outdated
| 'listgenerated': 'Line-path', | ||
| } | ||
|
|
||
| def _find_kpoints(self) -> list[str]: |
There was a problem hiding this comment.
i do not see this used anywhere except in kpoints_info. I suggest not defining this.
There was a problem hiding this comment.
Could move it into kpoints_info().
While I get the rationale behind reserving functions only for abstractions that are used more than once, I split it here for readability. This happens in other places of the VASP parser too. You let me know what you think.
|
Thanks for the fix. Please also add a test for this, maybe one outcar and one vasprun, you can also use this for the atom positions fix. |
…x for k-point extraction
TODO: correct the no. of extracted eigenvalues (and k-points) from vasprun.xml
|
Something to consider in the future, when we clean the VASP parser.
When it comes to eigenvalues, we mostly just need the no. irreducible k-points (at that step) to restructure the eigenvalues. |
Tackle changing symmetries (and k-point set) between ionic updates in newer VASP versions (about 6).
This solution packages
kpointsinto lists matching the no.calculations. The cases of single-point calculations and the older format are also handled (forvasprun.xml). No explicit information about the version is used.Closes #148