-
Notifications
You must be signed in to change notification settings - Fork 19
1018 use floating point types consistently #1307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
reneSchm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, one more detail in the tests, then I think everything looks good.
@jubicker @annawendler @MaxBetzDLR can you make a final pass as well?
annawendler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Since you implemented the use of ScalarType within the IDE-SEIR model, you can also close issue #594 with this PR (in agreement with @lenaploetzke) :)
MaxBetzDLR
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adaptation to Python bindings look good.
HenrZu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just had a look at some models. Only some short comments. Otherwise, it seems to be fine :)
Thank you for the effort Julian! However, its almost impossible to trace all changes (more trust that the tests are good :D )
Co-authored-by: Henrik Zunker <69154294+HenrZu@users.noreply.github.com>
Extended automatic differentiation (AD) support to the Secirvvs model. |
Changes and Information
This PR extends automatic differentiation (AD) support to the Secirvvs model: 1308
It also improves the consistency of floating-point types in the code.
However, achieving full consistency across the entire codebase is beyond the scope of this PR.
Please briefly list the changes (main added features, changed items, or corrected bugs) made:
UncertainValue<FP>to be used in math expressions with AD types.FP = double/FP = ScalarType.get_matrix_at(FP)→get_matrix_at(SimulationTime<FP>).autotypes with explicitFPto prevent segmentation faults with AD types.data/analyze_result.cpplogic into a header to enable templating.If need be, add additional information and what the reviewer should look out for in particular:
UncertainValue<FP>to automatically call .value() in math expressions.This is needed for the AD datatypes. It has no negative impact on performance in the benchmarks.
It is fairly unintuitive when to call .value() and would clutter the codebase unnecessary.
New FP templatizations
ContactMatrixGroup<FP>,MobilityCoefficients<FP>SimulationTime<FP>,AdoptionRate<FP,…>Simulation<FP>,SimulationNode<FP,…>,IntegratorCore<FP>ParameterStudy<FP,…>,Model<FP,…>,StartDay<FP>DampingMatrixExpressionGroup<FP>,Damping<FP>,Dampings<FP>StateAgeFunctionWrapper<FP>,StateAgeFunction<FP>Interesting things
function<FP>()can break AD instantiations.std::accumulaterequires explicit casting to FP.min({a,b,c})isn’t supported by AD overloads.std::numeric_limits<FP>does not work for AD types. E.g.::maxwould return 0.0. Usestd::numeric_limits<ScalarType>instead.Merge Request - Guideline Checklist
Please check our git workflow. Use the draft feature if the Pull Request is not yet ready to review.
Checks by code author
Checks by code reviewer(s)
Closes #1308 (Extending AD to the Secirvvs Model)
Closes #1258 (Bug in Example: ad_odeint_example)
Closes #1260 (Segmentation Fault in Example: ode_seair_optimization)
Closes #594 (comment)