Draft: Implementation of STS algorithm#36
Draft
doraemonho wants to merge 15 commits intolanl:developfrom
Draft
Conversation
1. Added sts folder and files to handle the update. 2. artemis.cpp/hpp modified a) added sts flag b) added Initialize for sts 3. artemis_driver.hpp a) added sts flag b) added the entry for sts in PreStepTasks() fn c) added the entry for sts in PostStepTasks() fn d) Added a check to remove diffusion from main step if sts is enabled 4. gas.cpp a) Added the sts flag b) Added the diffusion timetep for paramters c) Modified the timestep compuation
- changed `sts.cpp/hpp` for implmenting the RKL1 - Bug Fixed in enterting sts_integrator section in `artemis_driver.cpp`
Workable STS rkl1 Solver Update && Format Fix
- Bug fixed for sts.hpp - changed duffusion problem py script to sts solver - changed the conduction input file
STS test problem update
typo fixed in thermal_duffusion.py
Collaborator
Author
|
I think most of the work on RKL1 solver has been finished and passed the CI on Darwin. |
Collaborator
adamdempsey90
left a comment
adamdempsey90
left a comment
There was a problem hiding this comment.
Mostly organizational comments. Looks good otherwise.
Some other things to do:
- Update parameters for the new
<sts>node - Update documentation in
doc/
Comment on lines
+162
to
+168
| if (parthenon::Globals::my_rank == 0 && info_output) { | ||
| Real ratio = tm.dt / min_diff_dt; | ||
| std::cout << "STS ratio: " << ratio << ", Taking " << s_sts << " steps." << std::endl; | ||
| if (ratio > 200.0) { | ||
| std::cout << "WARNING: ratio is > 200. Proceed at own risk." << std::endl; | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
This output should respect the ncycle_out parameter in parthenon/time. So you should make sure this tm.ncycle % tm.ncycle_out == 0 as well.
| params.Add("do_sts", do_sts); | ||
|
|
||
| if (do_sts) { | ||
| params.Add("diff_dt", std::numeric_limits<Real>::max(), Params::Mutability::Mutable); |
Collaborator
There was a problem hiding this comment.
Suggested change
| params.Add("diff_dt", std::numeric_limits<Real>::max(), Params::Mutability::Mutable); | |
| params.Add("diff_dt", Big<Real>(), Params::Mutability::Mutable); |
|
|
||
| Real muj = (2. * stage - 1.) / stage; | ||
| Real nuj = (1. - stage) / stage; | ||
| Real muj_tilde = muj * 2. / (std::pow(nstages, 2.) + nstages); |
Collaborator
There was a problem hiding this comment.
Suggested change
| Real muj_tilde = muj * 2. / (std::pow(nstages, 2.) + nstages); | |
| Real muj_tilde = muj * 2. / ( SQR(nstages) + nstages); |
| //! \brief Assembles the tasks for the STS RKL1 integrator | ||
| // comment: Maybe it should be moved back to artemis_driver.cpp | ||
| template <Coordinates GEOM> | ||
| TaskCollection STSRKL1(Mesh *pmesh, const Real time, Real dt, int stage, int nstages) { |
Collaborator
There was a problem hiding this comment.
I would prefer any function that builds a TaskCollection or sets up Tasks to be in ArtemisDriver directly.
| // Getting the integrator & time ratio between hyperbolic and parabolic terms | ||
| std::string sts_intg_mothod = pin->GetOrAddString("sts", "integrator", "none"); | ||
| Real sts_max_dt_ratio = pin->GetOrAddReal("sts", "sts_max_dt_ratio", -1.0); | ||
| const bool info_output = pin->GetOrAddBoolean("sts", "info_output", false); |
Collaborator
There was a problem hiding this comment.
Is this extra parameter needed? Printing the number of stages every ncycle_out is fine
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
This is an ongoing and not yet finish development to implement the Super-Time-Stepping (STS) algorithm in Artemis to optimize diffusion calculations with relaxed timestep constraints.
Description of Changes
Core Changes
artemis_driver.cppandartemis_driver.hpp: Added STS control parameters and initialization logicartemis.cppandartemis.hpp: Implemented STS integration points and modified timestep handlinggas.cpp: Modified the timestep calculation for STS timestep managementNew Components
sts/directory containing implementation files for the STS algorithmImplementation Status
This is a work in progress, with the following features currently under development:
Checklist