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

Adding LinearFVEnthalpyFunctorMaterial #29532

Draft
wants to merge 11 commits into
base: next
Choose a base branch
from

Conversation

freiler
Copy link
Contributor

@freiler freiler commented Dec 12, 2024

Closes #29531.

@freiler
Copy link
Contributor Author

freiler commented Dec 12, 2024

@grmnptr will help with the modification of some of the existing classes in LinearFV to make this PR work properly.
Thanks!

@grmnptr grmnptr changed the title Adding LinearFVEnthalpyFunctorMaterial #29531 Adding LinearFVEnthalpyFunctorMaterial Dec 12, 2024
@grmnptr grmnptr self-assigned this Dec 12, 2024
@grmnptr grmnptr marked this pull request as draft December 12, 2024 16:33
Copy link
Contributor

@GiudGiud GiudGiud left a 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

Comment on lines 70 to 72
addFunctorProperty<Real>("h_from_p_T",
[this](const auto & r, const auto & t) -> Real
{ return (*_h_from_p_T_functor)(r, t); });
Copy link
Contributor

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 ?

Copy link
Contributor

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

@grmnptr
Copy link
Contributor

grmnptr commented Dec 12, 2024

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.

@tanoret
Copy link
Contributor

tanoret commented Dec 18, 2024

Please remember to add the testing that we previously talked about. These tests are:

  • 1D problem with cp=a+bT and verification against analytical solution
  • 2D problem with wall heating and verifying enthalpy balances (h_dot_out = h_dot_in + q''_wall * A_wall)
  • Equivalent problem in 3D and test against the previous 2D problem
  • Body with variable cp and rho cooling at a fixed heat rate and verify against the analytical solution

Thank you!

@moosebuild
Copy link
Contributor

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
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/docs/PRs/29532/clang_format/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format d9397150e6e61be3c4dd09f2fec52867a7fdd489

@moosebuild
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding Enthalpy Equation for LinearFV
5 participants