Skip to content

526 core exceptions#537

Merged
zasexton merged 29 commits intoSimVascular:mainfrom
zasexton:526-core-exceptions
May 5, 2026
Merged

526 core exceptions#537
zasexton merged 29 commits intoSimVascular:mainfrom
zasexton:526-core-exceptions

Conversation

@zasexton
Copy link
Copy Markdown
Collaborator

@zasexton zasexton commented Apr 28, 2026

Current situation

Related to #526.

adds a shared solver exception framework and starts migrating top-level application-driver error handling to it. The branch introduces Core/Exception.h, Core/PlatformSupport.h, and FE/Common/FEException.h, wires the new headers into the solver build, and updates XML parameter parsing plus main.cpp error handling.

This is important because it centralizes solver errors around consistent status codes, source locations, MPI rank awareness, and optional stack traces. It also replaces several XML parsing paths that could previously fall through as raw std::out_of_range, null text/attribute handling, or generic runtime errors with structured svmp::ParseException failures.

Release Notes

  • Added shared core exception infrastructure for solver-wide error reporting.
  • Added FE exception types built on the shared exception base.
  • Improved XML parameter parsing diagnostics by reporting missing or invalid attributes/text through
    ParseException.
  • Updated command-line solver error handling to report structured XML parse errors and clean up MPI
    consistently.
  • No user-facing input format changes needed

Documentation

The new exception headers include inline documentation and named exception/status types. No separate user documentation is required because these comprise internal error handling without changing solver XML syntax or runtime options.

Testing

Built the solver target successfully:

cmake --build cmake-build --target svMultiPhysics -j2

Remaining verification will come from GitHub action CI and current test infrastructure.

Code of Conduct & Contributing Guidelines

@zasexton zasexton self-assigned this Apr 28, 2026
@zasexton zasexton added the OOP Refactor Object-Oriented Programming Refactor of Code label Apr 28, 2026
@aabrown100-git aabrown100-git requested a review from Copilot April 28, 2026 18:47
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 introduces a shared, solver-wide exception infrastructure (status codes, source locations, MPI-rank awareness, optional stack traces) and migrates XML parameter parsing and top-level driver error handling toward structured svmp::ParseException failures (per #526).

Changes:

  • Added core exception/runtime headers (Core/Exception.h, Core/PlatformSupport.h) and FE-specific exception types (FE/Common/FEException.h).
  • Replaced many XML parsing std::runtime_error/null-text fall-through paths with svmp::raise<svmp::ParseException>(SVMP_HERE, ...) plus helper “require_*” utilities.
  • Wrapped the solver’s main() simulation path in a try/catch for parse errors and added MPI cleanup via the new runtime helpers.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
Code/Source/solver/main.cpp Adds a try/catch boundary for XML parse exceptions and uses new MPI abort/finalize helpers.
Code/Source/solver/Parameters.h Switches parsing-related failures from std::runtime_error to structured ParseException.
Code/Source/solver/Parameters.cpp Adds XML/map “require_*” helpers and migrates many parsing errors to ParseException with improved diagnostics.
Code/Source/solver/FE/Common/FEException.h Introduces FE-layer exception hierarchy built on the shared core exception base.
Code/Source/solver/Core/PlatformSupport.h Implements platform/MPI utilities and stack trace capture used by the exception system.
Code/Source/solver/Core/Exception.h Defines core exception types, formatter, raise/check helpers, and terminate handler.
Code/Source/solver/CMakeLists.txt Wires new Core/FE exception headers into the solver build and adds include directories.

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

Comment thread Code/Source/solver/Parameters.cpp Outdated
Comment thread Code/Source/solver/Parameters.cpp Outdated
Comment thread Code/Source/solver/Core/Exception.h Outdated
Comment thread Code/Source/solver/Core/PlatformSupport.inl
Comment thread Code/Source/solver/FE/Common/FEException.h Outdated
Comment thread Code/Source/solver/main.cpp
Comment thread Code/Source/solver/main.cpp Outdated
zasexton and others added 3 commits April 28, 2026 11:58
whoops, can't spell. correcting occured -> occurred

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Renamed Code/Source/solver/Core/PlatformSupport.h to PlatformSupport.inl.
- Updated Code/Source/solver/Core/Exception.h:131 to include it locally as "PlatformSupport.inl".
 - Added a private-include guard so PlatformSupport.inl errors if included directly instead of through Exception.h.
 - Updated Code/Source/solver/CMakeLists.txt:234 to include Core/*.inl in the solver source listing.
  - Code/Source/solver/Parameters.cpp:205: fixed occured to occurred.
  - Code/Source/solver/FE/Common/FEException.h:12: updated the comment to reference Exception.h.
  - Code/Source/solver/main.cpp:714: added the final `catch (...)` so unknown non-std::exception throws
    still report an error and call the MPI cleanup helpers.
@zasexton
Copy link
Copy Markdown
Collaborator Author

Cancelling all of the CI runs except the last two so that we do not have to wait forever.

Copy link
Copy Markdown
Collaborator

@aabrown100-git aabrown100-git left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 28, 2026

Codecov Report

❌ Patch coverage is 44.03670% with 183 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.15%. Comparing base (e72aba7) to head (67924fb).

Files with missing lines Patch % Lines
Code/Source/solver/Parameters.cpp 47.46% 83 Missing ⚠️
Code/Source/solver/Core/Exception.h 51.35% 54 Missing ⚠️
Code/Source/solver/Core/PlatformSupport.inl 32.25% 21 Missing ⚠️
Code/Source/solver/main.cpp 4.76% 20 Missing ⚠️
Code/Source/solver/Parameters.h 16.66% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #537      +/-   ##
==========================================
- Coverage   68.28%   68.15%   -0.13%     
==========================================
  Files         172      174       +2     
  Lines       33997    34160     +163     
  Branches     5929     5932       +3     
==========================================
+ Hits        23214    23281      +67     
- Misses      10646    10742      +96     
  Partials      137      137              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zasexton zasexton marked this pull request as ready for review April 28, 2026 22:15
@zasexton zasexton requested a review from mrp089 April 30, 2026 02:07
@zasexton zasexton requested a review from alexkaiser May 1, 2026 16:46
@aabrown100-git
Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

Copy link
Copy Markdown
Collaborator

@michelebucelli michelebucelli left a comment

Choose a reason for hiding this comment

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

Thank you @zasexton!

I have left a few comments, none of which is critical. This looks good to me!

Comment thread Code/Source/solver/Core/Exception.h Outdated
Comment thread Code/Source/solver/Core/Exception.h Outdated
Comment thread Code/Source/solver/Core/Exception.h
Comment thread Code/Source/solver/Core/Exception.h Outdated
Comment thread Code/Source/solver/FE/Common/FEException.h
Comment thread Code/Source/solver/Parameters.cpp Outdated
Comment thread Code/Source/solver/Parameters.cpp Outdated
addressing additional comments from michele
@zasexton zasexton merged commit 50a1a4a into SimVascular:main May 5, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OOP Refactor Object-Oriented Programming Refactor of Code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants