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

FV: Allow more flexible handling of block-restriction with mat prop ghosting #15991

Closed
rwcarlsen opened this issue Oct 21, 2020 · 2 comments
Closed
Labels
C: Modules/Navier Stokes Tickets pertaining to the navier_stokes module P: normal A defect affecting operation with a low possibility of significantly affects. T: task An enhancement to the software.

Comments

@rwcarlsen
Copy link
Contributor

rwcarlsen commented Oct 21, 2020

Reason

This is a problem with FV on the face loops where we are calculating material properties for elem and neighbor. We have this issue where a single material property/name could be used by kernels for two different variables. When calculating props on a face bordering two different blocks/subdomains - if one of the variables is block-restricted and only defined on one of the two blocks, but the other is defined on both blocks. We want to allow block-restriction to work seamlessly for users in FV.

Design

Assume the following scenario:

  • Material object foo defines property "complicatedness" on block 1 only
  • Material object bar defines property "complicatedness" on block 2 only
  • Variable A is defined on block 1
  • Variable B is defined only on block 2
  • Kernel "a" for variable "A" uses property "complicatedness"
  • Kernel "b" for variable "B" uses property "complicatedness"
  • A Dirichlet BC is defined for variable A on the interface between block 1 and 2.
  • A Dirichlet BC is defined for variable B on the interface between block 1 and 2.

Under this circumstance, when evaluating kernels on a face between blocks 1 and 2, to calculate the kernel/residual for kernel "a", we need to ghost the neighbor cell values - so wee need prop "complicatedness" to be set based on the neighbor cell value. But for kernel "b", the material prop "complicatedness" needs a ghosted elem cell value (while neighbor is calculated using object. However, material properties are only evaluated once for each time all kernels are evaluated on a given face. Because a given material property (name) can only have one value at a time, these two kernels "a" and "b" can't both simultaneously have their own particular version of "complicatedness" calculated (with opposite sides ghosted) at the same time. Booo.

So for now, we are restricting block-restricted materials in FV to not allow this duality. Alex is implementing this as we speak. But it should be possible to soften some of the restriction if somebody needs to and is brave enough in the future.
A less condensed version of this same explanation from some chatter between me and Alex is pasted below:

BEGIN PASTED CONVERSATION

The crux is discriminating between: same material prop (provided by multiple materials via block restriction) on different blocks operating under the same physics/vars/kernels; and same material prop (provided by multiple materials via block restriction) on different blocks operating under different physics/vars/kernels. Basically we have this case where there can be block-specific material properties that could share the same property name, but they are semantically different properties (like rho_solid vs rho_fluid but both named rho for example). And there are properties that are meant to be semantically the same - like a continuation of rho_fluid with different values in different blocks - even though rho_fluid isn't a good example for this kind of thing

In every case where it is "correct" to have semantically different properties of the same name, we expect at least one variable that uses the property to be defined on only one of two neighboring blocks (only on one side of a face) - and, if the user doesn't want a natural BC, to also have a BC defined on that (mesh-internal) face. This whole thing is only a problem when evaluating material properties on elem/neighbor for the face/flux loop. The "race condition" comes into play if one variable is defined on one side, and another variable is defined on both sides and kernels for both variables share/use the same material property. Because we only evaluate one material property value for each face calculation (for all kernels regardless of which variable they go with), we can't have that property be both ghosted and normal/not-ghosted at the same time. (edited)

One way we could "relax" your error/assert is we could only error if there are any variables defined only on one side of the face. If all variables are defined on both sides or no sides of the face, then it should still be safe to allow the calculation to procede - and in that case, we don't need to use any ghosting - since for this circumstance it is clear that the intent is that the property is "continuing" the same semantic property through.

And then we keep throwing the error for cases where some variables are only defined on one side of the face, but others are defined on both sides - because in that case, then we have this ambiguity/race condition problem where we (might) need 2 versions of the property simultaneously - one ghosted and the other not.

Here is my final conclusion for how to relax the error/restriction:If any two variables defined on at least one side of a face share a material property - and the side(s) the variables are defined on are not the same for both variables, then we must throw an error. Otherwise - it should be okay to: calculate prop like normal if all variables (using the prop) are defined on both sides; calculate the prop as ghosted if all variables (using the prop) are defined on the same side. (edited)
END PASTED CONVERSATION

Impact

This shouldn't affect existing APIs - just allow users to run FV error free in more sophisticated case with block restricted vars/props, etc.

@rwcarlsen rwcarlsen added T: task An enhancement to the software. P: normal A defect affecting operation with a low possibility of significantly affects. C: Finite Volume labels Oct 21, 2020
@GiudGiud
Copy link
Contributor

The current assert is too restrictive I think. I ll see if I can work something out to only capture the problematic cases (or at least to let mine through)

GiudGiud added a commit to GiudGiud/moose that referenced this issue Mar 3, 2021
GiudGiud added a commit to GiudGiud/moose that referenced this issue Mar 4, 2021
GiudGiud added a commit to GiudGiud/moose that referenced this issue Mar 4, 2021
aeslaughter pushed a commit to aeslaughter/moose that referenced this issue Jun 2, 2021
@lindsayad lindsayad added C: Modules/Navier Stokes Tickets pertaining to the navier_stokes module and removed C: Finite Volume labels Oct 12, 2021
@lindsayad
Copy link
Member

This is not an issue if you use functor material properties. Consequently, I consider this closed by #18395

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: Modules/Navier Stokes Tickets pertaining to the navier_stokes module P: normal A defect affecting operation with a low possibility of significantly affects. T: task An enhancement to the software.
Projects
None yet
Development

No branches or pull requests

3 participants