Vmec constructor -> Factory for error handling during initialization. #244
Conversation
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
This was referenced Apr 14, 2025
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
jurasic-pf
force-pushed
the
jurasic-mgrid-absl-errors
branch
from
00cbf09 to
5eefaa0
Compare
jurasic-pf
force-pushed
the
jurasic-vmec-from-indata
branch
from
9a284bb to
cf513ab
Compare
jurasic-pf
force-pushed
the
jurasic-mgrid-absl-errors
branch
from
5eefaa0 to
e582b31
Compare
jurasic-pf
force-pushed
the
jurasic-vmec-from-indata
branch
from
cf513ab to
84f3707
Compare
jurasic-pf
changed the base branch from
jurasic-mgrid-absl-errors
to
graphite-base/244
jurasic-pf
force-pushed
the
jurasic-vmec-from-indata
branch
from
84f3707 to
7cab8ad
Compare
jurasic-pf
force-pushed
the
graphite-base/244
branch
from
e582b31 to
c7640f1
Compare
jurasic-pf
changed the base branch from
graphite-base/244
to
jurasic-mgrid-error-handling
graphite-app
bot
changed the base branch from
jurasic-mgrid-error-handling
to
graphite-base/244
jurasic-pf
force-pushed
the
graphite-base/244
branch
from
c7640f1 to
40b8df7
Compare
jurasic-pf
force-pushed
the
jurasic-vmec-from-indata
branch
from
7cab8ad to
1c1ef4b
Compare
Merge activity
|
jurasic-pf
force-pushed
the
jurasic-vmec-from-indata
branch
from
1c1ef4b to
410e7f9
Compare
graphite-app bot
pushed a commit
that referenced
this pull request
Apr 17, 2025
## Issue The Vmec constructor had absl::CHECK calls in the constructor, which terminated not just the vmecpp execution but also the python wrapper process when mgrid file loading failed, without an Exception or error message that the user could handle. ## Proposed solution ### TLDR Init method for mgrid loading ### Long version The PR refactors the `Vmec` constructor to do pure, exception free initialization (Ignoring the possibility of a memory allocation error from std::vector) and moves all sources of exceptions to places where we can report an `absl::Status`. It removes the magnetic response table constructor of Vmec, and all mgrid related initialization is instead handled in `InitMgrid`, a new method that returns the completion status. **The downside** is, that `Vmec(indata)` does no longer produce a valid object for free-boundary input files. Instead, the mgrid load is finalized during the first call to `run()` if any file IO is required. **Positive** is that the error handling is now correct for missing, invalid mgrid files, wrong resolutions etc. and returns error codes/Python exceptions instead of terminating. A long term better API would be: #244
graphite-app bot
pushed a commit
that referenced
this pull request
Apr 17, 2025
## Issue The Vmec constructor had absl::CHECK calls in the constructor, which terminated not just the vmecpp execution but also the python wrapper process when mgrid file loading failed, without an Exception or error message that the user could handle. ## Proposed solution ### TLDR Init method for mgrid loading ### Long version The PR refactors the `Vmec` constructor to do pure, exception free initialization (Ignoring the possibility of a memory allocation error from std::vector) and moves all sources of exceptions to places where we can report an `absl::Status`. It removes the magnetic response table constructor of Vmec, and all mgrid related initialization is instead handled in `InitMgrid`, a new method that returns the completion status. **The downside** is, that `Vmec(indata)` does no longer produce a valid object for free-boundary input files. Instead, the mgrid load is finalized during the first call to `run()` if any file IO is required. **Positive** is that the error handling is now correct for missing, invalid mgrid files, wrong resolutions etc. and returns error codes/Python exceptions instead of terminating. A long term better API would be: #244
jurasic-pf
force-pushed
the
jurasic-vmec-from-indata
branch
from
410e7f9 to
1e2a5df
Compare
jurasic-pf
changed the base branch from
graphite-base/244
to
jurasic/eigen-bazel-version
This was referenced Apr 22, 2025
Merged
jurasic-pf
changed the base branch from
jurasic/eigen-bazel-version
to
graphite-base/244
jurasic-pf
force-pushed
the
graphite-base/244
branch
from
7b9f562 to
cedeccc
Compare
jurasic-pf
force-pushed
the
jurasic-vmec-from-indata
branch
from
1e2a5df to
610173e
Compare
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.
Building a Factory function is complicated by the current situation with move/copy assignment operators.
Revisit once this is resolved.
#164