Raise Python exception on FoX XML parse errors (closes #724)#725
Merged
Conversation
Invalid XML in Potential(param_filename=...) currently silently exits the Python interpreter because FoX_error_base calls Fortran stop directly. This test runs the user's reproducer in a subprocess and asserts a catchable exception is raised; in the current RED state the subprocess exits 0 with no exception caught. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Install a FoX error handler in system_initialise that calls system_abort instead of letting FoX issue a bare Fortran stop. When running under quippy this triggers f90wrap_abort, so invalid XML in a param_filename now raises a Python RuntimeError that can be caught with try/except, instead of silently exiting the interpreter. Build changes: * src/fox is now built before src/libAtoms so the FoX module files are available when System.F90 is compiled. * libAtoms links against fox to expose the new FoX_set_error_handler symbol. The fox submodule pointer is bumped to pick up the matching pluggable-handler commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jameskermode
force-pushed
the
fix-fox-xml-error-propagation-724
branch
from
4a12731 to
a803bdd
Compare
Member
Author
|
@bernstei Do you want to take a look before I merge libAtoms/fox#4 and this? I have verified the test, but not manually checked much more than that. One-short Claude from your bug report and instruction to do test driven development was enough. |
Contributor
|
Looks reasonable to me. I'm happy with you merging, or could test the branch locally first if you prefer |
Member
Author
|
I've run it locally so no need. One the CI passes I will merge - I think that interim failure is unrelated, due to flaky DNS. |
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
2 participants
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.
Summary
system_initialise(src/libAtoms/System.F90) that routes parse errors throughsystem_abort. When running under quippy this triggersf90wrap_abort→ PythonRuntimeError, sotry/exceptaroundPotential(param_filename=...)now actually works.src/foxsubmodule pointer to pick up the matching pluggable-handler hook (Add pluggable FoX_error_handler hook fox#4 — must merge first).src/foxis now built beforesrc/libAtoms, and libAtomslink_withs fox to exposeFoX_set_error_handler.tests/test_fox_xml_errors.py(subprocess-based, since the previous bug killed the interpreter).Bug
Closes #724. Before this change, the reporter's reproducer silently exits with status 0 —
try/exceptnever runs:After:
Root cause
FoX_error_basein the fox submodule called Fortranstopdirectly, bypassing libAtoms'system_abort+f90wrap_abortchain. libAtoms now installs a pluggable handler in fox (via libAtoms/fox#4) so the chain reaches Python.Test plan
tests/test_fox_xml_errors.pypasses (runs the user's exact reproducer in a subprocess and asserts a caught exception).tests/test_symbol_resolution.py::test_error_handling_raises_exceptions(file-not-found case) still passes — verifies no regression in the existing error-propagation contract.Ran 51 tests in 10.898s — OK (skipped=6).system_abortstill ends instop 1outside of quippy.Dependency
🔴 libAtoms/fox#4 must merge first — this PR's submodule pointer references a commit that exists only on that fox PR's branch. CI will fail to fetch the submodule until that PR lands.
🤖 Generated with Claude Code