Initial discrete adjoint capability with ERK #559

balos1 · 2024-08-27T08:35:25Z

No description provided.

Co-authored-by: Steven Roberts <roberts115@llnl.gov>

…/sunadjoint

drreynolds

I just went through the documentation updates. I'll look through the code soon.

doc/arkode/guide/source/Mathematics.rst

doc/shared/sunadjoint/SUNAdjointCheckpointScheme.rst

doc/shared/sunadjoint/SUNAdjointStepper.rst

doc/shared/sunadjoint/SUNAdjoint_Description.rst

Co-authored-by: Daniel R. Reynolds <reynolds@smu.edu>

…/sunadjoint

drreynolds

A few minor changes on the added/updated content, and I did see one of my previous comments that may still require investigation (inside doc/arkode/guide/source/Usage/ARKStep/ASA.rst)

drreynolds · 2024-11-13T03:06:03Z

doc/shared/sunadjoint/SUNAdjointCheckpointScheme.rst

+   :param cs: The :c:type:`SUNAdjointCheckpointScheme` object
+   :param step_num: the step number of the checkpoint
+   :param stage_num: the stage number of the checkpoint
+   :param t: the time of the checkpoint


t is not an argument to this function

Suggested change

:param t: the time of the checkpoint

(or if it is, then the signature above needs to be fixed)

drreynolds · 2024-11-13T03:09:37Z

doc/shared/sunadjoint/SUNAdjointCheckpointScheme.rst

+   :param keep: Keep data stored even after it is not needed anymore.
+   :param sunctx: The :c:type:`SUNContext` for the simulation.
+   :param check_scheme_ptr: Pointer to the newly constructed object.
+   :return: A :c:type:`SUNErrCode` indicating success or failure.


Does this need a preceding blank line (like the other .. c:function:: blocks above)?

Suggested change

:return: A :c:type:`SUNErrCode` indicating success or failure.

:return: A :c:type:`SUNErrCode` indicating success or failure.

drreynolds · 2024-11-13T03:14:13Z

doc/shared/sunadjoint/SUNAdjointStepper.rst

+
+   .. c:member:: SUNMatrix Jac
+
+      Matrix data for the Jacobian :math:`df/dy`.


I apologize in advance, but the math professor in me needs these Jacobians to be denoted correctly (since $f$ is a function of multiple inputs, and $d$ typically represents the total derivative). I'll "suggest" the relevant changes here and below.

Suggested change

Matrix data for the Jacobian :math:`df/dy`.

Matrix data for the Jacobian :math:`\partial f/\partial y`.

Good catch. It was not in :math: originally, it was plain text so\partial was not used. When I just did a find/replace to add the :math: I overlooked this.

drreynolds · 2024-11-13T03:15:14Z

doc/shared/sunadjoint/SUNAdjointStepper.rst

+
+   .. c:member:: SUNMatrix JacP
+
+      Matrix data for the Jacobian :math:`df/dp`.


Suggested change

Matrix data for the Jacobian :math:`df/dp`.

Matrix data for the Jacobian :math:`\partial f/\partial p`.

drreynolds · 2024-11-13T03:15:33Z

doc/shared/sunadjoint/SUNAdjointStepper.rst

+
+   .. c:member:: SUNRhsJacFn JacFn
+
+      Jacobian function pointer to evaluate :math:`df/dy`.


Suggested change

Jacobian function pointer to evaluate :math:`df/dy`.

Jacobian function pointer to evaluate :math:`\partial f/\partial y`.

drreynolds · 2024-11-13T03:26:58Z

doc/shared/sunadjoint/SUNAdjointStepper.rst

+   Sets the function pointers to evaluate :math:`(df/dy)^T v`  and :math:`(df/dp)^T v`
+
+   :param adj_stepper: The SUNAdjointStepper object.
+   :param Jvp: function that evaluates :math:`(df/dy)^T v`.
+   :param JPvp: function that evaluates :math:`(df/dp)^T v`.


Suggested change

Sets the function pointers to evaluate :math:`(df/dy)^T v` and :math:`(df/dp)^T v`

:param adj_stepper: The SUNAdjointStepper object.

:param Jvp: function that evaluates :math:`(df/dy)^T v`.

:param JPvp: function that evaluates :math:`(df/dp)^T v`.

Sets the function pointers to evaluate :math:`(\partial f/\partial y)^T v` and :math:`(\partial f/\partial p)^T v`

