Skip to content
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

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
c0f8e4f
Extract methods fJB and fJSparseB to AbstractModel.
Jan 18, 2020
5f50a3b
Prepare linear system for the JB matrix.
Jan 19, 2020
bb1f4b9
Remove fJv calls from NewtonSolverIterative.
Jan 20, 2020
c761883
Enable iterative Newton solver in solving JB*x=rhs.
Jan 20, 2020
8e31038
Simplify computing adjoint sensitivities for steady-states.
Jan 21, 2020
58965cb
Merge remote-tracking branch 'upstream/develop' into adjoint_sensitiv…
Jan 21, 2020
99b2275
Merge branch 'develop' into adjoint_sensitivities
paulstapor Feb 12, 2020
24c4e24
update testfunctions header
paulstapor Feb 12, 2020
994b4bf
Merge branch 'develop' into adjoint_sensitivities
paulstapor Feb 13, 2020
bcbb501
Merge pull request #1 from ICB-DCM/paszkow-adjoint_sensitivities
paszkow Feb 13, 2020
3feaa55
Rename a function to enable Newton solver in addjoint problem.
Feb 1, 2020
29512c8
Simplify computing the inner product.
Feb 1, 2020
17f56e8
Allow steady-state and time itegration in adjoint problem.
Feb 1, 2020
c661beb
Move Neton solver computations to SteadystateProblem.
Feb 1, 2020
94da2fa
Merge branch 'develop' into adjoint_sensitivities
paulstapor Feb 17, 2020
8a6e04e
Fix dimension mismatch for python generated models
Feb 21, 2020
0cee19b
Merge branch 'develop' into adjoint_sensitivities
paulstapor Feb 21, 2020
777f58d
Merge branch 'develop' into adjoint_sensitivities
paulstapor Feb 21, 2020
492a7a9
Merge branch 'develop' into adjoint_sensitivities
paulstapor Mar 2, 2020
f8fbf3f
Measure computation time in backward problem using Newton solver.
Mar 15, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 34 additions & 2 deletions include/amici/abstract_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,22 @@ class AbstractModel {
const AmiVector &dx, const AmiVector &xdot,
SUNMatrix J) = 0;

/**
* @brief Dense Jacobian function
* @param t time
* @param cj scaling factor (inverse of timestep, DAE only)
* @param x state
* @param dx time derivative of state (DAE only)
* @param xB Vector with the adjoint states
* @param dxB Vector with the adjoint derivative states
* @param xBdot Vector with the adjoint right hand side (unused)
* @param JB dense matrix to which values of the jacobian will be written
*/
virtual void fJB(const realtype t, realtype cj, const AmiVector &x,
Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Member

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!

Copy link
Contributor Author

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.

Copy link
Contributor

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.

At least for the small steady-state example problem, things looked correct when I tested them...

const AmiVector &dx, const AmiVector &xB,
const AmiVector &dxB, const AmiVector &xBdot,
SUNMatrix JB) = 0;

/**
* @brief Sparse Jacobian function
* @param t time
Expand All @@ -95,6 +111,22 @@ class AbstractModel {
const AmiVector &x, const AmiVector &dx,
const AmiVector &xdot, SUNMatrix J) = 0;

/**
* @brief Dense Jacobian function
* @param t time
* @param cj scaling factor (inverse of timestep, DAE only)
* @param x state
* @param dx time derivative of state (DAE only)
* @param xB Vector with the adjoint states
* @param dxB Vector with the adjoint derivative states
* @param xBdot Vector with the adjoint right hand side (unused)
* @param JB dense matrix to which values of the jacobian will be written
*/
virtual void fJSparseB(const realtype t, realtype cj, const AmiVector &x,
const AmiVector &dx, const AmiVector &xB,
const AmiVector &dxB, const AmiVector &xBdot,
SUNMatrix JB) = 0;

