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

Add inlet outlet bc #633

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from
Open

Add inlet outlet bc #633

wants to merge 19 commits into from

Conversation

g-rydquist
Copy link
Contributor

@g-rydquist g-rydquist commented Dec 5, 2024

Add an inlet boundary condition that prescribes a velocity on a sideset. Pressure and density are set such that they have zero gradient. A couple of potential features are missing as of now:

  1. Outlet BC has not been added.
  2. There is only functionality to assign a single inlet block to all sidesets (the first one in the .lua file). The functionality to read and store multiple inlets is there, but I wasn't sure how to, in BCFunctions::inlet(), differentiate which sideset was associated with which inlet, so I just take the first one that gets stored. However, this inlet can be assigned to multiple sidesets.

A further note: instead of removing the loop over the bclist::Keys in the MultiMat constructor and looping manually, I removed inlet from the list and just read it manually.


This change is Reviewable

@g-rydquist g-rydquist self-assigned this Dec 5, 2024
Copy link
Member

@adityakpandare adityakpandare 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 @g-rydquist. The inlet BC parsing code looks good. I have a few comments on the BC enforcement itself.

Reviewed 16 of 16 files at r1, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @g-rydquist)


src/Inciter/Transporter.cpp line 484 at r1 (raw file):

  using bclist = ctr::bclist::Keys;
  const auto& bcs = g_inputdeck.get< tag::bc >();
  brigand::for_each< bclist >( UserBC( g_inputdeck, usedsets ) );

This brigand::for_each is neither for timedep nor inlet BCs. Let's move it above the comment, as it was before (undo the change of moving it under the comment). Same for the bclist.


src/Inciter/Transporter.cpp line 489 at r1 (raw file):

      for (auto i : b.get< tag::sideset >())
        usedsets.insert(static_cast<int>(i));
    }

Nice! I like this! At some point, I will attempt at moving all BC's to this framework.


src/PDE/MultiMat/FVMultiMat.hpp line 102 at r1 (raw file):

            v.clear();
            v.insert(v.end(), sideset.begin(), sideset.end());
            m_bc.push_back( { v, inlet, noOpGrad } );

Is the noOpGrad correct for inlet? Shouldn't we be using velocity/temperature info from the BCs to get the grad? Or are we punting on it for now (which is okay; just making sure)?


src/Control/Inciter/InputDeck/InputDeck.hpp line 139 at r1 (raw file):

      tag::materialid,   std::size_t
    > >
  >,

Just to be a little "clear", let's move the tag::inlet to the very end, adjacent to tag::timedep, as a hint that inlet is treated similar to timedep.


src/PDE/MultiMat/BCFunctions.hpp line 137 at r1 (raw file):

    auto nmat = g_inputdeck.get< tag::multimat, tag::nmat >();
    const auto& solidx = g_inputdeck.get< tag::matidxmap, tag::solidx >();
    auto& inbc = g_inputdeck.get< tag::bc >()[0].get< tag::inlet >();

This index 0 corresponds to the mesh id. This indicates that for now, our MultiMat solver BCs aren't linked to the overset mesh solver.


src/PDE/MultiMat/BCFunctions.hpp line 140 at r1 (raw file):

    // inlet velocity and material
    auto u_in = inbc[0].get< tag::velocity >();

This is where only the first entry of the tag::inlet vector is used. I see. This will need some reworking. Let's create an issue on github, pointing to this function, and mentioning that it is not compatible with multiple inlet-bcs.


src/PDE/MultiMat/BCFunctions.hpp line 158 at r1 (raw file):

    auto v1r = 2.0*u_in[0] - v1l;
    auto v2r = 2.0*u_in[1] - v2l;
    auto v3r = 2.0*u_in[2] - v3l;

I'm not sure this vector algebra works out for an unstructured mesh an general velocity. Instead, the ghost-state velocity should be set to the inlet BC values. So basically, v1r = u_in[0] etc. This is what is called a "weak" BC enforcement that's typically used in FV/DG formulations.


