Adding SIMPLEC #29865

tanoret · 2025-02-12T07:23:51Z

tanoret · 2025-02-12T07:25:35Z

@grmnptr added a couple of tests showing that we can converge fine with pressure equation relaxation equal 1.0. Tried to add minimal changes to your architecture

@freiler don't think a technical review is needed at this point as this is mostly a step towards VOF. However, I leave this to your technical reviewer judgement. Thanks!

moosebuild · 2025-02-12T07:29:10Z

Job Precheck, step Clang format on a594e5e 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/29865/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 cd66266a7dec6c2ec22f5902dba3e2ab6eeb8b19

lindsayad · 2025-02-12T16:56:02Z

modules/navier_stokes/src/userobjects/RhieChowMassFlux.C

+  // Pressure projection
+  params.addParam<MooseEnum>(
+      "pressure_projection_method",
+      MooseEnum("SIMPLE SIMPLEC", "SIMPLE"),


Given the benefits espoused on the associated issue, why not make SIMPLEC the default?

lindsayad · 2025-02-12T16:56:41Z

modules/navier_stokes/src/userobjects/RhieChowMassFlux.C

+    if (_pressure_projection_method == "SIMPLEC")
+    {
+
+      // Consistent Corrections to Simple


Suggested change

// Consistent Corrections to Simple

// Consistent Corrections to SIMPLE

lindsayad · 2025-02-12T16:57:21Z

modules/navier_stokes/src/userobjects/RhieChowMassFlux.C

+        sum_vector.zero();
+
+        // Local row size
+        const unsigned int local_size = mmat->row_stop() - mmat->row_start();


Suggested change

const unsigned int local_size = mmat->row_stop() - mmat->row_start();

const auto local_size = mmat->local_m();

lindsayad · 2025-02-12T16:59:38Z

modules/navier_stokes/src/userobjects/RhieChowMassFlux.C

+        for (const auto row_i : make_range(local_size))
+        {
+          // Get all non-zero components of the row of the matrix
+          const unsigned int global_index = mmat->row_start() + row_i;


Suggested change

const unsigned int global_index = mmat->row_start() + row_i;

const auto global_index = mmat->row_start() + row_i;

lindsayad · 2025-02-12T17:00:56Z

modules/navier_stokes/src/userobjects/RhieChowMassFlux.C

+
+      // Create vector with new inverse projection matrix
+      auto Ainv_full = current_local_solution.zero_clone();
+      PetscVector<Number> * Ainv_full_vec = dynamic_cast<PetscVector<Number> *>(Ainv_full.get());


Suggested change

PetscVector<Number> * Ainv_full_vec = dynamic_cast<PetscVector<Number> *>(Ainv_full.get());

PetscVector<Number> * Ainv_full_vec = cast_ptr<PetscVector<Number> *>(Ainv_full.get());

We actually don't need this vector. The NumericVector is enough, it has the functions you need. The cost of the virtual call for this will be negligible compared to the operations within this function.

lindsayad · 2025-02-12T17:03:15Z

...es/navier_stokes/test/tests/finite_volume/ins/channel-flow/SIMPLEC/2d/2d-velocity-pressure.i

+[]
+
+[Executioner]
+  type = SIMPLE


It would be kind of nice to not have a discrepancy between the executioner name and pressure projection method, but maybe experienced CFD users wouldn't find this confusing?

Maybe this discrepancy wouldn't appear when using Physics? Then it would be much less a concern to me

I like this design. The projection type only comes in the picture through the fields that are computed in the RC object.

I agree the code design is nice

moosebuild · 2025-02-12T18:38:46Z

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

View the site here

This comment will be updated on new commits.

moosebuild · 2025-02-13T02:47:21Z

Job Coverage, step Generate coverage on fe10d3d wanted to post the following:

Framework coverage

	cd6626	#29865 fe10d3
	Total	Total	+/-	New
Rate	85.29%	85.29%	-0.00%	-
Hits	108457	108456	-1	0
Misses	18702	18703	+1	0

Diff coverage report

Full coverage report

Modules coverage

Navier stokes

	cd6626	#29865 fe10d3
	Total	Total	+/-	New
Rate	84.83%	84.85%	+0.02%	96.67%
Hits	17777	17805	+28	29
Misses	3178	3179	+1	1

Diff coverage report

Full coverage report

Full coverage reports

Reports

This comment will be updated on new commits.

grmnptr

Elegant solution! For the records, could you provide some numbers here on speedup on cases within the flow test bed?

grmnptr · 2025-02-24T20:54:33Z

modules/navier_stokes/doc/content/source/userobjects/RhieChowMassFlux.md

@@ -22,6 +22,15 @@ Besides these capabilities, this user object is also responsible for reconstruct
 cell velocities at the end of the pressure corrector step.
 For more information on these fields and processes, we suggest visiting [SIMPLE.md].

+The object enables the computation of the standard (SIMPLE) or consistent (SIMPLEC)


grmnptr · 2025-02-24T21:06:43Z

modules/navier_stokes/src/userobjects/RhieChowMassFlux.C

+          std::vector<Real> values;
+          mmat->get_row(global_index, indices, values);
+
+          // Use std::accumulate with a lambda to sum row elements (no absolute values)


Suggested change

// Use std::accumulate with a lambda to sum row elements (no absolute values)

// Sum row elements (no absolute values)

grmnptr · 2025-02-24T21:09:17Z

modules/navier_stokes/src/userobjects/RhieChowMassFlux.C

+        _console << "Performing SIMPLEC projection." << std::endl;
+
+      // Lambda function to calculate the sum of diagonal and neighbor coefficients
+      auto get_row_sum_excluding_diagonal = [mmat](NumericVector<Number> & sum_vector)


Suggested change

auto get_row_sum_excluding_diagonal = [mmat](NumericVector<Number> & sum_vector)

auto get_row_sum = [mmat](NumericVector<Number> & sum_vector)

Considering that the diagonal is actually in here

grmnptr · 2025-02-24T21:25:56Z

modules/navier_stokes/src/userobjects/RhieChowMassFlux.C

+      auto Ainv_full = current_local_solution.zero_clone();
+      PetscVector<Number> * Ainv_full_vec = dynamic_cast<PetscVector<Number> *>(Ainv_full.get());
+      *working_vector_petsc = 1.0;
+      Ainv_full_vec->pointwise_divide(*working_vector_petsc, *neighbor_sum_petsc);


Suggested change

Ainv_full_vec->pointwise_divide(*working_vector_petsc, *neighbor_sum_petsc);

Ainv_full->pointwise_divide(*working_vector_petsc, *row_sum);

grmnptr · 2025-02-24T21:29:05Z

modules/navier_stokes/src/userobjects/RhieChowMassFlux.C

+
+      // Create vector with new inverse projection matrix
+      auto Ainv_full = current_local_solution.zero_clone();
+      PetscVector<Number> * Ainv_full_vec = dynamic_cast<PetscVector<Number> *>(Ainv_full.get());


We actually don't need this vector. The NumericVector is enough, it has the functions you need. The cost of the virtual call for this will be negligible compared to the operations within this function.

grmnptr · 2025-02-24T21:36:50Z

modules/navier_stokes/src/userobjects/RhieChowMassFlux.C

+
+      // Correct HbyA
+      Ainv_full_vec->add(-1.0, Ainv);
+      working_vector_petsc->pointwise_mult(*Ainv_full_vec, *pressure_gradient[system_i]);


Suggested change

working_vector_petsc->pointwise_mult(*Ainv_full_vec, *pressure_gradient[system_i]);

working_vector_petsc->pointwise_mult(*Ainv_full, *pressure_gradient[system_i]);

grmnptr · 2025-02-24T21:40:49Z

modules/navier_stokes/src/userobjects/RhieChowMassFlux.C

+      HbyA.add(-1.0, *working_vector_petsc);
+
+      // Correct Ainv
+      Ainv = *Ainv_full_vec_hold;


Suggested change

Ainv = *Ainv_full_vec_hold;

Ainv = *Ainv_full_old;

grmnptr · 2025-02-24T22:01:33Z

...es/navier_stokes/test/tests/finite_volume/ins/channel-flow/SIMPLEC/2d/2d-velocity-pressure.i

+[]
+
+[Executioner]
+  type = SIMPLE


I like this design. The projection type only comes in the picture through the fields that are computed in the RC object.

grmnptr · 2025-02-24T22:02:20Z

modules/navier_stokes/test/tests/finite_volume/ins/channel-flow/SIMPLEC/2d/tests

@@ -0,0 +1,15 @@
+[Tests]
+  issues = '#27280 #28951 #29150'
+  design = 'SIMPLE.md PIMPLE.md LinearFVDivergence.md LinearWCNSFVMomentumFlux.md LinearFVMomentumPressure.md LinearFVEnergyAdvection.md LinearFVMomentumBoussinesq.md'


Suggested change

design = 'SIMPLE.md PIMPLE.md LinearFVDivergence.md LinearWCNSFVMomentumFlux.md LinearFVMomentumPressure.md LinearFVEnergyAdvection.md LinearFVMomentumBoussinesq.md'

design = 'RhieChowMassFlux.md SIMPLE.md'

grmnptr · 2025-02-24T22:03:43Z

modules/navier_stokes/test/tests/finite_volume/ins/channel-flow/SIMPLEC/3d/tests

@@ -0,0 +1,15 @@
+[Tests]
+  issues = '#27280'
+  design = 'SIMPLE.md LinearFVDivergence.md LinearWCNSFVMomentumFlux.md LinearFVMomentumPressure.md'


Suggested change

design = 'SIMPLE.md LinearFVDivergence.md LinearWCNSFVMomentumFlux.md LinearFVMomentumPressure.md'

design = 'RhieChowMassFlux.md SIMPLE.md'

tanoret requested review from lindsayad, GiudGiud and grmnptr as code owners February 12, 2025 07:23

tanoret force-pushed the simplec branch from a594e5e to d1281d1 Compare February 12, 2025 15:29

lindsayad reviewed Feb 12, 2025

View reviewed changes

adding SIMPLEC projection Refs idaholab#29864

fe10d3d

tanoret force-pushed the simplec branch from d1281d1 to fe10d3d Compare February 12, 2025 23:37

GiudGiud assigned lindsayad Feb 13, 2025

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

grmnptr requested changes Feb 24, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding SIMPLEC #29865

Adding SIMPLEC #29865

tanoret commented Feb 12, 2025

tanoret commented Feb 12, 2025

moosebuild commented Feb 12, 2025

lindsayad Feb 12, 2025

lindsayad Feb 12, 2025

lindsayad Feb 12, 2025

lindsayad Feb 12, 2025

lindsayad Feb 12, 2025

grmnptr Feb 24, 2025

lindsayad Feb 12, 2025

lindsayad Feb 12, 2025

grmnptr Feb 24, 2025

lindsayad Feb 24, 2025

moosebuild commented Feb 12, 2025 •

edited

Loading

moosebuild commented Feb 13, 2025

grmnptr left a comment

grmnptr Feb 24, 2025

grmnptr Feb 24, 2025

grmnptr Feb 24, 2025

grmnptr Feb 24, 2025

grmnptr Feb 24, 2025

grmnptr Feb 24, 2025

grmnptr Feb 24, 2025

grmnptr Feb 24, 2025

grmnptr Feb 24, 2025

grmnptr Feb 24, 2025

grmnptr Feb 24, 2025

	// Consistent Corrections to Simple
	// Consistent Corrections to SIMPLE

	const unsigned int local_size = mmat->row_stop() - mmat->row_start();
	const auto local_size = mmat->local_m();

	const unsigned int global_index = mmat->row_start() + row_i;
	const auto global_index = mmat->row_start() + row_i;

	PetscVector<Number> * Ainv_full_vec = dynamic_cast<PetscVector<Number> *>(Ainv_full.get());
	PetscVector<Number> * Ainv_full_vec = cast_ptr<PetscVector<Number> *>(Ainv_full.get());

	// Use std::accumulate with a lambda to sum row elements (no absolute values)
	// Sum row elements (no absolute values)

	auto get_row_sum_excluding_diagonal = [mmat](NumericVector<Number> & sum_vector)
	auto get_row_sum = [mmat](NumericVector<Number> & sum_vector)

	Ainv_full_vec->pointwise_divide(working_vector_petsc, neighbor_sum_petsc);
	Ainv_full->pointwise_divide(working_vector_petsc, row_sum);

	working_vector_petsc->pointwise_mult(Ainv_full_vec, pressure_gradient[system_i]);
	working_vector_petsc->pointwise_mult(Ainv_full, pressure_gradient[system_i]);

	design = 'SIMPLE.md PIMPLE.md LinearFVDivergence.md LinearWCNSFVMomentumFlux.md LinearFVMomentumPressure.md LinearFVEnergyAdvection.md LinearFVMomentumBoussinesq.md'
	design = 'RhieChowMassFlux.md SIMPLE.md'

	design = 'SIMPLE.md LinearFVDivergence.md LinearWCNSFVMomentumFlux.md LinearFVMomentumPressure.md'
	design = 'RhieChowMassFlux.md SIMPLE.md'

Adding SIMPLEC #29865

Are you sure you want to change the base?

Adding SIMPLEC #29865

Conversation

tanoret commented Feb 12, 2025

tanoret commented Feb 12, 2025

moosebuild commented Feb 12, 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

moosebuild commented Feb 12, 2025 • edited Loading

moosebuild commented Feb 13, 2025

Framework coverage

Modules coverage

Navier stokes

Full coverage reports

grmnptr 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

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 12, 2025 •

edited

Loading