WIP: Adding support for reading vaspwave.h5 in Wavecar and Chgcar class #3768
Conversation
WalkthroughThe recent changes across the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
pymatgen/io/vasp/outputs.py
Outdated
| ( | ||
| self.kpoints, | ||
| self.actual_kpoints, | ||
| self.actual_kpoints_weights, | ||
| ) = self._parse_kpoints(elem) |
There was a problem hiding this comment.
Ensure proper error handling in the __init__ method for file operations.
Consider adding a try-except block around the file opening and parsing logic to handle potential I/O errors or XML parsing issues gracefully.
pymatgen/io/vasp/outputs.py
Outdated
| [ | ||
| max(cbm_spins[0] - vbm_spins[0], 0), | ||
| max(cbm_spins[1] - vbm_spins[1], 0), | ||
| ], | ||
| [cbm_spins[0], cbm_spins[1]], | ||
| [vbm_spins[0], vbm_spins[1]], | ||
| [vbm_spins_kpoints[0] == cbm_spins_kpoints[0], vbm_spins_kpoints[1] == cbm_spins_kpoints[1]], | ||
| [ | ||
| vbm_spins_kpoints[0] == cbm_spins_kpoints[0], | ||
| vbm_spins_kpoints[1] == cbm_spins_kpoints[1], | ||
| ], |
There was a problem hiding this comment.
Refine the method for extracting eigenvalue band properties.
- def eigenvalue_band_properties(self):
+ def get_eigenvalue_band_properties(self):Rename the method to follow Python's naming conventions for methods that return data.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| [ | |
| max(cbm_spins[0] - vbm_spins[0], 0), | |
| max(cbm_spins[1] - vbm_spins[1], 0), | |
| ], | |
| [cbm_spins[0], cbm_spins[1]], | |
| [vbm_spins[0], vbm_spins[1]], | |
| [vbm_spins_kpoints[0] == cbm_spins_kpoints[0], vbm_spins_kpoints[1] == cbm_spins_kpoints[1]], | |
| [ | |
| vbm_spins_kpoints[0] == cbm_spins_kpoints[0], | |
| vbm_spins_kpoints[1] == cbm_spins_kpoints[1], | |
| ], | |
| [ | |
| max(cbm_spins[0] - vbm_spins[0], 0), | |
| max(cbm_spins[1] - vbm_spins[1], 0), | |
| ], | |
| [cbm_spins[0], cbm_spins[1]], | |
| [vbm_spins[0], vbm_spins[1]], | |
| [ | |
| vbm_spins_kpoints[0] == cbm_spins_kpoints[0], | |
| vbm_spins_kpoints[1] == cbm_spins_kpoints[1], | |
| ], |
Ensure the data structure for WSWQ is correctly initialized.
Verify that the initialization of the data arrays in the WSWQ class handles all possible configurations correctly, especially with different spin and k-point counts.
Optimize the file writing process in the write_file method.
- def write_file(self, filename):
+ def save_to_file(self, filename):Rename the method to better reflect its functionality and align with common Python practices.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| [ | |
| max(cbm_spins[0] - vbm_spins[0], 0), | |
| max(cbm_spins[1] - vbm_spins[1], 0), | |
| ], | |
| [cbm_spins[0], cbm_spins[1]], | |
| [vbm_spins[0], vbm_spins[1]], | |
| [vbm_spins_kpoints[0] == cbm_spins_kpoints[0], vbm_spins_kpoints[1] == cbm_spins_kpoints[1]], | |
| [ | |
| vbm_spins_kpoints[0] == cbm_spins_kpoints[0], | |
| vbm_spins_kpoints[1] == cbm_spins_kpoints[1], | |
| ], | |
| [ | |
| max(cbm_spins[0] - vbm_spins[0], 0), | |
| max(cbm_spins[1] - vbm_spins[1], 0), | |
| ], | |
| [cbm_spins[0], cbm_spins[1]], | |
| [vbm_spins[0], vbm_spins[1]], | |
| [ | |
| vbm_spins_kpoints[0] == cbm_spins_kpoints[0], | |
| vbm_spins_kpoints[1] == cbm_spins_kpoints[1], | |
| ], |
tests/io/vasp/test_outputs.py
Outdated
| @@ -1736,6 +1819,32 @@ def test_standard(self): | |||
| finally: | |||
| sys.stdout = saved_stdout | |||
|
|
|||
| def test_vaspwave(self): | |||
There was a problem hiding this comment.
This functions checks whether the information read from legacy and hdf5 files are same or not.
tests/io/vasp/test_outputs.py
Outdated
| for b in range(vaspwave.nb): | ||
| assert len(vaspwave.coeffs[k][b]) == len(wavecar.Gpoints[k]) | ||
|
|
||
| with pytest.raises(ValueError, match="invalid filetype='json'"): |
There was a problem hiding this comment.
Proper filetype should be given.
tests/io/vasp/test_outputs.py
Outdated
| with pytest.raises(ValueError, match="invalid filetype='json'"): | ||
| Wavecar(f"{VASP_OUT_DIR}/vaspwave.fccSi.h5.gz", filetype="json", vasp_type="std") | ||
|
|
||
| with pytest.raises(ValueError, match="Invalid rtag=.+, must be one of"): |
There was a problem hiding this comment.
Default filetype is legacy. It will result in Value error for in valid rtag while reading hdf5 file as it tries to read the hdf5 file as legacy WAVECAR file. So, hdf5 filetype should be provided while reading hdf5 file.
|
Thanks for the PR @Shibu778! My sincere apologies there was no attention given to this PR before now. Could you resolve the current merge conflicts and ensure the automated CI checks still pass? The pymatgen code has now been moved into Other than this, I notice a lot of code is duplicated between It should also be possible to detect the filetype automatically: use the existing logical if the filename is |
janosh
added
enhancement
janosh
force-pushed
the
master
branch
2 times, most recently
from
e3fbc67
to
41e6d99
Compare
found with regex \b\s{2,}\b[^\d]
… returning nothing fix brittle TestQuasiHarmonicDebyeApprox.test_gruneisen_parameter from tight assert_allclose abs tol
The pdfs list should be constructed from filenames only without other path components, i.e. use last entry from split()
* Fix `apply_operation(fractional=True)` * Add tests for `apply_operation` * pre-commit auto-fixes --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…config='cp39-*', skip_config='*musllinux*', requires_python=<SpecifierSet('>=3.10')>, prerelease_pythons=False)
* Use distinct names for artifacts * Bump `upload-artifact` * allow any v4 download-artifact --------- Co-authored-by: Janosh Riebesell <[email protected]>
* Update surface and interface generating functions 1. fixing problem for the lll_reduce process when making slabs, doing mapping before updating the structure 2. allow to set ftol of the termination distances for hierarchical cluster so that some non-identical terminations close to each other can be identified 3. allow to add index for terminations so that terminations with the same space group can be distinguished Interfaces made by identical slabs can be non-identical because the relative transformation of the misorientation and the termination variation do not ensure symmetry, especially when the film and substrate have different point groups. Therefore, the termination finding function should allow to generate all the possible terminations. This can help others to develop more robust algorithm to group the equivalent interfaces made by different terminations. --------- Signed-off-by: Jason Xie <[email protected]> Signed-off-by: Jason Xie <[email protected]> Co-authored-by: Janosh Riebesell <[email protected]>
* push a random commit to get a baseline * install native micromamba + native uv * trigger uv to run a second time, hopefully cached * get a benchmark of pip install uv * remove manual install of build dependencies * more verbose uv * remove debug output
* fix all ruff DOC202 https://docs.astral.sh/ruff/rules/docstring-extraneous-returns/ * doc str format: "list[...]: List of ..." -> "list[...]: ..."
…rialsproject#4071) * fixed electronic step check with algo = exact and nelm = 1 * pre-commit auto-fixes * fixed the cases in which ALGO does not appear in incar * pre-commit auto-fixes --------- Co-authored-by: yanghan-microsoft <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Added DebyeCalculator as an external tool Signed-off-by: Andy S. Anker <[email protected]>
* Update `slme` function so it doesn't overwrite the input data * Basic plotting updates; remove hard-coded x-limit, add some labels
* Fix bug with species defaults NEed to specify the default value properly * pre-commit auto-fixes --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* use np.testing to check dict equality * add unit tests * perhaps use a helper function instead? * add test for misc, thanks gpt * fix typo * add check if return type
* pip install BoltzTraP2 * remove windows exclusion pin and see what happens * bump bt2 ver * skip bt2 for windows * bump bt2 ver again * still skip win for now as cannot build * give VS build tools a try * revert to skip windows * remove unnecessary venv activate
|
Dear @janosh |
|
Now I have synced the forked repository properly. I will start adding the feature reading WAVECAR and CHGCAR in hdf5 format soon. |
Dear Maintainers,
This is my first contribution to
pymatgen. I am relatively new to opensource and would appreciate your feedback and guidance in improving the added features and this PR. In further commits, I will addread_from_hdf5functionality toChgcarand write all the tests.Summary
Major changes:
read_from_hdf5inWavecarclass to readWAVECARrelated information fromvaspwave.h5file (refer to vaspwiki).WAVECARformat that was previously implemented inWavecarclass.pymatgen/io/vasp/outputs.pyfile got formatted by black formatterTodos
This is work in progress, following needs to be done
Chgcarclasshdf5files in usingWavecarandChgcarclassPre-commit result
pre-commit run --all-filespassed all everythingChecklist
ruff.mypy.Wavecarclass remains unchanged to read legacyWAVECARfileSummary by CodeRabbit
Refactor
Documentation