Repository navigation
Add Implicit Trapezoidal method - #588
Steven-Roberts wants to merge 13 commits into
Conversation
|
Will be invaluable for validating EMT. Important! |
| #define BUBBLE_STEP_FAIL(arg) \ | ||
| do \ | ||
| { \ | ||
| if (int err = (arg)) \ | ||
| { \ | ||
| jacobian_valid_ = false; \ | ||
| (void) restoreAcceptedState(); \ | ||
| return err; \ | ||
| } \ | ||
| } while (false) |
There was a problem hiding this comment.
instead of this scope-guard looking thing, why can't we use something like the experimental scope guards? there are similar things implemented in third-party libraries which could also be used.
| #define BUBBLE_FAIL(arg) \ | ||
| do \ | ||
| { \ | ||
| if (int err = (arg)) \ | ||
| { \ | ||
| return err; \ | ||
| } \ | ||
| } while (false) |
There was a problem hiding this comment.
this seems indicative of an argument for more use of exceptions to avoid this sort of code being used all over the place. not very relevant to this pr, though, outside of wanting to remove these file-local error handling helpers.
There was a problem hiding this comment.
I followed the convention in Rosenbrock.cpp, but I agree exceptions would be more idiomatic than int return values and macros like this.
There was a problem hiding this comment.
@alexander-novo, could we think of a better way to handle errors?
@Steven-Roberts, how are similar instances handled in SUNDIALS?
There was a problem hiding this comment.
@Steven-Roberts, how are similar instances handled in SUNDIALS?
With a similar macro or just writing out the if statement to check the return value. But SUNDIALS does not have access to exceptions, std::expected, and other C++ error handling mechanisms.
There was a problem hiding this comment.
@alexander-novo, could we think of a better way to handle errors?
The returning an int for an error code is a very C-like way of handling errors, and using these kind of quality-of-life macros isn't so uncommon in C I think.
A more C++-like approach might be more natural. std::expected or std::variant as suggested by @superwhiskers work very well in Rust, but won't help us with this specific boilerplate issue. Throwing exceptions will reduce the boilerplate heavily, but we'll need to make sure to heavily document which kinds of exceptions can be thrown by functions.
39c0ad2 to
9084820
Compare
dc6388d to
646b46c
Compare
Description
Adds a fixed step implicit trapezoidal method for comparing against adaptive and partitioned integrators. It uses Newton's method with line search for solving nonlinear systems which was found to be key at large time steps.
Proposed changes
VectorHandleroperations for normsInfNormwhich had an initialization bug and instead use newVectorHandleroperationsImplicitTrapezoidalclassChecklist
-Wall -Wpedantic -Wconversion -Wextra.Further comments
Currently, this is not used outside tests. In a separate PR, we'll need to add support for swapping between IDA, Rosenbrock, and this for phasor dynamics application and other "drivers".