/**
* @brief Diagonal Jacobian function
* @param t time
Expand Down Expand Up @@ -669,13 +701,13 @@ class AbstractModel {
const realtype *p, const realtype *k, const realtype *h,
const realtype *w, const realtype *tcl,
const realtype *stcl);

/**
* @brief Model specific implementation for dwdp, column pointers
* @param indexptrs column pointers
**/
virtual void fdwdp_colptrs(sunindextype *indexptrs);

/**
* @brief Model specific implementation for dwdp, row values
* @param indexvals row values
Expand Down
7 changes: 7 additions & 0 deletions include/amici/backwardproblem.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Contributor

Choose a reason for hiding this comment

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

ideally move to steadystateproblem.h

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, good point!
Yet, philosophy-wise, I'd prefer to include ExpData into SteadyStateProblem rather than mixing the steadystate and the adjoint part...


/**
* @brief Compute likelihood sensitivities.
*/
Expand Down
10 changes: 9 additions & 1 deletion include/amici/model_dae.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ class Model_DAE : public Model {
void fJ(realtype t, realtype cj, N_Vector x, N_Vector dx, N_Vector xdot,
SUNMatrix J);

void fJB(const realtype t, realtype cj, const AmiVector &x,
const AmiVector &dx, const AmiVector &xB, const AmiVector &dxB,
const AmiVector &xBdot, SUNMatrix JB) override;

/**
* @brief Jacobian of xBdot with respect to adjoint state xB
* @param t timepoint
Expand All @@ -102,7 +106,6 @@ class Model_DAE : public Model {
* @param dxB Vector with the adjoint derivative states
* @param JB Matrix to which the Jacobian will be written
**/

void fJB(realtype t, realtype cj, N_Vector x, N_Vector dx, N_Vector xB,
N_Vector dxB, SUNMatrix JB);

Expand All @@ -121,6 +124,11 @@ class Model_DAE : public Model {
void fJSparse(realtype t, realtype cj, N_Vector x, N_Vector dx,
SUNMatrix J);

void fJSparseB(const realtype t, realtype cj, const AmiVector &x,
const AmiVector &dx, const AmiVector &xB,
const AmiVector &dxB, const AmiVector &xBdot,
SUNMatrix JB) override;

/** JB in sparse form (for sparse solvers from the SuiteSparse Package)
* @param t timepoint
* @param cj scalar in Jacobian
Expand Down
9 changes: 9 additions & 0 deletions include/amici/model_ode.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ class Model_ODE : public Model {
**/
void fJ(realtype t, N_Vector x, N_Vector xdot, SUNMatrix J);

void fJB(const realtype t, realtype cj, const AmiVector &x,
const AmiVector &dx, const AmiVector &xB, const AmiVector &dxB,
const AmiVector &xBdot, SUNMatrix JB) override;

/** implementation of fJB at the N_Vector level, this function provides an
*interface to the model specific routines for the solver implementation
* @param t timepoint
Expand All @@ -119,6 +123,11 @@ class Model_ODE : public Model {
*/
void fJSparse(realtype t, N_Vector x, SUNMatrix J);

void fJSparseB(const realtype t, realtype cj, const AmiVector &x,
const AmiVector &dx, const AmiVector &xB,
const AmiVector &dxB, const AmiVector &xBdot,
SUNMatrix JB) override;

/** implementation of fJSparseB at the N_Vector level, this function
* provides an interface to the model specific routines for the solver
* implementation
Expand Down
56 changes: 51 additions & 5 deletions include/amici/newton_solver.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class NewtonSolver {
void computeNewtonSensis(AmiVectorArray &sx);

/**
* Writes the Jacobian for the Newton iteration and passes it to the linear
* Writes the Jacobian (J) for the Newton iteration and passes it to the linear
* solver
*
* @param ntry integer newton_try integer start number of Newton solver
Expand All @@ -88,6 +88,16 @@ class NewtonSolver {
*/
virtual void prepareLinearSystem(int ntry, int nnewt) = 0;

/**
* Writes the Jacobian (JB) for the Newton iteration and passes it to the linear
* solver
*
* @param ntry integer newton_try integer start number of Newton solver
* (1 or 2)
* @param nnewt integer number of current Newton step
*/
virtual void prepareLinearSystemB(int ntry, int nnewt) = 0;

/**
* Solves the linear system for the Newton step
*
Expand Down Expand Up @@ -125,7 +135,10 @@ class NewtonSolver {
AmiVector *x;
/** current state time derivative (DAE) */
AmiVector dx;

/** current adjoint state */
AmiVector xB;
/** current adjoint state time derivative (DAE) */
AmiVector dxB;
};

/**
Expand Down Expand Up @@ -158,7 +171,7 @@ class NewtonSolverDense : public NewtonSolver {
void solveLinearSystem(AmiVector &rhs) override;

/**
* Writes the Jacobian for the Newton iteration and passes it to the linear
* Writes the Jacobian (J) for the Newton iteration and passes it to the linear
* solver
*
* @param ntry integer newton_try integer start number of Newton solver
Expand All @@ -167,6 +180,16 @@ class NewtonSolverDense : public NewtonSolver {
*/
void prepareLinearSystem(int ntry, int nnewt) override;

/**
* Writes the Jacobian (JB) for the Newton iteration and passes it to the linear
* solver
*
* @param ntry integer newton_try integer start number of Newton solver
* (1 or 2)
* @param nnewt integer number of current Newton step
*/
void prepareLinearSystemB(int ntry, int nnewt) override;

private:
/** temporary storage of Jacobian */
SUNMatrixWrapper Jtmp;
Expand Down Expand Up @@ -204,7 +227,7 @@ class NewtonSolverSparse : public NewtonSolver {
void solveLinearSystem(AmiVector &rhs) override;

/**
* Writes the Jacobian for the Newton iteration and passes it to the linear
* Writes the Jacobian (J) for the Newton iteration and passes it to the linear
* solver
*
* @param ntry integer newton_try integer start number of Newton solver
Expand All @@ -213,6 +236,16 @@ class NewtonSolverSparse : public NewtonSolver {
*/
void prepareLinearSystem(int ntry, int nnewt) override;

/**
* Writes the Jacobian (JB) for the Newton iteration and passes it to the linear
* solver
*
* @param ntry integer newton_try integer start number of Newton solver
* (1 or 2)
* @param nnewt integer number of current Newton step
*/
void prepareLinearSystemB(int ntry, int nnewt) override;

private:
/** temporary storage of Jacobian */
SUNMatrixWrapper Jtmp;
Expand Down Expand Up @@ -249,7 +282,7 @@ class NewtonSolverIterative : public NewtonSolver {
void solveLinearSystem(AmiVector &rhs) override;

/**
* Writes the Jacobian for the Newton iteration and passes it to the linear
* Writes the Jacobian (J) for the Newton iteration and passes it to the linear
* solver.
* Also wraps around getSensis for iterative linear solver.
*
Expand All @@ -259,6 +292,17 @@ class NewtonSolverIterative : public NewtonSolver {
*/
void prepareLinearSystem(int ntry, int nnewt) override;

/**
* Writes the Jacobian (JB) for the Newton iteration and passes it to the linear
* solver.
* Also wraps around getSensis for iterative linear solver.
*
* @param ntry integer newton_try integer start number of Newton solver
* (1 or 2)
* @param nnewt integer number of current Newton step
*/
void prepareLinearSystemB(int ntry, int nnewt) override;

/**
* Iterative linear solver created from SPILS BiCG-Stab.
* Solves the linear system within each Newton step if iterative solver is
Expand Down Expand Up @@ -296,6 +340,8 @@ class NewtonSolverIterative : public NewtonSolver {
AmiVector ns_tmp;
/** ??? */
AmiVector ns_Jdiag;
/** temporary storage of Jacobian */
SUNMatrixWrapper ns_J;
};


Expand Down
Loading