Skip to content

Conversation

@simone-silvestri
Copy link
Collaborator

@simone-silvestri simone-silvestri commented Apr 22, 2025

This PR reworks how advection is implemented by passing a reduced_order integer to the interpolate functions.

At the moment, upwind and centered reconstruction do not formally change; the ifelse that was upstream before in the alt_interpolate functions is just pushed downstream in the reconstruction function.

For WENO, the reconstruction approach changes because the order is accounted for by changing the reconstruction coefficients and the smoothness coefficients.
This method should give us a boost in performance in the WENO case for bounded and immersed boundary grids, as there is much less computation to be performed. I will post some benchmarking later on.

This PR is still exploratory, so there is a bit of benchmarking and cleaning up to do.

EDIT: I think this PR might be ready

  • PRO1: faster code with a computational saving that might scale much more efficiently for larger kernels
  • PRO2: same behavior for immersed and non-immersed grids (previously halos in Bounded directions were considered inactive in an immersed grid and active in a non-immersed one, which might have made it difficult to do open boundaries for immersed boundary grids)
  • PRO3: much simpler code for the reduction of the order, which requires less metaprogramming.
  • PRO4: this PR exposed a bug in the distributed computations, where corners were not exchanged for pencil configurations near a bounded wall. This is fixed here, because of PRO3, but it might be a bit more difficult in main to find and fix.
  • CON1: not possible to mix and match different schemes together (for example, having WENO that falls back to Centered near the boundaries)
  • CON2: more difficult to implement higher orders (in main it is sufficient to add a number to the buffers and add the specific coefficients, here you would have to add also the coefficients for the intra-order reconstructions). I am not sure how important this CON is since I don't envision using advection orders higher than 10 for centered and 9 for upwind schemes
  • CON3: a more complicated (and lengthy) code for the WENO coefficients.

Removes unused features:

  • orders 12 for centered and 11 for upwind (never used, unstable, and just a waste of space)
  • vestigial code for stretched advection (now we only have constant coefficients, but some code to enable stretched advection remained after Remove stretched advection #4411)

@navidcy
Copy link
Member

navidcy commented Oct 17, 2025

@simone-silvestri could you resolve the conflicts? I'll try to review... sorry for the slack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmark performance runs preconfigured benchamarks and spits out timing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants