-
Notifications
You must be signed in to change notification settings - Fork 31
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
Adjoint sensitivities, fix #876 #917
Changes from 10 commits
c0f8e4f
5f50a3b
bb1f4b9
c761883
8e31038
58965cb
99b2275
24c4e24
994b4bf
bcbb501
3feaa55
29512c8
17f56e8
c661beb
94da2fa
8a6e04e
0cee19b
777f58d
492a7a9
f8fbf3f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,6 +101,13 @@ class BackwardProblem { | |
*/ | ||
realtype getTnext(std::vector<realtype> const& troot, int iroot, int it); | ||
|
||
/** | ||
* @brief Computes the adjoint part of the gradient by solving a linear system. | ||
* | ||
* Valid only if the computed solution is a steady-state. | ||
*/ | ||
void computeIntegralForSteadyState(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ideally move to steadystateproblem.h There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will check it out. I need ExpData to generate the rhs of the problem or pass dJydy from the forward problem. That is why I place that in backwardproblem.h. The right-hand side of the problem is constructed for free in handleDataPointB. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, good point! |
||
|
||
/** | ||
* @brief Compute likelihood sensitivities. | ||
*/ | ||
|
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.
I think this is what causes the CI errors...
Not yet sure what the problem is precisely. Will have a closer look tomorrow
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.
Done @paszkow . If you accept my pull request into your fork, then the CI errors should be fixed now.
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.
Sure I will. I already have all your comments implemented. Need to double-check the correctness of the results I am getting before pushing.
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.
Would be great if you could add these tests as unittests for the proposed, if you need help with that, please let me know!
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.
Sounds like a good idea. Will do.
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.
At least for the small steady-state example problem, things looked correct when I tested them...