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

enhanced first-order flux correction #29

Open
BenWibking opened this issue Feb 27, 2023 · 7 comments
Open

enhanced first-order flux correction #29

BenWibking opened this issue Feb 27, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@BenWibking
Copy link
Contributor

Two things are not clear to me:

  1. Does first-order flux correction (as implemented here) drop to first order fluxes for each stage separately? (i.e., does it still do higher-order time integration if you attempt to use it with RK integrators?)

  2. What happens if there is a bad cell at the edge of a MeshBlock? Are the new fluxes communicated to neighboring MeshBlocks?

@BenWibking
Copy link
Contributor Author

If the answer to 1 is yes, based on a discussion with @forrestglines last week, I think it may be more robust to drop to first-order in time as well. I think we need to keep every RK stage in memory in this case.

@pgrete
Copy link
Contributor

pgrete commented Mar 6, 2023

  1. yes, and I agree that a drop to first order in time would also be more consistent/robust, but would have required additional complexity (regarding keeping all stages in memory as you say).
  2. Yes and no:
  • Yes, for multilevel bounds because Multilevel FluxCorrection is always called. Basically "calc flux -> first_oder_correct -> multi_level_correct -> hope for the best" (with hope for the best because in principle that multi level correct may introduce negative fluxes again)
  • No, for same level bounds, though that might be an easy fix by just extending the bounds for the kernel by +/- 1 in each direction.

@BenWibking
Copy link
Contributor Author

  • No, for same level bounds, though that might be an easy fix by just extending the bounds for the kernel by +/- 1 in each direction.

Would this require increasing the number of ghost zones by 1? That's not too bad of a tradeoff, though, IMO.

@BenWibking
Copy link
Contributor Author

BenWibking commented Mar 6, 2023

  1. yes, and I agree that a drop to first order in time would also be more consistent/robust, but would have required additional complexity (regarding keeping all stages in memory as you say).

Would it be straightforward to add support for ButcherIntegrator from this Parthenon PR: parthenon-hpc-lab/parthenon#840?

Or would you prefer to stick to the low-storage ones in AthenaPK?

@pgrete
Copy link
Contributor

pgrete commented Mar 7, 2023

  • No, for same level bounds, though that might be an easy fix by just extending the bounds for the kernel by +/- 1 in each direction.

Would this require increasing the number of ghost zones by 1? That's not too bad of a tradeoff, though, IMO.

Probably not (assuming that you're running with at least second order/2 ghost zones otherwise).
Given that first order flux correction uses piecewise constant/donor cell reconstruction it just needs a single ghost one.
Extending the "interior" region by one should thus be fine if I didn't miss anything.

@pgrete
Copy link
Contributor

pgrete commented Mar 16, 2023

  • No, for same level bounds, though that might be an easy fix by just extending the bounds for the kernel by +/- 1 in each direction.

Would this require increasing the number of ghost zones by 1? That's not too bad of a tradeoff, though, IMO.

Probably not (assuming that you're running with at least second order/2 ghost zones otherwise). Given that first order flux correction uses piecewise constant/donor cell reconstruction it just needs a single ghost one. Extending the "interior" region by one should thus be fine if I didn't miss anything.

I was just about to implement this, but then realized that it also won't be correct (without even more logic).
The issue is that the first predicted ghost cell layer would need to have all higher order fluxes calculated for an identical prediction as the neighboring block actually owing the data.
This mean would have to extend the the calculation of the standard fluxes fluxes, which, for example, for PPM mean to increase the number of ghost zones required by one.
Do we want/need this at the moment?

@BenWibking
Copy link
Contributor Author

At the resolutions I've run on CPU, it's not needed for my simulations. Maybe we should discuss whether it's needed for the AGN jet simulations.

@BenWibking BenWibking added the enhancement New feature or request label Nov 19, 2023
@BenWibking BenWibking changed the title how does first-order flux correction work in detail? enhanced first-order flux correction Nov 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants