Skip to content
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

Support running VL+CT Integrator as a single stage integrator #315

Merged
merged 25 commits into from
Feb 22, 2024

Conversation

mabruzzo
Copy link
Contributor

@mabruzzo mabruzzo commented May 31, 2023

Overview

This PR is long-overdue.

  • It adds support for running the VL+CT machinery as a single-stage integrator (for reference the VL+CT is a 2-stage predictor-corrector scheme). This will be really useful for debugging (specifically I need to this to help debug a flux-correction bug).
  • It also lays the ground-work for introducing Runge-Kutta time-integration in the future (just like in Athena++). But that's a topic for another day

Actual Changes

To accomplish these goals, this PR does the following:

  1. It introduces the EnzoMHDIntegratorStageCommands class, which encapsulates all of the functionality that is necessary to update hydro/mhd quantities over an individual stage (the number of stages per timestep depends on the time-integration strategy). This class was necessary since the EnzoMethodMHDVlct class was starting to do too much (and becoming hard to understand). In detail:
    • I moved a bunch of the contents of EnzoMethodMHDVlct::compute into EnzoMHDIntegratorStageCommands::compute_update_stage.
    • While doing this it made sense to migrate a few helper methods, like the compute_flux_ and compute_source_terms_, as well as a bunch of attributes from EnzoMethodMHDVlct to EnzoMHDIntegratorStageCommands.
    • I also migrated a bunch of logic from EnzoMethodMHDVlct's constructor and ``EnzoMethodMHDVlct::timestepover toEnzoMHDIntegratorStageCommands` (I'm less sure that these were optimal choices).
    • EnzoMHDIntegratorStageCommands is ignorant of how the stages in the MHD/Hydro integrator are linked together. That's handled by EnzoMethodMHDVlct. The Method class also tries to take most of the responsibility for interfacing with the rest of Enzo-E
    • The docstrings in the code should hopefully make things pretty clear.
  2. I deprecated the Method:mhd_vlct:half_dt_reconstruct_method and Method:mhd_vlct:full_dt_reconstruct_method parameters. The former only existed due to an oversight on my part (it never made any sense for this to exist - the VL+CT scheme doesn't work unless nearest-neighbor reconstruction is used). The latter is replaced by the new Method:mhd_vlct:reconstruct_method. Backwards compatibility has been maintained.
  3. I introduced the Method:mhd_vlct:time_scheme method to choose the time integration method. Right now, it defaults to "vl" (the predictor-corrector scheme) and the only other choice is "euler" (the simpler 1-stage integrator for testing purposes). In the future, this could be extended to accept other choices (e.g. for a Runge-Kutta integrator).
  4. I updated the website documentation to reflect the changes in the parameters.

PR Dependency

This PR unfortunately needed to build on changes from PR #302 (there would have been too many merge conflicts) and this probably can't be reviewed until that PR is merged (it should be merged soon - just waiting on a reviewer to hit approve after I made their requested changes). I will update this PR when it's ready

EDIT: This is now ready for review.

@mabruzzo mabruzzo added the enhancement New feature or request label May 31, 2023
@mabruzzo mabruzzo force-pushed the variable-mhd-time-integration branch from 85dabf1 to 0680a46 Compare June 2, 2023 15:05
@mabruzzo mabruzzo added the hydro/mhd Issue/PR associated with hydro/mhd label Jun 5, 2023
@jwise77 jwise77 self-requested a review July 10, 2023 16:12
mabruzzo added a commit to mabruzzo/enzo-e that referenced this pull request Jul 21, 2023
Changes introduced in PR enzo-project#315 make it unnecessary for individual classes in the hydro-infrastructure to have PUP routines. Instead of directly serializing the individual classes, the changes introduced in that PR direct the solver to serialize the parameters used to configure the solver, and then reconstructs all of the component classes after a restart. This simplifies things quite a bit.

This commit sheds the now-unnecessary PUP routines.
@mabruzzo mabruzzo requested a review from gregbryan July 26, 2023 17:38
Copy link
Contributor

@gregbryan gregbryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great -- clearly commented.

@jwise77 jwise77 self-assigned this Oct 27, 2023
Copy link
Contributor

@jwise77 jwise77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for this addition! It is structured well and heavily commented. I'm approving and merging.

@jwise77
Copy link
Contributor

jwise77 commented Feb 21, 2024

Oops, no merging yet. @mabruzzo Can you resolve the merge conflict and then we can merge after it passes the tests (again).

@mabruzzo mabruzzo merged commit ab06a70 into enzo-project:main Feb 22, 2024
7 checks passed
@mabruzzo mabruzzo deleted the variable-mhd-time-integration branch February 22, 2024 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request hydro/mhd Issue/PR associated with hydro/mhd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants