-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Adding LinearFVEnthalpyFunctorMaterial #29532
base: next
Are you sure you want to change the base?
Conversation
@grmnptr will help with the modification of some of the existing classes in LinearFV to make this PR work properly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this PR will make more sense once the kernels are introduced. It's not obvious how you want to make this work to me yet
modules/navier_stokes/include/functormaterials/LinearFVEnthalpyFunctorMaterial.h
Outdated
Show resolved
Hide resolved
modules/navier_stokes/include/functormaterials/LinearFVEnthalpyFunctorMaterial.h
Outdated
Show resolved
Hide resolved
modules/navier_stokes/include/functormaterials/LinearFVEnthalpyFunctorMaterial.h
Show resolved
Hide resolved
modules/navier_stokes/src/functormaterials/LinearFVEnthalpyFunctorMaterial.C
Outdated
Show resolved
Hide resolved
addFunctorProperty<Real>("h_from_p_T", | ||
[this](const auto & r, const auto & t) -> Real | ||
{ return (*_h_from_p_T_functor)(r, t); }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I m not sure we need this. This new "h_from_p_T" functor could just be replaced by the one the user made ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as in, if the user already has this functor handy, why are we just renaming it? The user can just pass this where "h_from_p_T" will be used
modules/navier_stokes/src/functormaterials/LinearFVEnthalpyFunctorMaterial.C
Outdated
Show resolved
Hide resolved
modules/navier_stokes/src/functormaterials/LinearFVEnthalpyFunctorMaterial.C
Outdated
Show resolved
Hide resolved
modules/navier_stokes/src/functormaterials/LinearFVEnthalpyFunctorMaterial.C
Outdated
Show resolved
Hide resolved
Yeah thanks for the early review! I'll address the comments, but at this point this is at a draft phase, will change a lot. Ramiro is testing it now on a simple channel case. |
9097b5a
to
bdb8226
Compare
Please remember to add the testing that we previously talked about. These tests are:
Thank you! |
dd1f014
to
58b6daf
Compare
Job Precheck, step Clang format on 5085b85 wanted to post the following: Your code requires style changes. A patch was auto generated and copied here
Alternatively, with your repository up to date and in the top level of your repository:
|
Job Documentation, step Docs: sync website on 066b4b4 wanted to post the following: View the site here This comment will be updated on new commits. |
Closes #29531.