:param adj_stepper: The SUNAdjointStepper object.

:param Jvp: function that evaluates :math:`(\partial f/\partial y)^T v`.

:param JPvp: function that evaluates :math:`(\partial f/\partial p)^T v`.

drreynolds · 2024-11-13T03:27:36Z

doc/shared/sunadjoint/SUNAdjointStepper.rst

+   Sets the function pointers to evaluate :math:`v^T (df/dy)`  and :math:`v^T (df/dp)`
+
+   :param adj_stepper: The SUNAdjointStepper object.
+   :param Jvp: function that evaluates :math:`v^T (df/dy)`.
+   :param JPvp: function that evaluates :math:`v^T (df/dp)`.


Suggested change

Sets the function pointers to evaluate :math:`v^T (df/dy)` and :math:`v^T (df/dp)`

:param adj_stepper: The SUNAdjointStepper object.

:param Jvp: function that evaluates :math:`v^T (df/dy)`.

:param JPvp: function that evaluates :math:`v^T (df/dp)`.

Sets the function pointers to evaluate :math:`v^T (\partial f/\partial y)` and :math:`v^T (\partial f/\partial p)`

:param adj_stepper: The SUNAdjointStepper object.

:param Jvp: function that evaluates :math:`v^T (\partial f/\partial y)`.

:param JPvp: function that evaluates :math:`v^T (\partial f/\partial p)`.

drreynolds · 2024-11-13T03:30:25Z

doc/shared/sunmatrix/SUNMatrix_Operations.rst

+   Performs the matrix-vector product *y \gets A^Tx*.  It should
+   only be called with vectors *x* and *y* that are compatible with
+   the matrix *A^T* -- both in storage type and dimensions.  The return
+   value denotes the success/failure of the operation:


Should these use :math: mode?

Suggested change

Performs the matrix-vector product *y \gets A^Tx*. It should

only be called with vectors *x* and *y* that are compatible with

the matrix *A^T* -- both in storage type and dimensions. The return

value denotes the success/failure of the operation:

Performs the matrix-vector product :math:`y \gets A^Tx`. It should

only be called with vectors :math:`x` and :math:`y` that are compatible with

the matrix :math:`A^T` -- both in storage type and dimensions. The return

value denotes the success/failure of the operation:

drreynolds · 2024-11-13T03:55:37Z

test/unit_tests/arkode/CXX_serial/CMakeLists.txt

+    # "ark_test_erkadjoint.cpp\;--check-freq 1 --no-stages\;"
+    # "ark_test_erkadjoint.cpp\;--check-freq 2 --no-stages\;"
+    # "ark_test_erkadjoint.cpp\;--check-freq 5 --no-stages\;"
+    # "ark_test_erkadjoint.cpp\;--check-freq 1 --dont-keep --no-stages\;"
+    # "ark_test_erkadjoint.cpp\;--check-freq 2 --dont-keep --no-stages\;"
+    # "ark_test_erkadjoint.cpp\;--check-freq 5 --dont-keep --no-stages\;"


Should these just be deleted from the file instead of commented out?

Suggested change

# "ark_test_erkadjoint.cpp\;--check-freq 1 --no-stages\;"

# "ark_test_erkadjoint.cpp\;--check-freq 2 --no-stages\;"

# "ark_test_erkadjoint.cpp\;--check-freq 5 --no-stages\;"

# "ark_test_erkadjoint.cpp\;--check-freq 1 --dont-keep --no-stages\;"

# "ark_test_erkadjoint.cpp\;--check-freq 2 --dont-keep --no-stages\;"

# "ark_test_erkadjoint.cpp\;--check-freq 5 --dont-keep --no-stages\;"

drreynolds · 2024-11-13T03:57:48Z

test/unit_tests/arkode/CXX_serial/ark_test_erkadjoint.cpp

+  // TODO(CJB): figure out why ForwardDiff, CVODES, and ERK adjoint all differ
+  // if (check_sensitivities_backward(sf))
+  // {
+  //   fprintf(stderr,
+  //           ">>> FAILURE: adjoint solution does not match correct answer\n");
+  //   return -1;
+  // };
+  // printf(">>> PASS\n");


Is this "TODO(CJB)" still relevant, or should the comment and associated code be deleted?

balos1 and others added 24 commits August 20, 2024 00:30

Merge branch 'feature/sunstepper' into feature/sunadjoint

96503a8

