BC::shouldApply

[andrsd]

Background

In CFD, people want to have so called reversible BCs, i.e . one have to interrogate the sign of u * \hat n (u is the velocity vector) and based on that decide what BC will be used (positive for outflow, negative for inflow BC). For the flow equations, we might be applying up to 5 BC on 5 different variables and (to make it more interesting) it can be a mix of nodal and integrated ones.

How to implement this

In RELAP-7, we use the BC::shouldApply() call, which decides if the BC is inflow or outflow and based on that the BC is applied or not. This is very clean solution, however it has a major flaw (see below the 1st paragraph of 'the problem' section).

Now, we could say, let's interrogate the sign in computeResidual(), but that is not possible, because for one (or more) variable(s) we need to apply an integrated BC for the outflow case and nodal BC for the inflow case.

If there is a better way how to do this, I'd like to hear it.

The problem

In ThreadedElementLoopBase, we call subdomainChanged() that calls shouldApply. Now shouldApply wants to look at the value of u variable, but we did not call reinitElem/Face yet, so we cannot access its values. This is all good later on in onElement(), because we reinited the values.

So, why we call shouldApply in subdomainChanged? We are trying to figure out what variables need to be reinited. We build such a list and then reinitElem uses it. So, for shouldApply to get the correct answer (in our flow case), we need to reinit before we build a list of variable that we want to reinit.

Solutions

1. We treat shouldApply as always returning true in subdomainChanged(), i.e. removing the if statement there. Now, if we have a BC that is time depened (i.e. active in t = 0..5 s), we would be reiniting the values from time t=5s on, even if we did not need them.
2. We reinit based on coupled values, i.e. what is coupled to kernels, BCs, DGkernels and reinit those.
3. Something else?

   Why this was not a problem before?

If we run in the legacy mode (i.e. aux kernels execute even if there were not supposed to), something resizes the variable sizes, so we can access some values, but they are not up to date, because of the missing reinit. So it was a problem, we just did not see it...

[friedmud]

I propose having two different functions: shouldApply() and shouldApplyQp() (or something). shouldApply() will still do what it does now (coarse grained on/off).

shouldApplyQp() will get called on every qp/node individually...

[YaqiWang]

Just a reminder you could have a side on which part is inflow and part is outflow. Seems to me mixing modal and integrated BC is not elegant at the beginning. We had something similar when solving convection equation with a least square scheme. (Our case is simpler because u is fixed and outflow BC is natural.) We decided to device an integrated BC for inflow finally.

[andrsd]

@friedmud I like shouldApplyQp

@YaqiWang We have reversible inflow BC and reversible outflow BC. The initial set up is given by the user's choice of the BC. Based on u.n different sets of C++ classes get applied. I do not see where the problem is... Obviously, we are not changing the setup during the runtime. Moreover, mixing nodal and integrated BCs should be possible regardless of the application.

[WilkAndy]

I'm not confiduent i understand what you're talking about exactly, sorry. But it does appear to me that you've got your physics wrong. It this modelling a one-way valve or something similar? Please just ignore this if i'm way off the mark!

In my "richards" stuff, i have boundary conditions that are different depending on the flow direction. They are usually implemented as flow = C*(P-Pref), where P is pressure, Pref is a reference pressure (e.g. atmospheric pressure), and "C" is dependent on P-Pref. Eg, it might be zero for P<Pref (no flow into the system), but 1 for P>=Pref (flow is allowed out).

a

[andrsd]

This is not about physics. This is about technical capability:

1. be able to use variable values in shouldApply()
2. use different set of BC-types (i.e. nodal and integrated) for inflow/outflow situation

I can postpone the decision until I get into computeResidual/Jacobian(), but using shouldApply() is a lot cleaner and I can reuse more code. However, it has problems described above.

[andrsd]

So, I was thinking about this again overnight and we probably do not need shouldApplyQp. We can postpone the decision to computeResidual/Jacobian. If we need to combine nodal and integrated BC, the integrated one will return 0 for both jacobian and residual. The code will not be that nice, but that's fine.

I'll improve the doco for shouldApply so that people are not allowed to use variable values, because at that point they are not up-to-date/computed.