Fix #57: Add energy validation for ORCA output files#60
Open
manvirsingh01 wants to merge 2 commits intotheochem:masterfrom
Open
Fix #57: Add energy validation for ORCA output files#60manvirsingh01 wants to merge 2 commits intotheochem:masterfrom
manvirsingh01 wants to merge 2 commits intotheochem:masterfrom
Conversation
- Added validation in get_dict_energy() to check if molecule.energy is None - Raises clear ValueError with guidance for users using ORCA files (.wfn, .mkl) - Added test case for the new validation - ORCA files don't contain total energy, so users are directed to use .fchk files
Member
|
@manvirsingh01 I've @'d @FarnazH who is the primary maintainer of ChemTools. You should know that there is a MAJOR refactor of ChemTools underway (in a private repository) so I wouldn't invest too much time in this current version. |
There was a problem hiding this comment.
Pull request overview
Adds explicit handling for molecule files that lack total energy (common with ORCA-generated .wfn/.mkl) to prevent downstream crashes when building energy dictionaries for conceptual DFT workflows.
Changes:
- Validate
molecule.energy/mol.energyinget_dict_energy()and raise a clearerValueErrorwhen missing. - Add a regression test that simulates a molecule missing energy to ensure the error is raised.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
chemtools/toolbox/utils.py |
Adds None-energy validation and raises clearer ValueError instead of crashing during FMO energy construction. |
chemtools/toolbox/test/test_utils.py |
Adds a test that forces energy=None on a loaded molecule and asserts get_dict_energy() raises. |
💡 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.
Fix ORCA file handling error in
CondensedConceptualDFT.from_file()This PR fixes #57, where ORCA-generated
.wfnand.mklfiles caused a runtime crash when loaded usingCondensedConceptualDFT.from_file().Issue
ORCA output files do not include total energy information. However,
get_dict_energy()assumedmolecule.energywas always present, leading to the following error:Solution
get_dict_energy()to check whethermolecule.energyisNoneValueErrorexplaining the limitation.fchkfiles or alternative conversion workflowsChanges
chemtools/toolbox/utils.py: Added energy availability validationchemtools/toolbox/test/test_utils.py: Added regression test for missing energy dataTesting
hortondependency