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

Linear FV Fluid Energy equation #28952

Open
wants to merge 8 commits into
base: next
Choose a base branch
from

Conversation

freiler
Copy link
Contributor

@freiler freiler commented Oct 28, 2024

Closes #28951

@@ -21,11 +21,14 @@ LinearFVSource::validParams()
"Represents the matrix and right hand side contributions of a "
"solution-independent source term in a partial differential equation.");
params.addParam<MooseFunctorName>("source_density", 1.0, "The source density.");
params.addParam<Real>("scaling_factor", 1.0, "Coefficient to multiply by the body force term");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
params.addParam<Real>("scaling_factor", 1.0, "Coefficient to multiply by the body force term");
params.addParam<Real>("scaling_factor", 1.0, "Coefficient to multiply the body force term with");

@@ -33,4 +33,7 @@ class LinearFVSource : public LinearFVElementalKernel
protected:
/// The functor for the source density
const Moose::Functor<Real> & _source_density;

/// Scale factor
const Real & _scale;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const Real & _scale;
const Real _scale;

for efficiency. If you use a reference, every time the kernel acts it needs to go retrieve the value from the parameters' memory

the only reason to make it a reference would be to make it controllable

* Kernel that adds the component of the pressure gradient in the momentum
* equations to the right hand side.
*/
class LinearFVMomentumBoussinesq : public LinearFVElementalKernel
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok let's wait for the design discussion before including this?


protected:
/// Fluid Temperature
MooseLinearVariableFV<Real> & getTemperatureVariable(const std::string & vname);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const ?
same for the attribute

const RealVectorValue _gravity;
/// The thermal expansion coefficient
const Moose::Functor<Real> & _alpha;
/// Reference temperature at which the value of _rho was measured
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Reference temperature at which the value of _rho was measured
/// Reference temperature at which the reference value of the density was measured

rho_0 does not show up in your implementation so it's not as clear what you mean

params.suppressParameter<unsigned int>("energy_l_max_its");
// params.suppressParameter<Real>("energy_l_tol");
// params.suppressParameter<Real>("energy_l_abs_tol");
// params.suppressParameter<unsigned int>("energy_l_max_its");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

InputParameters params = LinearFVFluxKernel::validParams();
params.addClassDescription("Represents the matrix and right hand side contributions of an "
"advection term for the energy e.g. h=cp*T. A user may still "
"override what quantity is advected, but the default is cp*T.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"override what quantity is advected, but the default is cp*T.");
"override what quantity is advected, but the default is the specific enthalpy");

what specific enthalpy means is defined elsewhere than the kernel

registerMooseObject("MooseApp", LinearFVEnergyAdvection);

InputParameters
LinearFVEnergyAdvection::validParams()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should consider with @grmnptr whether we want a kernel for energy when it's the same as scalar and we could just call it scalarAdvection

params.addParam<MooseFunctorName>("alpha_name",
NS::alpha_boussinesq,
"The name of the thermal expansion coefficient"
"this is of the form rho = rho*(1-alpha (T-T_ref))");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"this is of the form rho = rho*(1-alpha (T-T_ref))");
"The density is then computed as rho*(1-alpha
*(T-T_ref))");

@GiudGiud GiudGiud self-assigned this Oct 30, 2024
@moosebuild
Copy link
Contributor

Job Precheck, step Clang format on 988efef 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/28952/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 b8db753c73821a52d1624333d28c3c8bf9b2a008

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 fluid energy equation solving capabilities for FV linear SIMPLE solver
4 participants