Functor material properties #18395

lindsayad · 2021-07-20T19:42:16Z

As per bec76f3 I think I should make a FunctorMaterial or something that doesn't get added in triplicate like our current Materials do

To-do:

Make all functor material property lambdas block restricted
Demonstrate feasibility of caching
Allow functions to be called like our other Functors. This would allow a FunctorMaterial version of, for instance, GenericFunctionMaterial

Closes #16809

moosebuild · 2021-07-20T20:48:25Z

Job Documentation on 0653f11 wanted to post the following:

View the site here

This comment will be updated on new commits.

rwcarlsen · 2021-07-21T15:37:50Z

This has some really good ideas. And a few things that I'm uncomfortable with. I love that we get some of that on-demand property calcs stuff I've been wanting. But I am concerned at the limitations of your Elem * arg to the lambda. What about qps, on nodes, etc.? If we are going to make the effort to start supporting a new type of material property, we should try to make it work for more use cases than just the particular (FV) itch being scratched here. Along similar lines, the lack of support for old/older/stateful props is an additional inconsistency. I think the API approach we take for a new type of material property should be carefully thought out and discussed so we can be sure we know what we want before we commit to bifurcating the APIs.

lindsayad · 2021-07-21T16:00:32Z

But I am concerned at the limitations of your Elem * arg to the lambda

You can add new geometric overloads at any time. For instance today I will add a FaceInfo version because I'm tired of the material property ghosting. Node can be similarly added. qp is a little less obvious for an on-demand calculation. I think a better argument for an on-demand calculation would be something like a std::pair<const Elem *, Point>

lindsayad

@GiudGiud I'm curious what you think about this commit message

GiudGiud · 2021-07-23T22:22:55Z

looking at the commit rather than the message, wrt to INSFV you have not changed the behavior?

not sure how we would evaluate rho * v directly (eg not rho_f v_f) on the face unless we make it the variable instead of v

lindsayad · 2021-07-23T23:11:24Z

wrt to INSFV you have not changed the behavior?

The behavior should be changed. In the interior we would interpolate rhou together before, e.g. rhou_avg = (rho_elem * u_elem + rho_neighbor * u_neighbor) / 2. Now rho and u are interpolated separately and multiplied, e.g. rhou_avg = (rho_elem + rho_neighbor) / 2. * (u_elem + u_neighbor) / 2.

not sure how we would evaluate rho * v directly (eg not rho_f v_f) on the face unless we make it the variable instead of v

Correct. Right now with the functor implementation this would be the only way to "interpolate them together"

GiudGiud · 2021-07-23T23:13:15Z

Oh rho_u used to be the advected quantity, true true

GiudGiud · 2021-07-24T04:02:12Z

btw I was thinking you were going to evaluate rho(T_f, P_f) at the face, not really (rho_elem + rho_neighbor) / 2.
The former requires a full reimplementation of fluid_properties right?

lindsayad · 2021-07-24T04:12:47Z

btw I was thinking you were going to evaluate rho(T_f, P_f) at the face

Yes any call to evaluate rho at a face will result in a query of fluid properties for rho_from_T_p(T_f, p_f). However a user could still ask for rho_elem and then they would get rho_from_T_p(T_elem, p_elem)

lindsayad · 2021-07-24T04:14:38Z

@joe61vette thinks are looking promising. Here are for a couple different Rayleigh numbers

joe61vette · 2021-07-26T13:14:28Z

I would say it looks more than a little promising. I also prefer keeping to the FVMatAdvection scheme whenever possible. It is so easy (using the AD system) to build up compound material properties (eg, alfarhou) that can be combinations of nonlinear variables and fluid properties to be advected.

lindsayad · 2021-07-26T15:46:44Z

It is still easy to build compound properties with the functor property scheme and it halves the necessary data members in residual objects. But if being able to do interpolations together, in the sense that (rhou)_avg != rho_avg * u_avg, is important, then that is a technical issue that would require more thought.

joe61vette · 2021-07-26T15:53:33Z

I didn't make my comment very clear (sorry). For the interpolations, I would want to interpolate between adv_quantity_elem and adv_quantity_neighbor. Where adv_quantity_elem would be a compound material property (eg, alfa * rho * u / eps). Where alfa & u would be non-linear variables (volume fraction and superficial velocity), rho would be a fluid property (fn(P,T)), and eps would be an AuxVar for the porosity. I believe this is what you are proposing.

lindsayad · 2021-07-26T16:00:16Z

So, functionally speaking, you can do this very easily with the current functor implementation, but your result would be this:

interp(alfa * rho * u / eps) = interp(alfa) * interp(rho) * interp(u) / interp(eps)

If this is not desirable to you, then I can think harder about how to do aggregated interpolations with the functor system

joe61vette · 2021-07-26T16:27:26Z

It has been a long time since I have looked into the proper way to go from the PDEs to the finite volume formulation. But I believe that interpolating each quantity separately could lead to problems. For example, let's consider a compound material property say "momentum_x", for our conditions momentum_x_elem < momentum_x_neighbor. If we interpolate each quantity separately, I don't believe there is a guarantee that:

