Linear kepsilon #29847

tanoret · 2025-02-09T19:09:54Z

tanoret · 2025-02-09T19:12:18Z

@grmnptr @GiudGiud - could you please do the code review?
@freiler - could you please do the technical review? I normally run the tests in the flow test bed and was getting the same results than the k-epsilon with the nonlinear assembly

Thanks!!

… taken into account for linear systems.

moosebuild · 2025-02-10T20:31:11Z

Job Precheck, step Clang format on b590f08 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/29847/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 5fce0ac136837e85742ff9c1539c66314ba1f773

lindsayad

Need some communication about why we are changing the framework objects. Seems to be to allow BCs on aux variables?

lindsayad · 2025-02-10T22:30:30Z

framework/src/linearfvbcs/LinearFVBoundaryCondition.C

@@ -49,7 +49,7 @@ LinearFVBoundaryCondition::LinearFVBoundaryCondition(const InputParameters & par
    MooseVariableInterface(this,
                           false,
                           "variable",
-                           Moose::VarKindType::VAR_SOLVER,
+                           Moose::VarKindType::VAR_ANY,


why this change?

this is allowing BCs on aux variables. Peter will do a separate PR

Well, this is in this PR:
#29532

lindsayad · 2025-02-10T22:31:27Z

framework/src/problems/FEProblemBase.C

-  try
-  {
-    computeSystems(EXEC_NONLINEAR);
-  }
-  catch (MooseException & e)
-  {
-    _console << "\nA MooseException was raised during Auxiliary variable computation.\n"
-             << "The next solve will fail, the timestep will be reduced, and we will try again.\n"
-             << std::endl;


we definitely want try-catch

Yeah, try-catch should be good, we do this not to compute the aux system every time.

moosebuild · 2025-02-10T23:48:47Z

Job Documentation, step Docs: sync website on 57a1783 wanted to post the following:

View the site here

This comment will be updated on new commits.

GiudGiud · 2025-02-11T02:11:32Z

framework/src/systems/LinearSystem.C

-    _rhs_non_time_tag(-1),
+    // We add this vector tag so that objects acting on the aux system inheriting
+    // from the tagging interface can still be used without any nonlinear systems
+    _rhs_non_time_tag(_fe_problem.addVectorTag("NONTIME")),


there's a cost to this I think ? Like in memory. Griffin people wont be happy

I don't think they use linear systems in their simulations. Also, this is just the tag that can be used, it does not mean there is a vector underneath.

GiudGiud · 2025-02-11T02:11:59Z

framework/src/systems/LinearSystem.C

@@ -91,6 +94,10 @@ LinearSystem::LinearSystem(FEProblemBase & fe_problem, const std::string & name)
  // We create a tag for the right hand side, the vector is already in the libmesh system
  _rhs_tag = _fe_problem.addVectorTag("RHS");

+  // We add other wector tags so that objects acting on the aux system inheriting
+  // from the tagging interface can still be used without any nonlinear systems
+  _rhs_non_time_tag = _fe_problem.addVectorTag("NONTIME");


are we doing this operation twice? it s right above

Yeah I think one of these should be enough, probably the one in the initialized list..

GiudGiud · 2025-02-11T02:13:22Z

framework/src/systems/LinearSystem.C

+  for (THREAD_ID tid = 0; tid < libMesh::n_threads(); tid++)
+  {
+    std::vector<LinearFVElementalKernel *> fv_elemental_kernels;
+    _fe_problem.theWarehouse()
+        .query()
+        .template condition<AttribSystem>("LinearFVElementalKernel")
+        .template condition<AttribThread>(tid)
+        .queryInto(fv_elemental_kernels);
+
+    for (auto * fv_kernel : fv_elemental_kernels)
+      fv_kernel->initialSetup();
+
+    std::vector<LinearFVFluxKernel *> fv_flux_kernels;
+    _fe_problem.theWarehouse()
+        .query()
+        .template condition<AttribSystem>("LinearFVFluxKernel")
+        .template condition<AttribThread>(tid)
+        .queryInto(fv_flux_kernels);
+
+    for (auto * fv_kernel : fv_flux_kernels)
+      fv_kernel->initialSetup();
+  }


why not use LinearFVKernel for the query instead?
initialSetup exists for this class, since it comes from the "LinearSystemContributionObject " base

GiudGiud · 2025-02-11T02:14:04Z

modules/navier_stokes/doc/content/source/linearfvkernels/LinearFVTKEDSourceSink.md

@@ -0,0 +1,113 @@
+# LinearFVTKEDSourceSink
+
+This kernel implements a version of [`INSFVTKEDSourceSink`](INSFVTKEDSourceSink.md) for a linear kernel.


Suggested change

This kernel implements a version of [`INSFVTKEDSourceSink`](INSFVTKEDSourceSink.md) for a linear kernel.

This kernel implements a version of [`INSFVTKEDSourceSink`](INSFVTKEDSourceSink.md) for the linear finite volume discretization of the $k-\epsilon$ turbulence equations.

same for all other doco files

GiudGiud · 2025-02-11T02:14:42Z

modules/navier_stokes/doc/content/source/linearfvkernels/LinearFVTKEDSourceSink.md

+# LinearFVTKEDSourceSink
+
+This kernel implements a version of [`INSFVTKEDSourceSink`](INSFVTKEDSourceSink.md) for a linear kernel.
+The documentation is repeated down here for completeness.


use a file include instead? Create a new folder above source/ to host incomplete doco files that get included.

This will reduce maintenance cost

GiudGiud · 2025-02-11T02:56:11Z

modules/navier_stokes/src/linearfvkernels/LinearFVTKEDSourceSink.C

+      // _console << "position: " << _current_elem_info->elem()->vertex_average() << std::endl;
+      // _console << "y_plus: " << y_plus << std::endl;
+      // _console << "TKE: " << TKE << std::endl;
+      // _console << "distance_vec[i]: " << distance_vec[i] << std::endl;
+      // _console << "destruction: " << destruction << std::endl;


Suggested change

// _console << "position: " << _current_elem_info->elem()->vertex_average() << std::endl;

// _console << "y_plus: " << y_plus << std::endl;

// _console << "TKE: " << TKE << std::endl;

// _console << "distance_vec[i]: " << distance_vec[i] << std::endl;

// _console << "destruction: " << destruction << std::endl;

GiudGiud · 2025-02-11T02:56:26Z

modules/navier_stokes/src/linearfvkernels/LinearFVTKEDSourceSink.C

+  // Do nothing
+  // return 0.0;


Suggested change

// Do nothing

// return 0.0;

GiudGiud · 2025-02-11T02:57:12Z

modules/navier_stokes/src/linearfvkernels/LinearFVTKESourceSink.C

+  params.addParam<MooseEnum>("wall_treatment",
+                             wall_treatment,
+                             "The method used for computing the wall functions "
+                             "'eq_newton', 'eq_incremental', 'eq_linearized', 'neq'");


it seems only one method is implemented below

are the other s just the same for this kernel?

GiudGiud · 2025-02-11T03:01:14Z

modules/navier_stokes/src/linearfvkernels/LinearFVTKESourceSink.C

+    // Assembly variables
+    Real destruction = 0.0;
+    Real tot_weight = 0.0;
+    std::vector<Real> y_plus_vec, velocity_grad_norm_vec;


you should reserve a size for these two vectors for performance purposes rather than allocating dynamically later on

GiudGiud · 2025-02-11T03:04:53Z

modules/navier_stokes/src/linearfvkernels/LinearFVTurbulentLimitedAdvection.C

+
+  // If we are on an internal face, we populate the four entries in the system matrix
+  // which touch the face
+  if (_current_face_type == FaceInfo::VarFaceNeighbors::BOTH)


let's discuss this tomorrow. this is a ton code duplication for something that block restriction should achieve naturally?

if you dont think so, then let's create "addMatrixContrib/ETCInternal in the advection class.

then in the base class (regular advection) you do: addMatrixContrib() { addMatrixContribInternal() }

and in the derived class, you do

if (on boundary) code code else addMatrixContribInternal();

grmnptr

Very nice, just a few suggestions!

grmnptr · 2025-02-12T22:36:25Z

framework/src/problems/FEProblemBase.C

-  try
-  {
-    computeSystems(EXEC_NONLINEAR);
-  }
-  catch (MooseException & e)
-  {
-    _console << "\nA MooseException was raised during Auxiliary variable computation.\n"
-             << "The next solve will fail, the timestep will be reduced, and we will try again.\n"
-             << std::endl;


Yeah, try-catch should be good, we do this not to compute the aux system every time.

grmnptr · 2025-02-12T22:39:15Z

framework/src/systems/LinearSystem.C

-    _rhs_non_time_tag(-1),
+    // We add this vector tag so that objects acting on the aux system inheriting
+    // from the tagging interface can still be used without any nonlinear systems
+    _rhs_non_time_tag(_fe_problem.addVectorTag("NONTIME")),


I don't think they use linear systems in their simulations. Also, this is just the tag that can be used, it does not mean there is a vector underneath.

grmnptr · 2025-02-12T22:40:01Z

framework/src/systems/LinearSystem.C

@@ -91,6 +94,10 @@ LinearSystem::LinearSystem(FEProblemBase & fe_problem, const std::string & name)
  // We create a tag for the right hand side, the vector is already in the libmesh system
  _rhs_tag = _fe_problem.addVectorTag("RHS");

+  // We add other wector tags so that objects acting on the aux system inheriting
+  // from the tagging interface can still be used without any nonlinear systems
+  _rhs_non_time_tag = _fe_problem.addVectorTag("NONTIME");


Yeah I think one of these should be enough, probably the one in the initialized list..

grmnptr · 2025-02-12T22:40:31Z

framework/src/systems/LinearSystem.C

@@ -107,6 +114,32 @@ LinearSystem::initialSetup()
      mooseError("You are trying to add a nonlinear variable to a linear system! The variable "
                 "which is assigned to the wrong system: ",
                 name);
+
+  // Calling initial setpup for the linear kernels


Suggested change

// Calling initial setpup for the linear kernels

// Calling initial setup for the linear kernels

grmnptr · 2025-02-12T23:59:09Z

modules/navier_stokes/doc/content/source/linearfvkernels/LinearFVTurbulentLimitedAdvection.md

+# LinearFVTurbulentLimitedAdvection
+
+This object adds a $\nabla \cdot \vec u \phi$ term for an arbitrary scalar field
+$\phi$, where $\phi$ corresponds to the nonlinear variable that this kernel acts


Suggested change

$\phi$, where $\phi$ corresponds to the nonlinear variable that this kernel acts

$\phi$, where $\phi$ corresponds to the variable that this kernel acts

grmnptr · 2025-02-21T00:22:59Z

...ests/finite_volume/ins/turbulence/lid-driven/linear-segregated/lid-driven-turb-non-eq-wall.i

+    type = LinearFVDiffusion
+    variable = vel_x
+    diffusion_coeff = ${mu}
+    use_nonorthogonal_correction = true


Suggested change

use_nonorthogonal_correction = true

use_nonorthogonal_correction = false

grmnptr · 2025-02-21T00:23:06Z

...ests/finite_volume/ins/turbulence/lid-driven/linear-segregated/lid-driven-turb-non-eq-wall.i

+    v = vel_y
+    momentum_component = 'x'
+    rhie_chow_user_object = 'rc'
+    use_nonorthogonal_correction = true


Suggested change

use_nonorthogonal_correction = true

use_nonorthogonal_correction = false

grmnptr · 2025-02-21T00:24:22Z

...t/tests/finite_volume/ins/turbulence/lid-driven/linear-segregated/lid-driven-turb-std-wall.i

@@ -0,0 +1,358 @@
+### Thermophysical Properties ###


We only have a few options different in this, we should either do the inlude syntax and split the input file or just the cli_args.

grmnptr · 2025-02-21T00:24:58Z

...les/navier_stokes/test/tests/finite_volume/ins/turbulence/lid-driven/linear-segregated/tests

+  design = 'INSFVTurbulentAdvection.md INSFVTurbulentDiffusion.md INSFVTKESourceSink.md INSFVTKEDSourceSink.md INSFVTurbulentViscosityWallFunction.md kEpsilonViscosityAux.md'
+  issues = '#9007'
+  [lid_driven_turb_std_wall_linear]
+    requirement = 'The system shall be able to run fluid flow with k-epsilon turbulence model in enclosures with standard wall functions, '


Suggested change

requirement = 'The system shall be able to run fluid flow with k-epsilon turbulence model in enclosures with standard wall functions, '

requirement = 'The system shall be able to run fluid flow with k-epsilon turbulence model in enclosures with standard wall functions and the linear finite volume discretization, '

grmnptr · 2025-02-21T00:25:17Z

...les/navier_stokes/test/tests/finite_volume/ins/turbulence/lid-driven/linear-segregated/tests

+    []
+  []
+  [lid_driven_turb_non_eq_wall_linear]
+    requirement = 'The system shall be able to run fluid flow with k-epsilon turbulence model in enclosures with non-equilibrium wall functions, '


Suggested change

requirement = 'The system shall be able to run fluid flow with k-epsilon turbulence model in enclosures with non-equilibrium wall functions, '

requirement = 'The system shall be able to run fluid flow with k-epsilon turbulence model in enclosures with non-equilibrium wall functions and the linear finite volume discretization, '

tanoret requested review from lindsayad, GiudGiud, grmnptr and roystgnr as code owners February 9, 2025 19:09

tanoret force-pushed the linear_kepsilon branch from 226a995 to 81a7048 Compare February 9, 2025 20:17

tanoret and others added 12 commits February 10, 2025 13:26

initial includes and src implementation for lienar kEpsilon Refs #0

348a2f7

adding channel test Refs #000

34e249b

Register nontime tag just in case, make sure automatic scaling is not…

75e7fd7

… taken into account for linear systems.

Correct deviatoric terms in the momentum kernel.

d749821

Add gradient refreshers to SNL.

0957b42

Add tests to verify that we converge to analytic solutions.

78c5d8b

Move solution update to the momentum system loop.

32847bd

Register nontime tag just in case, make sure automatic scaling is not…

27edf62

… taken into account for linear systems.

first working linear k epsilon model Refs idaholab#29846

2a1a97d

adding channel bfs lid-driven tests Refs idaholab#29846

b91b6e9

adding transients tests Refs idaholab#29846

2671c32

adding documentation Refs idaholab#29846

b10d203

tanoret force-pushed the linear_kepsilon branch from 8c858aa to b590f08 Compare February 10, 2025 20:26

correcting style Refs idaholab#29846

57a1783

tanoret force-pushed the linear_kepsilon branch from b590f08 to 57a1783 Compare February 10, 2025 21:26

lindsayad reviewed Feb 10, 2025

View reviewed changes

GiudGiud requested changes Feb 11, 2025

View reviewed changes

GiudGiud assigned grmnptr Feb 13, 2025

loganharbour force-pushed the next branch from cfc8970 to 86b9411 Compare February 19, 2025 14:51

grmnptr requested changes Feb 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linear kepsilon #29847

Linear kepsilon #29847

tanoret commented Feb 9, 2025

tanoret commented Feb 9, 2025

moosebuild commented Feb 10, 2025

lindsayad left a comment

lindsayad Feb 10, 2025

GiudGiud Feb 11, 2025

grmnptr Feb 12, 2025

lindsayad Feb 10, 2025

grmnptr Feb 12, 2025

moosebuild commented Feb 10, 2025

GiudGiud Feb 11, 2025

grmnptr Feb 12, 2025

GiudGiud Feb 11, 2025

grmnptr Feb 12, 2025

GiudGiud Feb 11, 2025

GiudGiud Feb 11, 2025

GiudGiud Feb 11, 2025

GiudGiud Feb 11, 2025

GiudGiud Feb 11, 2025

GiudGiud Feb 11, 2025

GiudGiud Feb 11, 2025

GiudGiud Feb 11, 2025

GiudGiud Feb 11, 2025 •

edited

Loading

grmnptr left a comment •

edited

Loading

grmnptr Feb 12, 2025

grmnptr Feb 12, 2025

grmnptr Feb 12, 2025

grmnptr Feb 12, 2025

grmnptr Feb 12, 2025

grmnptr Feb 21, 2025

grmnptr Feb 21, 2025

grmnptr Feb 21, 2025

grmnptr Feb 21, 2025

grmnptr Feb 21, 2025

		@@ -0,0 +1,113 @@
		# LinearFVTKEDSourceSink

		This kernel implements a version of [`INSFVTKEDSourceSink`](INSFVTKEDSourceSink.md) for a linear kernel.

	// Calling initial setpup for the linear kernels
	// Calling initial setup for the linear kernels

	$\phi$, where $\phi$ corresponds to the nonlinear variable that this kernel acts
	$\phi$, where $\phi$ corresponds to the variable that this kernel acts

	use_nonorthogonal_correction = true
	use_nonorthogonal_correction = false

	requirement = 'The system shall be able to run fluid flow with k-epsilon turbulence model in enclosures with standard wall functions, '
	requirement = 'The system shall be able to run fluid flow with k-epsilon turbulence model in enclosures with standard wall functions and the linear finite volume discretization, '

Linear kepsilon #29847

Are you sure you want to change the base?

Linear kepsilon #29847

Conversation

tanoret commented Feb 9, 2025

tanoret commented Feb 9, 2025

moosebuild commented Feb 10, 2025

lindsayad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moosebuild commented Feb 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GiudGiud Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

grmnptr left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GiudGiud Feb 11, 2025 •

edited

Loading

grmnptr left a comment •

edited

Loading