-
Notifications
You must be signed in to change notification settings - Fork 117
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
New Low Order MRI Methods #439
Conversation
Co-authored-by: Daniel R. Reynolds <reynolds@smu.edu>
… feature/low-order-mri
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.
All of this looks good to me, but this is missing documentation of the new tables, as well as comments in the change log and recent changes files.
Co-authored-by: David Gardner <gardner48@llnl.gov>
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.
One tiny typo, otherwise this looks great.
Co-authored-by: Daniel R. Reynolds <reynolds@smu.edu>
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.
Once all of the CI tests complete, I think that this is good to merge.
Should be ready to merge now pending the jenkins test and any final reviews |
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.
Some minor doc updates (I'll make the changes in a sec), otherwise this looks good.
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.
Updated the constants table with the new methods and added default first order methods in ERKStep and ARKStep.
MRIStepSetOrder
consistent withARKStepSetOrder
andERKStepSetOrder
so that a negative value sets the default order and a positive value out of range prints an error (when method initialization occurs not whenSetOrder
is called).Before I update the docs, stability plots, changelog, etc, I want to make sure everyone's ok with these new methods.