-
Notifications
You must be signed in to change notification settings - Fork 125
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
Feature/sunadjoint #559
base: feature/sunstepper
Are you sure you want to change the base?
Feature/sunadjoint #559
Conversation
Co-authored-by: Daniel R. Reynolds <[email protected]>
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.
Since some commits are being made while reviewing, I'll just submit what I have now so they don't get out of sync. I'm mostly through the docs
.. c:function:: SUNErrCode SUNAdjointStepper_RecomputeFwd(SUNAdjointStepper adj_stepper, int64_t start_idx,\ | ||
sunrealtype t0, sunrealtype tf, N_Vector y0) | ||
|
||
Evolves the forward system in time from start_idx/t0 to stop_idx/tf with dense checkpointing. |
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.
Unclear if start_idx/t0
is a division?
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.
These are the comments for the rest of the docs
Allocates a ``SUNMemory`` object whose ``ptr`` field is allocated for | ||
``mem_size`` bytes and is of type ``mem_type``. The new object will have | ||
ownership of ``ptr`` and will be deallocated when | ||
:c:func:`SUNMemoryHelper_Dealloc` is called. |
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.
This appears to be copied from SUNMemoryHelper_Alloc
, and it's unclear to me if the description needs to be different to account for the stride
argument.
The implementation provides the following operations defined by the | ||
``SUNMemoryHelper`` API: | ||
|
||
.. c:function:: SUNErrCode SUNMemoryHelper_Alloc_Sys(SUNMemoryHelper helper, \ |
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 realize this is following the convention of the other SUNMemoryHelpers so this will need to be considered outside this PR, but I don't see why these should be made public. Shouldn't users call SUNMemoryHelper_Alloc
rather than SUNMemoryHelper_Alloc_Sys
?
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.
@gardner48 Do you think we should document the implementation specific overrides of the generic functions? We do so somewhat inconsistently as of now .
Co-authored-by: Steven Roberts <[email protected]>
Co-authored-by: Steven Roberts <[email protected]>
No description provided.