Add Rosenbrock integrators#479
Conversation
3d9d33b to
e8405a0
Compare
ca6fca6 to
b0ec169
Compare
|
@pelesh I've updated the PR to make use of GridKit's own |
Using a linear solver from Re::Solve is a good thing :). We can add bare KLU interface in a separate PR, if needed. |
Allowing for the possibility of return values optimization
Also properly configure the max_steps parameter
shakedregev
left a comment
There was a problem hiding this comment.
Fix the minor issues and good to merge. I tested it.
pelesh
left a comment
There was a problem hiding this comment.
Very nice compact implementation.
Before merging it would be helpful to create clear interfaces between Rosenbrock integrator and GridKit and between Rosenbrock integrator and linear solver.
Also, please implement each class/structure in a separate source/header files.
| /** | ||
| * @brief Converts a GridKit \ref MemorySpace to its corresponding Re::Solve MemorySpace | ||
| * | ||
| */ | ||
| inline ReSolve::memory::MemorySpace memorySpaceAsResolve(MemorySpace memspace) |
There was a problem hiding this comment.
This should not be necessary. GridKit should not see Re::Solve memory space and vice versa.
| template <typename RealT> | ||
| struct StepControl | ||
| { |
There was a problem hiding this comment.
This structure and everything that is not Rosenbrock class should be in separate file. For each class or structure there should be a separate *.hpp and *.cpp files.
| template <typename RealT> | ||
| class StepController | ||
| { |
There was a problem hiding this comment.
Same here. Please move this to a separate file.
| ReSolve::SystemSolver& lin_solver, | ||
| GridKit::LinearAlgebra::VectorHandler<ScalarT, IdxT>& vector_handler, | ||
| const ErrorNorm<ScalarT, IdxT>* err_norm, |
There was a problem hiding this comment.
VectorHandler and ErrorNorm probably should be members of Rosenbrock class. I am not sure why these are passed as constructor parameters.
| /** | ||
| * @brief A simple textbook adaptive `StepController` which seeks to meet a relative and absolute tolerance | ||
| * based on an error estimate. | ||
| * | ||
| */ | ||
| template <typename RealT> | ||
| class AdaptiveStep : public StepController<RealT> | ||
| { |
There was a problem hiding this comment.
This class should be in a separate file.
| template <class ScalarT, typename IdxT> | ||
| Rosenbrock<ScalarT, IdxT>::Tableau Rosenbrock<ScalarT, IdxT>::Tableau::linImplicitEuler() | ||
| { | ||
| constexpr size_t num_stages = 1; |
There was a problem hiding this comment.
I would keep tableaus in the Rosenbrock.cpp file.
| InfNorm<ScalarT, IdxT>::RealT InfNorm<ScalarT, IdxT>::errorNorm(State& err, State& y, State& yprev, GridKit::LinearAlgebra::VectorHandler<ScalarT, IdxT>& handler, ReSolve::memory::MemorySpace memspace) const | ||
| { |
There was a problem hiding this comment.
Re::Solve memory space should not show up here. It is internal to Re::Solve and should not be used in GridKit's VectorHandler.
| resolve_rhs_ = std::make_unique<ReSolve::vector::Vector>(size); | ||
| resolve_lhs_ = std::make_unique<ReSolve::vector::Vector>(size); |
There was a problem hiding this comment.
Re::Solve vector should not appear in Rosenbrock class.
Description
Add Rosenbrock integrators, which are lighter-weight alternatives to IDA that work well with co-simulation.
Proposed changes
The current version depends on ReSolve and needs porting work to be used with GridKit vectors. The operations used by the Rosenbrock integrators are:
This PR also need's ReSolve's
SystemSolverto be ported.As well, I think some more testing wouldn't go amiss... An integration test on an example power grid would work nicely.
Checklist
-Wall -Wpedantic -Wconversion -Wextra.Further comments