Refactor DAE solver - #2644
Conversation
|
@charlesm93 interest in reviewing this? |
…4.1 (tags/RELEASE_600/final)
|
Yes, I can take a stab at reviewing this by the end of the week. |
Jenkins Console Log Machine informationProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010CPU: G++: Clang: |
charlesm93
left a comment
There was a problem hiding this comment.
Overall this looks good. I'm not familiar with some of the C++ code, so I'll need to have another read and I'll likely ask more questions.
The API suggests a more general solver than I anticipated, because f doesn't need to depend linearly on y'. This means we can specify ODEs which are non-linear in y'. By contrast, functors we pass to the ODE solver have the format y' = f. Does this mean the DAE solver expands the class of ODEs we can solve?
Just checking: how are the sensitivities computed? I'm assuming we're using a forward method which computes the full Jacobian of sensitivities and that the adjoint method is currently not an option for DAEs.
I would propose to add two unit tests: one in which the solver solves a regular ODE and one in which it solves an algebraic equation. These seem to be important limiting cases. That said, I don't think these tests are required for the PR.
For the DAE the functor specifies the residual, not the right-hand-side. The residual f(t, y, y', ts) = 0
Adjoint is provided by IDAS but I choose to only implement the forward method. Since this is a refactor of things I did a few years ago, IMO it's better to leave major features to the future.
Good point, I should add them to unit tests. |
…4.1 (tags/RELEASE_600/final)
…4.1 (tags/RELEASE_600/final)
Jenkins Console Log Machine informationProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010CPU: G++: Clang: |
There was a problem hiding this comment.
At this point my main concerns have been addressed. @yizhang-yiz you can do additional changes as you described in your comments and then merge the PR.
|
Thanks a lot @charlesm93 |
Jenkins Console Log Machine informationProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010CPU: G++: Clang: |
Addressing #2643.
Summary
The refactored DAE implementation consists of
dae(...)with default tolerance & max num stepsdae_tol(...)with user-supplied controlsTests
Typed tests:
analytical_dae_typed_test: a simple DAE with closed-form solutionchem_dae_typed_test: Robertson's modelpph_dae_typed_test: predator-prey-harvesting modelThe tests are done with type combinations of
yy(state),yp(state derivative), andtheta(parameters). Each of the three is tested withdoubleandstan::math::var_value<double>. The validity of the results are tested againstanalytical_dae_typed_test)chem_dae_typed_test&pph_dae_typed_test)Side Effects
n/a
Release notes
Refactor DAE solver.
Checklist
Math issue Refactor DAE implementation #2643
Copyright holder: Metrum Research Group. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
./runTests.py test/unit)make test-headers)make test-math-dependencies)make doxygen)make cpplint)the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested