Skip to content

Conversation

@jtcooper10
Copy link
Collaborator

@jtcooper10 jtcooper10 commented May 1, 2022

Implements support for CVODE integrator options in C++ solvers, as well as implementing a more extensible approach for validating and modifying supported integrator options for each solver.

Each solver may "declare" which integrator options they support by overriding the GillesPySolver.get_supported_integrator_options class method and returning a set of supported options.

Changes

  • TauHybridCSolver#run and ODECSolver#run now support integrator_options as an argument
    • Supports rtol, atol, and max_step as options

Fixes

  • Added reset to integration guard

Closes #730

- Added class methods for getting/validating integrator options
- Updated `TauHybridCSolver` to use it
@jtcooper10 jtcooper10 added the enhancement New feature or request label May 1, 2022
@jtcooper10 jtcooper10 self-assigned this May 1, 2022
jtcooper10 added 2 commits May 2, 2022 13:32
- Move `--atol`, `--reltol`, and `--max_step` results into separate struct
- Update ODE and Hybrid C++ solvers to use this struct
- Added `--max_step` support to `ODECSolver`
- Added `configure()` method to `TauHybridCSolver`'s integrator
@jtcooper10 jtcooper10 marked this pull request as ready for review May 2, 2022 18:29
@BryanRumsey BryanRumsey mentioned this pull request May 3, 2022
@briandrawert
Copy link
Member

Please add unit test to see that an warning is not raised when --rtol, etc are passed to the solver object.

Copy link
Contributor

@BryanRumsey BryanRumsey left a comment

Choose a reason for hiding this comment

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

Integrator options were not added to ODECSolver.run

@jtcooper10
Copy link
Collaborator Author

Integrator options were not added to ODECSolver.run

Added along with a unit test.

@jtcooper10
Copy link
Collaborator Author

Please add unit test to see that an warning is not raised when --rtol, etc are passed to the solver object.

A unit test was added.

@BryanRumsey BryanRumsey self-requested a review May 31, 2022 13:49
@briandrawert briandrawert merged commit a8cde13 into develop Jun 1, 2022
@briandrawert briandrawert deleted the cpp-integrator-options branch June 1, 2022 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TauHybridCSolver, ODECSolver do not use tau_tol, or integrator_options

4 participants