Reduce order at user BCs for Godunov#673
Reduce order at user BCs for Godunov#673baperry2 merged 10 commits intoAMReX-Combustion:developmentfrom
Conversation
Source/Godunov.cpp
Outdated
| // because the user specifies conditions at N | ||
| // bcnormal used for "Hard" (Inflow=1) and "UserBC" (6) | ||
| for (int idir = 0; idir < 3; idir++) { | ||
| if (bclo[idir] == Inflow || bclo[idir] == 6) { |
There was a problem hiding this comment.
instead of 6, I think we have a variable to check that? Or maybe we dont...
There was a problem hiding this comment.
I don't believe we do. The BC defines come from AMReX and they don't have a UserBC: https://github.com/AMReX-Codes/amrex/blob/e7d3d5a4fad6559ada6bfaa011438f9e7e317ca7/Src/Base/AMReX_BC_TYPES.H#L103
We may want to have our own defines like ERF: https://github.com/erf-model/ERF/blob/9ea6df90fc497bde2bc2a2e538480d4792507b9c/Source/IndexDefines.H#L159
There was a problem hiding this comment.
Added an enum for this.
Source/Godunov.cpp
Outdated
| yflxbx, [=] AMREX_GPU_DEVICE(int i, int j, int k) noexcept { | ||
| pc_cmpflx( | ||
| i, j, k, bcly, bchy, dly, dhy, qymarr, qyparr, fyarr, q2, qaux, cdir); | ||
| i, j, k, bcly, bchy, dly, dhy, qymarr, qyparr, fyarr, gdtemp, qaux, |
There was a problem hiding this comment.
did you mean to switch from q2 to gdtemp?
There was a problem hiding this comment.
Yeah, although I will have to double check because I'm seeing unexpected errors. But in 3D all the equivalent calls put this into a temporary fab because it shouldn't be needed
There was a problem hiding this comment.
Turns out that was the issue, good catch
|
@jrood-nrel can you take a look at the test that's failing? I haven't seen something fail there before. |
|
I think the Github virtual environments may have been updated. It looks like we mostly have the option that would fix this, which is |
3 participants
In bcnormal, users specify boundaries at step N, but the Godunov fluxes are computed at N+1/2. That leads to an inconsistency. This PR addresses that by computing fluxes for any boundary specified through bcnormal using the data only at step N, with face data always obtained with PLM at these boundaries. This does not affect other types of boundaries.