momentum_x_elem < momentum_x_face < momentum_x_neighbor

I admit that I am not sure about this. Perhaps @GiudGiud can weigh in as well. At any rate, I thought this was the direction you were headed based on the above comment:

"The behavior should be changed. In the interior we would interpolate rhou together before, e.g. rhou_avg = (rho_elem * u_elem + rho_neighbor * u_neighbor) / 2. Now rho and u are interpolated separately and multiplied, e.g. rhou_avg = (rho_elem + rho_neighbor) / 2. * (u_elem + u_neighbor) / 2."

GiudGiud · 2021-07-26T16:53:22Z

~~I would rather we do not lose momentum and energy conservation over this.~~
There could be an option to interpolate the combined property over evaluating each of its components. It's probably much cheaper to do too (but as often, might not show up in profiling)

lindsayad · 2021-07-26T17:36:35Z

It's probably much cheaper to do to

How is that? In my head there's the same number of operations

lindsayad · 2021-07-26T17:42:58Z

My hatred of performing material property evaluation "at neighbor" stems from the fact that it requires material property ghosting which has raised a whole slate of issues for us both in terms of code maintenance and in terms of user input file construction. If we can train developers to think about what they do at domain boundaries when using FVFluxKernels then some-day we could remove the need to do material property ghosting

lindsayad · 2021-07-26T17:44:46Z

I just don't like ghosting period. If we are at a domain boundary, we should evaluate a quantity based on Dirichlet information or with getExtrapolatedBoundaryFaceValue

Also clean-up documentation to reflect that fully compressible only works with a momentum parameter and incompressible only works with a density parameter and superficial velocity variable set

lindsayad · 2021-10-05T16:23:38Z

Ok I think I hit all the new requested tests. @GiudGiud I edited #18395 (comment) to reflect what I added tests for and where I removed code. I may add another test for when a property is initially retrieved as an ordinary functor property and is later declared as another property type

Removing standalone NSFV and PNSFV for gravity and friction

modules/navier_stokes/doc/content/bib/navier_stokes.bib

GiudGiud · 2021-10-05T17:37:50Z

cant resolve all the conversations from the files tab btw

@loganharbour this will need to break CI with Pronghorn
we ll make an announcement

lindsayad · 2021-10-05T17:42:50Z

Hold on, in the process of increasing coverage, I found a bug 😆 Boy, who would ever complain about a code coverage target! They must be really dumb!

moosebuild · 2021-10-05T18:20:34Z

Job Doxygen on 0653f11 wanted to post the following:

View the modules doxygen here.

This comment will be updated on new commits.

This revealed what should not have been a surprise: deleting an object makes any reference access to that object bad access. So now I use the letter and envelope idiom to allow changing out of the underlying functor material property type

lindsayad · 2021-10-05T21:26:26Z

It seems like @GiudGiud is at the point of acceptance on this. @rwcarlsen @friedmud ?

loganharbour · 2021-10-05T23:42:57Z

Hold on, in the process of increasing coverage, I found a bug 😆 Boy, who would ever complain about a code coverage target! They must be really dumb!

🍻

friedmud · 2021-10-06T01:35:47Z

At this point - any input of mine would not be useful. We can continue to evolve this system as we go.

Good work all!

I approved the changes...

lindsayad · 2021-10-06T15:47:19Z

@rwcarlsen ?

lindsayad · 2021-10-11T18:15:20Z

Ok I'm going to give another day on this, and then I'm going to dismiss @rwcarlsen 's review unless he indicates otherwise

@rwcarlsen

@rwcarlsen currently unresponsive

Fixup #18395

lindsayad requested a review from rwcarlsen as a code owner July 20, 2021 19:42

moosebuild added the PR: Failed but allowed label Jul 20, 2021

lindsayad requested a review from GiudGiud as a code owner July 20, 2021 23:31

moosebuild removed the PR: Failed but allowed label Jul 20, 2021

lindsayad force-pushed the functor-material branch from 22e5abd to ccf08d1 Compare July 21, 2021 00:28

lindsayad changed the title ~~Functor material properties~~ [WIP] Functor material properties Jul 21, 2021

lindsayad marked this pull request as draft July 21, 2021 20:46

lindsayad force-pushed the functor-material branch 3 times, most recently from 0b85090 to 6822778 Compare July 23, 2021 21:44

lindsayad commented Jul 23, 2021

View reviewed changes

lindsayad force-pushed the functor-material branch from 3b66934 to 42771c5 Compare July 24, 2021 03:45

Move PINSFVFunctorBC to test objects

c7f3cab

lindsayad force-pushed the functor-material branch from f3287fa to c7f3cab Compare October 4, 2021 21:57

lindsayad added 2 commits October 4, 2021 15:40

Test advected mat prop in FE contexts in InternalVolumetricFlowRatePP

9584906

Delete momentum_name code-path from incompressible friction object

2566c29

Also clean-up documentation to reflect that fully compressible only works with a momentum parameter and incompressible only works with a density parameter and superficial velocity variable set

