search

ReactionMechanismGenerator / ReactionMechanismSimulator.jl

The amazing Reaction Mechanism Simulator for simulating large chemical kinetic mechanisms
https://reactionmechanismgenerator.github.io/ReactionMechanismSimulator.jl
MIT License
70 stars 33 forks source link

Fix Adjoint Sensitivities #237

Closed mjohnson541 closed 4 months ago

mjohnson541 commented 9 months ago

Issue #235 seems to suggest the issue may be resolved by updating to SciMLSensitivities.jl.

codecov[bot] commented 4 months ago

Codecov Report

Attention: 51 lines in your changes are missing coverage. Please review.

Comparison is base (eed1896) 49.82% compared to head (e80bb70) 49.70%. Report is 30 commits behind head on main.

Files Patch % Lines
src/ReactionMechanismSimulator.jl 0.00% 23 Missing :warning:
src/Simulation.jl 52.00% 12 Missing :warning:
src/Parse.jl 57.14% 9 Missing :warning:
src/fluxdiagrams.jl 0.00% 7 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #237 +/- ## ========================================== - Coverage 49.82% 49.70% -0.13% ========================================== Files 31 31 Lines 8086 8138 +52 ========================================== + Hits 4029 4045 +16 - Misses 4057 4093 +36 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mjohnson541 commented 4 months ago

Ultimately, the nasty error message was a red herring. However, I did resolve it by changing the typing for getadjointsensitivities. The real issue I suspect is that in SciMLSensitivity.jl at too tight of absolute tolerances the CVODE_BDF adjoint solve fails returning an integration that only accounts for the early trajectory of the adjoint solve resulting in nonsense sensitivities. CVODE_BDF does give a warning when this happens, but I think it's one that we're used to ignoring. Reducing the absolute tolerances a little resolves this issue for all tests.