src/PDE/MultiMat/BCFunctions.hpp line 177 at r1 (raw file):

      // zero bulk pressure and density gradient
      ur[ncomp+pressureIdx(nmat, k)] = ur[volfracIdx(nmat, k)] * p;

This quantity is the material pressure, not the bulk pressure. So if you had different EOS for each material, this code is incorrect. If the inlet BC is supposed to be used for subsonic conditions, use the pressure from inside (one characteristic moving out from inside at subsonic conditions):

ur[ncomp+pressureIdx(nmat, k)] = ur[volfracIdx(nmat,k)] * ul[ncomp+pressureIdx(nmat,k)]/ul[volfracIdx(nmat,k)];

If the inlet BC is supposed to be used for supersonic conditions, we need to accept inlet PC pressure and use that as the pressure (all characteristics moving in at supersonic conditions):

ur[ncomp+pressureIdx(nmat, k)] = ur[volfracIdx(nmat,k)] * p_in;

src/PDE/MultiMat/BCFunctions.hpp line 178 at r1 (raw file):

      // zero bulk pressure and density gradient
      ur[ncomp+pressureIdx(nmat, k)] = ur[volfracIdx(nmat, k)] * p;
      ur[densityIdx(nmat,k)] = ur[volfracIdx(nmat,k)] * rho;

This quantity is the material density, not the bulk density. So if you had different EOS for each material, this code is incorrect. We need to use the EOS to compute material density based on the user-specified inlet BC temperature and appropriate pressure (based on sub or super sonic).


src/PDE/MultiMat/BCFunctions.hpp line 180 at r1 (raw file):

      ur[densityIdx(nmat,k)] = ur[volfracIdx(nmat,k)] * rho;
      ur[energyIdx(nmat,k)] = ur[volfracIdx(nmat,k)] *
        mat_blk[k].compute< EOS::totalenergy >(rho, v1r, v2r, v3r, p);

Material density should be used here: ur[densityIdx...]/ur[volfracIdx...].


src/PDE/MultiMat/BCFunctions.hpp line 185 at r1 (raw file):

    ur[momentumIdx(nmat, 0)] = rho * v1r;
    ur[momentumIdx(nmat, 1)] = rho * v2r;
    ur[momentumIdx(nmat, 2)] = rho * v3r;

Bulk density rho needs to be recomputed once each material density is computed from EOS as commented above.


src/PDE/MultiMat/DGMultiMat.hpp line 108 at r1 (raw file):

          }
        }
      }

This function repeats in all fv/dg implementations. Let's move it outside to DGPDE.cpp and have everyone call that function.

Copy link
Contributor Author

@g-rydquist g-rydquist left a comment

Choose a reason for hiding this comment

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

Reviewable status: 9 of 18 files reviewed, 13 unresolved discussions (waiting on @adityakpandare)


src/Control/Inciter/InputDeck/InputDeck.hpp line 139 at r1 (raw file):

Previously, adityakpandare (Aditya Pandare) wrote…

Just to be a little "clear", let's move the tag::inlet to the very end, adjacent to tag::timedep, as a hint that inlet is treated similar to timedep.

Done.


src/Inciter/Transporter.cpp line 484 at r1 (raw file):

Previously, adityakpandare (Aditya Pandare) wrote…

This brigand::for_each is neither for timedep nor inlet BCs. Let's move it above the comment, as it was before (undo the change of moving it under the comment). Same for the bclist.

Done.


src/PDE/MultiMat/BCFunctions.hpp line 140 at r1 (raw file):

Previously, adityakpandare (Aditya Pandare) wrote…

This is where only the first entry of the tag::inlet vector is used. I see. This will need some reworking. Let's create an issue on github, pointing to this function, and mentioning that it is not compatible with multiple inlet-bcs.

