arkode-fullrhs #293
arkode-fullrhs #293
Conversation
…ing module (and in turn, the MRIStepInnerStepper module).
…le (output files now differ due to reduced numbers of RHS calls, particularly for MRI).
drreynolds
requested review from
balos1,
gardner48 and
cswoodward
as code owners
doc/arkode/guide/source/Usage/MRIStep_c_interface/Custom_Inner_Stepper/Description.rst
Outdated
Show resolved
Hide resolved
doc/arkode/guide/source/Usage/MRIStep_c_interface/Custom_Inner_Stepper/Description.rst
Show resolved
Hide resolved
| MSG_ARK_RHSFUNC_FAILED); | ||
| return(ARK_RHSFUNC_FAIL); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think this could be removed by adding || ark_mem->h == ZERO to if (ark_mem->call_fullrhs_start || ark_mem->call_fullrhs_end) in arkInitialSetup
There was a problem hiding this comment.
@gardner48 Are you referring to this entire change block?
| @@ -766,6 +766,18 @@ int erkStep_TakeStep(void* arkode_mem, realtype *dsmPtr, int *nflagPtr) | |||
| ark_mem->nst, ark_mem->h, ark_mem->tcur); | |||
| #endif | |||
|
|
|||
| /* if fullrhs has not been called, fill in F[0] */ | |||
| if (!ark_mem->call_fullrhs_start) { | |||
There was a problem hiding this comment.
This should not be possible, since nothing should set call_fullrhs_* to false and erkStep_Init sets both to true.
There was a problem hiding this comment.
It seems like it may be safer to handle this even though it is not currently possible to reach it. If it must always be a broken state to reach this, then an error should be thrown if it is reached.
There was a problem hiding this comment.
It might be better to throw an error so we don't unintentionally end up with an extra RHS evaluation at the start of each step.
| @@ -1462,6 +1465,30 @@ int mriStep_TakeStep(void* arkode_mem, realtype *dsmPtr, int *nflagPtr) | |||
| ark_mem->nst, ark_mem->h, ark_mem->tcur); | |||
| #endif | |||
|
|
|||
| /* if fullrhs has not been called, fill in Fse[0] and Fsi[0] as applicable */ | |||
| if (!ark_mem->call_fullrhs_start) { | |||
There was a problem hiding this comment.
Here call_fullrhs_start is at the start of each step but I think this could similarly be accomplished by
- Filling
fse[0]andfsi[0]in the stepper init - If FullRHS is called in
arkInitialSetupit just copies formfse[0]andfsi[0]and adds on the fast time scale contribution - At the end of a step fill
fse[0]andfsi[0]for the next step unlesscall_fullrhs_endis true in which case the FullRHS call inarkCompleteStepwill fill these
There was a problem hiding this comment.
@gardner48 What is the advantage of your proposal?
There was a problem hiding this comment.
I think this would make the code more consistent. call_fullrhs_start is really more like call the full RHS at initialization and then this call (like in ERKStep) never actually happens and the full RHS is actually updated at the end of the step.
Co-authored-by: David Gardner <[email protected]>
|
Closing this PR (similar updates are included in an upcoming PR by @gardner48) |
This PR makes the necessary changes so that
fullrhsis an optional routine for both an ARKODE time-stepping module, and for anMRIInnerSteppermodule. To test this, the PR also converts many ARKODE examples to request the Lagrange interpolation module.The PR is not quite complete, but I'm submitting it now for 2 reasons:
.outfiles for the now-changed results.In addition to missing the new
.outfiles, this PR also needs updates to both the ARKODE documentation andCHANGELOG.md.