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

Reaction mask implementation gpu #805

Open
wants to merge 9 commits into
base: development
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Source/PeleC.H
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,10 @@ protected:

static amrex::Vector<int> src_list;

static bool use_chem_mask;
Copy link
Contributor

Choose a reason for hiding this comment

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

The best way to do this is actually a bit different. You would add the option(s) here and run the Source/Params/mk_params.sh script to make this all automatic. I will do the definition/declaration/parmparse for you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also add some documentation about what these new input parameters mean for the user?

static amrex::GpuArray<amrex::Real, AMREX_SPACEDIM> lo_chem_mask_coordinate;
static amrex::GpuArray<amrex::Real, AMREX_SPACEDIM> hi_chem_mask_coordinate;

static bool use_typical_vals_chem;
static bool use_typical_vals_chem_usr;
static amrex::Real typical_rhoY_val_min;
Expand Down
16 changes: 16 additions & 0 deletions Source/PeleC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ amrex::Vector<std::string> PeleC::m_diagVars;

amrex::Vector<int> PeleC::src_list;

bool PeleC::use_chem_mask = false; // Flag to check if mask is activated.
amrex::GpuArray<amrex::Real, AMREX_SPACEDIM>
PeleC::lo_chem_mask_coordinate; // Box coordinate low
amrex::GpuArray<amrex::Real, AMREX_SPACEDIM>
PeleC::hi_chem_mask_coordinate; // Box coordinate high

// this will be reset upon restart
amrex::Real PeleC::previousCPUTimeUsed = 0.0;
amrex::Real PeleC::startCPUTime = 0.0;
Expand Down Expand Up @@ -250,6 +256,16 @@ PeleC::read_params()
pp.query("use_typ_vals_chem", use_typical_vals_chem);
pp.query("use_typ_vals_chem_usr", use_typical_vals_chem_usr);

pp.query("chem_mask", use_chem_mask);

// Reading chemistry mask box coordinates
if (use_chem_mask) {
for (int n = 0; n < AMREX_SPACEDIM; ++n) {
pp.get("lo_chemmask", lo_chem_mask_coordinate[n], n);
Copy link
Contributor

Choose a reason for hiding this comment

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

let's be more explicit with this user facing string lo_chemistry_mask e.g.

pp.get("hi_chemmask", hi_chem_mask_coordinate[n], n);
}
}

if (use_typical_vals_chem_usr) {
use_typical_vals_chem = true;
}
Expand Down
35 changes: 34 additions & 1 deletion Source/React.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ PeleC::react_state(

const amrex::Real strt_time = amrex::ParallelDescriptor::second();

const auto plo = geom.ProbLoArray();
const auto dx = geom.CellSizeArray();

AMREX_ASSERT(do_react == 1);

if ((verbose != 0) && amrex::ParallelDescriptor::IOProcessor()) {
Expand Down Expand Up @@ -126,6 +129,15 @@ PeleC::react_state(
#pragma omp parallel if (amrex::Gpu::notInLaunchRegion())
#endif
{

amrex::RealBox Chem_Masked_Region(
Copy link
Contributor

Choose a reason for hiding this comment

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

name this masked_chemistry_box, make it const.

AMREX_D_DECL(
lo_chem_mask_coordinate[0], lo_chem_mask_coordinate[1],
lo_chem_mask_coordinate[2]),
AMREX_D_DECL(
hi_chem_mask_coordinate[0], hi_chem_mask_coordinate[1],
hi_chem_mask_coordinate[2]));

for (amrex::MFIter mfi(S_new, amrex::TilingIfNotGPU()); mfi.isValid();
++mfi) {

Expand Down Expand Up @@ -170,6 +182,20 @@ PeleC::react_state(
auto const& mask = dummyMask.array(mfi);
Copy link
Contributor

Choose a reason for hiding this comment

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

while we are at it. Let's rename this "dummyMask` to something that means something.

auto const& fc = fctCount.array(mfi);

if (use_chem_mask) {
amrex::ParallelFor(
bx, [=] AMREX_GPU_DEVICE(int i, int j, int k) noexcept {
amrex::XDim3 point;
Copy link
Contributor

Choose a reason for hiding this comment

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

can simplify: const amrex::IntVect iv = {AMREX_D_DECL(plo[0] + (i + 0.5) * dx[0], etc)}; see other examples in the code.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably just happen once? No need to do it every step? if so, can we move to the init somewhere?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think @marchdf means const amrex::RealVect = .... The AMREX_D_DECL is needed so the code also works in 2d. You can also use the pc_cmp_loc function which wraps this.

point.x = plo[0] + (i + 0.5) * dx[0];
point.y = plo[1] + (j + 0.5) * dx[1];
point.z = plo[2] + (k + 0.5) * dx[2];

if (Chem_Masked_Region.contains(point)) {
mask(i, j, k) = -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

You can then make this a ternary expression: mask(i, j, k) = Chem_Masked_Region.contains(iv)? -1 : 0;. It's also weird to me that mask is -1 when masked. Should it be 0? And then 1 for when unmasked. That would fit my mental definition of true/false and other uses of the term "mask" in amrex codes.

}
});
}

amrex::ParallelFor(
bx, [=] AMREX_GPU_DEVICE(int i, int j, int k) noexcept {
// work on old state
Expand Down Expand Up @@ -209,7 +235,7 @@ PeleC::react_state(
);

amrex::Gpu::Device::streamSynchronize();

bool use_chem_mask_d = use_chem_mask;
Copy link
Contributor

Choose a reason for hiding this comment

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

const this.

// unpack data
amrex::ParallelFor(
bx, [=] AMREX_GPU_DEVICE(int i, int j, int k) noexcept {
Expand All @@ -236,6 +262,13 @@ PeleC::react_state(
- rho_old * e_old) // old internal energy
/ dt;

if (use_chem_mask_d && mask(i, j, k) == -1) {
for (int nsp = 0; nsp < NUM_SPECIES; nsp++) {
rhoY(i, j, k, nsp) += nonrs_arr(i, j, k, UFS + nsp) * dt;
}
rhoY(i, j, k, NUM_SPECIES) += rhoedot_ext * dt;
}

amrex::Real umnew =
sold_arr(i, j, k, UMX) + dt * nonrs_arr(i, j, k, UMX);
amrex::Real vmnew =
Expand Down