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}
| ( | ||
| 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.
| [ | ||
| 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], | |
| ], |
| @@ -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.
| 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.
| 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.
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