Add radiation

[majosm]

Adds radiation sink term to fluid-wall coupling.

Questions for the review:

• Is the scope and purpose of the PR clear?
  • The PR should have a description.
  • The PR should have a guide if needed (e.g., an ordering).
• Is every top-level method and class documented? Are things that should be documented actually so?
• Is the interface understandable? (I.e. can someone figure out what stuff does?) Is it well-defined?
• Does the implementation do what the docstring claims?
• Is everything that is implemented covered by tests?
• Do you see any immediate risks or performance disadvantages with the design? Example: what do interface normals attach to?

[tulioricci]

@majosm , two comments:

1. should we standardize the nomenclature of BCs? Right now, this is what we have:
   IsothermalSlipWallBoundary
   IsothermalWallBoundary
   AdiabaticSlipBoundary
   AdiabaticNoslipWallBoundary

2. replace "wall_epsilon" with "wall_emissivity"

[majosm]

2. replace "wall_epsilon" with "wall_emissivity"

Done (forgot to push 🙂).

[MTCam]

@majosm , two comments:

1. should we standardize the nomenclature of BCs? Right now, this is what we have:
   IsothermalSlipWallBoundary
   IsothermalWallBoundary
   AdiabaticSlipBoundary
   AdiabaticNoslipWallBoundary

I dislike the disparity in naming, too. I also dislike when every BC has "WallBoundary" attached to it, if they all have it, then why have it at all? For me, these could easily be IsothermalSlip, IsothermalNoslip , etc. It's already known as a BC because we from mirgecom.boundary import IsothermalNoslip.

Now that we have multiple domains and materials, we could re-think some of the code structuring too, like renaming mirgecom.boundary to mirgecom.fluid_boundary. But again, these are all non-essential changes that can easily distract us and seem unimportant unless we are already set up to really nail the prediction.

Edit: also out of scope for this pr

[tulioricci]

If the function is deprecated, I wouldn't updated it with radiation.

[majosm]

31ce323

[tulioricci]

And we don't need to get the gradient here (

mirgecom/mirgecom/multiphysics/thermally_coupled_fluid_wall.py

Line 1839 in d512ebd

return_grad_u=return_gradients, dd=wall_dd, grad_u=wall_grad_temperature,

)
because it is already computed above

[tulioricci]

Should we default it to its actual value 5.67e-8?

[majosm]

I worry about specifying default values for constants, because it invites the possibility of omitting them by accident, leading to silently incorrect results if the value is supposed to be something other than the default. Better to produce an error I think.

[majosm]

And we don't need to get the gradient here (

mirgecom/mirgecom/multiphysics/thermally_coupled_fluid_wall.py

Line 1839 in d512ebd

return_grad_u=return_gradients, dd=wall_dd, grad_u=wall_grad_temperature,

)
because it is already computed above

It's not actually being recomputed there, since we're passing it in as grad_u (and similarly for the fluid side). So basically it's just cosmetic. I tried removing it (code here), but I don't know if it's any cleaner. 🤷‍♂️ Thoughts?

[tulioricci]

And we don't need to get the gradient here (

mirgecom/mirgecom/multiphysics/thermally_coupled_fluid_wall.py

Line 1839 in d512ebd

return_grad_u=return_gradients, dd=wall_dd, grad_u=wall_grad_temperature,

)
because it is already computed above

It's not actually being recomputed there, since we're passing it in as grad_u (and similarly for the fluid side). So basically it's just cosmetic. I tried removing it (code here), but I don't know if it's any cleaner. man_shrugging Thoughts?

I prefer the new version, but it is not a big deal since, in practice, we may end up having this in the driver itself... 🤷‍♂️

[tulioricci]

LGTM, but I will let @MTCam give the final word.

[MTCam]

👍