move trystep flahs

3c3a406

cleanup

cbfbd79

add yp arg

e6815d6

add setstoptime

c7d3f39

rename SUNAdjointSolver

fa21304

Merge branch 'feature/sunstepper' into feature/sunadjoint

32832c1

store time with state data

f9a2c49

add reinit function

9676a70

rename local variables called adj_solver to adj_stepper

1e5eac7

rename adjoint stepper functions to match sunstepper

0b17693

add comment

10d9370

minor cleanup

532dd48

more cleanup

27fe770

minor fixes

b7d7e08

Merge branch 'feature/sunstepper' into feature/sunadjoint

b95404b

add missing SetForcing declaration

491765f

Merge branch 'feature/sunstepper' into feature/sunadjoint

5229e68

Update src/sundials/sundials_stepper.c

bc58f0c

Co-authored-by: Steven Roberts <roberts115@llnl.gov>

Merge remote-tracking branch 'origin/feature/sunstepper' into feature…

14be3e5

…/sunadjoint

wip

8e7e08a

wip

d80ef7a

fix checkpoint throwaway

151a271

add missing arkode functions

acdaef3

balos1 requested review from gardner48, cswoodward and drreynolds as code owners August 27, 2024 08:35

drreynolds reviewed Aug 27, 2024

View reviewed changes

drreynolds self-requested a review August 27, 2024 15:04

Apply suggestions from code review

dbc0207

Co-authored-by: Daniel R. Reynolds <reynolds@smu.edu>

balos1 added 25 commits November 1, 2024 16:44

format

164933a

fix fsal indexing

7237b2c

check error codes

1b18e8c

swig

f2a9b0d

Merge branch 'develop' into feature/sunstepper

a8ab522

format

06e99e3

Basic --> Fixed

eba5765

reorder tests

d386417

switch order

6cf811a

Merge remote-tracking branch 'origin/feature/sunstepper' into feature…

3b7e733

…/sunadjoint

remove unnecessary if do_adjoint

6ff3a78

fix matrix compatability check

59ea02e

bump answers

5a5fd64

loosen tolerances for single precision

b2b8254

support integration in both directions

0be20be

fix comments

0e2ae77

factor out lotka volterra functions into their own header

2560fa7

rename test file

059046b

rename test file

3b7d7f3

bump answers

bd36a0f

make unit test pass/fail

5526f03

format

93a9957

check backwards answer

5a78a2a

update output files for new cost function

066fc3c

comment out backwards adjoint check

abe96c2

drreynolds reviewed Nov 13, 2024

View reviewed changes

balos1 modified the milestones: SUNDIALS Next, SUNDIALS After Next Nov 20, 2024

Base automatically changed from feature/sunstepper to develop November 22, 2024 01:58

balos1 changed the title ~~Feature/sunadjoint~~ Initial discrete adjoint capability with ERK Nov 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial discrete adjoint capability with ERK #559

Initial discrete adjoint capability with ERK #559

balos1 commented Aug 27, 2024

drreynolds left a comment

drreynolds left a comment

drreynolds Nov 13, 2024

drreynolds Nov 13, 2024

drreynolds Nov 13, 2024

balos1 Nov 13, 2024 •

edited

Loading

drreynolds Nov 13, 2024

drreynolds Nov 13, 2024

drreynolds Nov 13, 2024

drreynolds Nov 13, 2024

drreynolds Nov 13, 2024

drreynolds Nov 13, 2024

drreynolds Nov 13, 2024

	:return: A :c:type:`SUNErrCode` indicating success or failure.

	:return: A :c:type:`SUNErrCode` indicating success or failure.


		.. c:member:: SUNMatrix Jac

		Matrix data for the Jacobian :math:`df/dy`.

	Matrix data for the Jacobian :math:`df/dy`.
	Matrix data for the Jacobian :math:`\partial f/\partial y`.


		.. c:member:: SUNMatrix JacP

		Matrix data for the Jacobian :math:`df/dp`.


		.. c:member:: SUNRhsJacFn JacFn

		Jacobian function pointer to evaluate :math:`df/dy`.

Initial discrete adjoint capability with ERK #559

Are you sure you want to change the base?

Initial discrete adjoint capability with ERK #559

Conversation

balos1 commented Aug 27, 2024

drreynolds left a comment

Choose a reason for hiding this comment

drreynolds left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balos1 Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balos1 Nov 13, 2024 •

edited

Loading