Conversation
…_volumes is specified, process only the corresponding folders instead of processing all folders and filtering afterward; improve type hints in ThermalElectronic; adjust ThermalElectronicData and Configuration; update tests
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the thermal-electronic DOS ingestion workflow to support selecting explicit elec_* folders (as an alternative to selecting by volume), and updates tests/expected-results artifacts accordingly.
Changes:
- Add
selected_folders(mutually exclusive withselected_volumes) to thermal-electronic data loading APIs. - Propagate the new selection option through
Configuration.process_thermal_electronic(...)andThermalElectronicData. - Extend unit tests (and the expected-results notebook) to cover folder selection and the mutual-exclusion error.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_thermal_electronic_data/thermal_electronic_expected_results.ipynb |
Refresh expected-results notebook (clear outputs, add examples for selected_folders + mutual exclusivity). |
tests/test_thermal_electronic.py |
Add test coverage for selected_folders and the mutual-exclusion error condition. |
dfttk/thermal_electronic/thermal_electronic_data.py |
Add selected_folders support to VASP input harvesting and propagate into thermal-electronic processing. |
dfttk/thermal_electronic/thermal_electronic.py |
Add selected_folders to read_total_electron_dos and adjust volume filtering behavior. |
dfttk/configuration.py |
Wire selected_folders / vasprun_name through the Configuration-level processing entrypoint. |
Comments suppressed due to low confidence (1)
dfttk/thermal_electronic/thermal_electronic_data.py:196
selected_foldersis accepted but not validated against the detectedelec_folders(or existence on disk). If a folder is misspelled/missing, the loop will fail later withFileNotFoundErrorwhen reading INCAR/KPOINTS/CONTCAR, which is harder to diagnose than a targetedValueError. Consider validatingselected_foldersup front (and possibly enforcing thefolder_prefix) and raising a clear error listing any missing folders.
# Iterate over the requested folders; if none are provided, use all detected elec folders
folders_to_process = natsorted(
elec_folders if selected_folders is None else selected_folders
)
# Read the INCAR, KPOINTS, and structures for each electronic DOS folder
for elec_folder in folders_to_process:
incar_data = {}
for key, name in zip(incar_keys, incar_names):
incar_data[key] = Incar.from_file(
os.path.join(self.path, elec_folder, name)
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.