Further stratification of (P)INSFV and (P)CNSFV

e5c5bfe

Removing standalone NSFV and PNSFV for gravity and friction

lindsayad force-pushed the functor-material branch from db87895 to e5c5bfe Compare October 5, 2021 16:43

GiudGiud reviewed Oct 5, 2021

View reviewed changes

modules/navier_stokes/doc/content/bib/navier_stokes.bib Show resolved Hide resolved

GiudGiud previously approved these changes Oct 5, 2021

View reviewed changes

lindsayad marked this pull request as draft October 5, 2021 17:42

lindsayad dismissed GiudGiud’s stale review via 0653f11 October 5, 2021 20:44

lindsayad marked this pull request as ready for review October 5, 2021 21:25

friedmud approved these changes Oct 6, 2021

View reviewed changes

lindsayad merged commit ddd263b into idaholab:next Oct 12, 2021

lindsayad deleted the functor-material branch October 12, 2021 20:47

GiudGiud mentioned this pull request Oct 12, 2021

Adapt all Pronghorn inputs to functor fluid properties idaholab/virtual_test_bed#28

Closed

lindsayad added a commit to lindsayad/moose that referenced this pull request Oct 13, 2021

Fixup idaholab#18395

d3a6b3e

moosebuild added a commit that referenced this pull request Oct 13, 2021

Merge pull request #19085 from lindsayad/fixups-18395

429cf7e

Fixup #18395

This was referenced Nov 24, 2021

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

Closed

FV: materials initialized to FE quadrature in FV contexts #15194

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Functor material properties #18395

Functor material properties #18395

lindsayad commented Jul 20, 2021 •

edited

moosebuild commented Jul 20, 2021 •

edited

rwcarlsen commented Jul 21, 2021

lindsayad commented Jul 21, 2021

lindsayad left a comment •

edited

GiudGiud commented Jul 23, 2021 •

edited

lindsayad commented Jul 23, 2021 •

edited

GiudGiud commented Jul 23, 2021 •

edited

GiudGiud commented Jul 24, 2021

lindsayad commented Jul 24, 2021

lindsayad commented Jul 24, 2021

joe61vette commented Jul 26, 2021 •

edited

lindsayad commented Jul 26, 2021

joe61vette commented Jul 26, 2021

lindsayad commented Jul 26, 2021

joe61vette commented Jul 26, 2021

GiudGiud commented Jul 26, 2021 •

edited

lindsayad commented Jul 26, 2021

lindsayad commented Jul 26, 2021

lindsayad commented Jul 26, 2021 •

edited

lindsayad commented Oct 5, 2021 •

edited

GiudGiud commented Oct 5, 2021 •

edited

lindsayad commented Oct 5, 2021

moosebuild commented Oct 5, 2021 •

edited

lindsayad commented Oct 5, 2021

loganharbour commented Oct 5, 2021

friedmud commented Oct 6, 2021

lindsayad commented Oct 6, 2021

lindsayad commented Oct 11, 2021

Functor material properties #18395

Functor material properties #18395

Conversation

lindsayad commented Jul 20, 2021 • edited

moosebuild commented Jul 20, 2021 • edited

rwcarlsen commented Jul 21, 2021

lindsayad commented Jul 21, 2021

lindsayad left a comment • edited

Choose a reason for hiding this comment

GiudGiud commented Jul 23, 2021 • edited

lindsayad commented Jul 23, 2021 • edited

GiudGiud commented Jul 23, 2021 • edited

GiudGiud commented Jul 24, 2021

lindsayad commented Jul 24, 2021

lindsayad commented Jul 24, 2021

joe61vette commented Jul 26, 2021 • edited

lindsayad commented Jul 26, 2021

joe61vette commented Jul 26, 2021

lindsayad commented Jul 26, 2021

joe61vette commented Jul 26, 2021

GiudGiud commented Jul 26, 2021 • edited

lindsayad commented Jul 26, 2021

lindsayad commented Jul 26, 2021

lindsayad commented Jul 26, 2021 • edited

lindsayad commented Oct 5, 2021 • edited

GiudGiud commented Oct 5, 2021 • edited

lindsayad commented Oct 5, 2021

moosebuild commented Oct 5, 2021 • edited

lindsayad commented Oct 5, 2021

loganharbour commented Oct 5, 2021

friedmud commented Oct 6, 2021

lindsayad commented Oct 6, 2021

lindsayad commented Oct 11, 2021

lindsayad commented Jul 20, 2021 •

edited

moosebuild commented Jul 20, 2021 •

edited

lindsayad left a comment •

edited

GiudGiud commented Jul 23, 2021 •

edited

lindsayad commented Jul 23, 2021 •

edited

GiudGiud commented Jul 23, 2021 •

edited

joe61vette commented Jul 26, 2021 •

edited

GiudGiud commented Jul 26, 2021 •

edited

lindsayad commented Jul 26, 2021 •

edited

lindsayad commented Oct 5, 2021 •

edited

GiudGiud commented Oct 5, 2021 •

edited

moosebuild commented Oct 5, 2021 •

edited