Skip to content

Fix load_many crash on single-atom/single-step FCHK optimizations#404

Merged
tovrstra merged 8 commits into
theochem:mainfrom
Ao-chuba:fix/fchk-single-atom-load
Feb 7, 2026
Merged

Fix load_many crash on single-atom/single-step FCHK optimizations#404
tovrstra merged 8 commits into
theochem:mainfrom
Ao-chuba:fix/fchk-single-atom-load

Conversation

@Ao-chuba
Copy link
Copy Markdown
Member

@Ao-chuba Ao-chuba commented Jan 23, 2026

This PR fixes a LoadError in load_many for single-atom or single-step FCHK optimizations where the trajectory block is missing.

Changes :
iodata/formats/fchk.py : Added a fallback to construct a single frame trajectory from Current cartesian coordinates, Total Energy, andCartesian Gradientwhen the standard trajectory block is absent but the file is an optimization (FOpt, Scan, IRC).

Tests: Added regression test iodata/test/test_fchk_single_atom.py and data iodata/test/data/atom_opt.fchk to verify the fix and prevent regression.
Screenshot 2026-01-23 213304

I ran a manual test script against the newly added regression data (_iodata/test/data/atom_opt.fchk_ ) to verify the fix before submitting. As shown in the screenshot, load_many now correctly parses the file, extracting 1 frame with the expected energy (-0.4665...) and coordinates, instead of crashing.

Closes #386

@Ao-chuba Ao-chuba force-pushed the fix/fchk-single-atom-load branch from be23fce to 357d963 Compare January 23, 2026 16:18
Comment thread iodata/formats/fchk.py
@tovrstra tovrstra changed the title Fix load_many crash on single-atom/single-step FCHK optimizations (#386) Fix load_many crash on single-atom/single-step FCHK optimizations Jan 24, 2026
@tovrstra
Copy link
Copy Markdown
Member

@Ao-chuba Thanks for working on this.

(Minor point: Instead of mentioning the issue being fixed in the PR title, you can add Closes #385 to the initial message. This will link them correctly.)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses issue #386 by fixing a LoadError in load_many when processing FCHK files from single-atom or single-step optimizations where the standard trajectory block (IRC/Optimization Number of geometries) is missing.

Changes:

  • Added fallback logic to construct a single-frame trajectory from current geometry data when trajectory blocks are absent
  • Refactored frame creation into a reusable _create_frame helper function
  • Added regression test and test data to verify the fix

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
iodata/formats/fchk.py Implements fallback for missing trajectory blocks and refactors frame creation into _create_frame helper function
iodata/test/test_fchk_single_atom.py Adds regression test for single-atom optimization loading
iodata/test/data/atom_opt.fchk Test data file representing a single-atom hydrogen optimization

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread iodata/test/test_fchk_single_atom.py Outdated
Comment thread iodata/test/test_fchk_single_atom.py Outdated
Copy link
Copy Markdown
Member

@tovrstra tovrstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot picked up a few good points to check.

Just one minor question from the human reviewer. Do you suggest generally splitting up the test files? (So far, we've been keeping tests related to each format together in one file, to reuse utility functions.)

Comment thread iodata/test/test_fchk_single_atom.py Outdated
@Ao-chuba
Copy link
Copy Markdown
Member Author

@tovrstra

Copilot picked up a few good points to check.

Just one minor question from the human reviewer. Do you suggest generally splitting up the test files? (So far, we've been keeping tests related to each format together in one file, to reuse utility functions.)

That makes total sense regarding utility reuse. I completely agree it's better to keep the tests consolidated for consistency.
I have moved the test case directly into iodata/test/test_fchk.py (_and removed test_fchk_single_atom.py) to match the existing structure. I also added the assertions Copilot suggested (verifying zero gradients and ensuring reaction_coordinate is absent)

Copy link
Copy Markdown
Member

@tovrstra tovrstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for having been out of touch for a while. All looks good. Thanks!

@tovrstra tovrstra merged commit e6a606e into theochem:main Feb 7, 2026
11 checks passed
@Ao-chuba
Copy link
Copy Markdown
Member Author

Ao-chuba commented Feb 7, 2026

@tovrstra no worries Sir, your Welcome .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

load_many from fchk fails if the Gaussian job is an optimization of a single atom

3 participants