Done.


src/PDE/MultiMat/BCFunctions.hpp line 158 at r1 (raw file):

Previously, adityakpandare (Aditya Pandare) wrote…

I'm not sure this vector algebra works out for an unstructured mesh an general velocity. Instead, the ghost-state velocity should be set to the inlet BC values. So basically, v1r = u_in[0] etc. This is what is called a "weak" BC enforcement that's typically used in FV/DG formulations.

So I'd been thinking about this, and I'm not fully sure I'm following. I hadn't heard of weak enforcement of BC's, but after reading up on them, I get what's going on. However, I'm not exactly able to make the jump from including the Dirichlet BC's in the variational form to setting the ghost cells to them directly (unless I've been reading up on something different.) And then why are no-slip BC's calculated by "mirroring" the interior cell with the ghost cell, instead of setting the ghost cell directly to 0?


src/PDE/MultiMat/BCFunctions.hpp line 177 at r1 (raw file):

Previously, adityakpandare (Aditya Pandare) wrote…

This quantity is the material pressure, not the bulk pressure. So if you had different EOS for each material, this code is incorrect. If the inlet BC is supposed to be used for subsonic conditions, use the pressure from inside (one characteristic moving out from inside at subsonic conditions):

ur[ncomp+pressureIdx(nmat, k)] = ur[volfracIdx(nmat,k)] * ul[ncomp+pressureIdx(nmat,k)]/ul[volfracIdx(nmat,k)];

If the inlet BC is supposed to be used for supersonic conditions, we need to accept inlet PC pressure and use that as the pressure (all characteristics moving in at supersonic conditions):

ur[ncomp+pressureIdx(nmat, k)] = ur[volfracIdx(nmat,k)] * p_in;

Sounds good. As I was putting these in, I realized we should also likely have some error checking for if the user puts in a velocity that is flowing out as well...


src/PDE/MultiMat/BCFunctions.hpp line 178 at r1 (raw file):

Previously, adityakpandare (Aditya Pandare) wrote…

This quantity is the material density, not the bulk density. So if you had different EOS for each material, this code is incorrect. We need to use the EOS to compute material density based on the user-specified inlet BC temperature and appropriate pressure (based on sub or super sonic).

Done.


src/PDE/MultiMat/BCFunctions.hpp line 180 at r1 (raw file):

Previously, adityakpandare (Aditya Pandare) wrote…

Material density should be used here: ur[densityIdx...]/ur[volfracIdx...].

Done.


src/PDE/MultiMat/BCFunctions.hpp line 185 at r1 (raw file):

Previously, adityakpandare (Aditya Pandare) wrote…

Bulk density rho needs to be recomputed once each material density is computed from EOS as commented above.

Done.


src/PDE/MultiMat/DGMultiMat.hpp line 108 at r1 (raw file):

Previously, adityakpandare (Aditya Pandare) wrote…

This function repeats in all fv/dg implementations. Let's move it outside to DGPDE.cpp and have everyone call that function.

Done.


src/PDE/MultiMat/FVMultiMat.hpp line 102 at r1 (raw file):

Previously, adityakpandare (Aditya Pandare) wrote…

Is the noOpGrad correct for inlet? Shouldn't we be using velocity/temperature info from the BCs to get the grad? Or are we punting on it for now (which is okay; just making sure)?

Yes, I suppose it should be. This was an oversight on my part, and I'll think on it a bit more and add it in a later commit.


src/PDE/MultiMat/BCFunctions.hpp line 273 at r1 (raw file):

        // material pressures
        ur[ncomp+pressureIdx(nmat, k)] = ul[volfracIdx(nmat, k)] * fp;

Based on the discussion above, this is technically incorrect, no? We should be using the volume fraction of the exterior cell, as follows, correct?

Code snippet:

ur[ncomp+pressureIdx(nmat, k)] = ur[volfracIdx(nmat, k)] * fp;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants