Create a postprocessing function for cfd-dem #1147

yashuuzi · 2024-05-17T19:08:11Z

Description of the problem

There were no post-processing functions for cfd-dem, so we need framework to organize them
We need the post-processing function which calculate total volume in order to check if total volume is conserved

Description of the solution

Create postprocessing_cfd_dem.cc and call it in cfd_dem_coupling.cc
Create some prameters for prm file

How Has This Been Tested?

Launched several example, and confirmed that the volume calculated with analytical solution is matched with the volume of geometry

Documentation

I didn't add this to the documentation or doxygen

postprocessing_cfd_dem.cc

q_points is missed

working properly

voferreira

Good job, Soki. I have some comments. I will test it on my side before approving as well.

include/fem-dem/gls_vans.h

voferreira · 2024-05-17T22:17:09Z

include/fem-dem/parameters_cfd_dem.h

+
+  struct CFDDEM_postprocessing
+  {
+    bool calculate_total_volume;


I find "calculate total volume" a bit generic. What do you think about "compute volume phases"?

I agree, I think that would makes more sense

In this case I think it is better to be even more specific.
calculate_fluid_and_particle_volumes

or something like that. Because here this function is only used in CFD-DEM, it's used anywhere elese.

include/fem-dem/postprocessing_cfd_dem.h

source/fem-dem/gls_vans.cc

voferreira · 2024-05-17T22:24:55Z

source/fem-dem/parameters_cfd_dem.cc

+  CFDDEM_postprocessing::declare_parameters(ParameterHandler &prm)
+  {
+    prm.enter_subsection("post-processing");
+    prm.declare_entry("calculate total volume",


I feel like total volume does not give you an idea of what volume you are referring to.

voferreira · 2024-05-17T22:26:10Z

source/fem-dem/postprocessing_cfd_dem.cc

+// Base
+#include <deal.II/base/quadrature_lib.h>
+
+// Lac
+#include <deal.II/lac/dynamic_sparsity_pattern.h>
+#include <deal.II/lac/vector.h>
+
+// grid
+#include <deal.II/grid/grid_tools.h>
+
+// Dofs
+#include <deal.II/dofs/dof_accessor.h>
+#include <deal.II/dofs/dof_handler.h>
+
+// Lac - Trilinos includes
+#include <deal.II/lac/trilinos_parallel_block_vector.h>
+#include <deal.II/lac/trilinos_vector.h>
+
+// Fe
+#include <deal.II/fe/fe_q.h>
+#include <deal.II/fe/fe_system.h>
+#include <deal.II/fe/fe_values.h>
+#include <deal.II/fe/mapping_q.h>
+
+// Lethe includes
+#include <core/boundary_conditions.h>
+#include <core/parameters.h>
+#include <core/rheological_model.h>
+#include <core/vector.h>
+
+#include <fem-dem/postprocessing_cfd_dem.h>
+
+
+using namespace dealii;
+
+


You could follow the other files for this part.

voferreira · 2024-05-17T22:27:39Z

source/fem-dem/postprocessing_cfd_dem.cc

+calculate_total_volume(const DoFHandler<dim> &void_fraction_dof_handler,
+                           const VectorType   &present_void_fraction_solution,
+                           const Quadrature<dim> &quadrature_formula,
+                           std::shared_ptr<Mapping<dim>>    mapping)


@blaisb You could parse the argument as Mapping and dereference here, no?. Additionally, would it be possible to make it const?

I am unsure what you mean by parse and dereference.
Here the argument could be a const reference to a mapping, that's for sure. Using a share_ptr is kinda equivalent except that it copies the pointer. If it's a shared_ptr, it cannot be const, because shared_ptr have a ref_count mechanism that is modified when you copy it.
Passing as a const reference here would be the best choice IMO. we don't do it for the other post-processing which is a mistake, it should be like that everywhere. Just things we became better at as time went on.

I did not word it correctly but that's what I meant :)

voferreira · 2024-05-17T22:33:32Z

source/fem-dem/cfd_dem_coupling.cc

+          this->pcout << "Total volume of fluid: "
+                      << std::setprecision(
+                           this->simulation_control->get_log_precision())
+                      << this->simulation_parameters.physical_properties_manager
+                             .get_density_scale() *
+                           total_volume_fluid
+                      << " m^3" << std::endl;
+          this->pcout << "Total volume of fluid: "
+                      << std::setprecision(
+                           this->simulation_control->get_log_precision())
+                      << this->simulation_parameters.physical_properties_manager
+                             .get_density_scale() *
+                           total_volume_solid
+                      << " m^3" << std::endl;


You can use the announce string from utilities here, I feel.

voferreira · 2024-05-18T22:55:21Z

As this introduces a parameter, this parameter needs to be added to the documentation. Additionally, since this feature is used to have an assessment of the accuracy of the void fraction scheme, I feel it would be very useful to have application tests using it. This way, if we try to improve something on the current schemes, we can make sure it gives the same result.

blaisb · 2024-05-22T12:24:31Z

@yashuuzi can you re-indent the code?

blaisb

A few comments. Nothing major

blaisb · 2024-05-22T12:25:46Z

include/fem-dem/cfd_dem_coupling.h

@@ -358,5 +366,8 @@ class CFDDEMSolver : public GLSVANSSolver<dim>
  const unsigned int            this_mpi_process;
  const unsigned int            n_mpi_processes;
  LagrangianPostProcessing<dim> dem_post_processing_object;
+
+  // Post-processing variables
+  TableHandler total_volume_table;


I am unsure if total_volume is the best name, but I can't think of a better name.
Add a clearer description of what the table contains above that table, otherwise we don't know what it is used for. Post-processing variable is very vague.

blaisb · 2024-05-22T12:26:43Z

include/fem-dem/cfd_dem_simulation_parameters.h

@@ -40,6 +40,7 @@ class CFDDEMSimulationParameters

  std::shared_ptr<Parameters::VoidFraction<dim>> void_fraction;
  Parameters::CFDDEM                             cfd_dem;
+  Parameters::CFDDEM_postprocessing              cfd_dem_postprocessing;


I don't think this requires a new set of parameter. This could be directly into post-processing instead of having a new post-processing section. We already have Multiphysics postprocessing in the postprocessing block, so I think having some CFD-DEM stuff there won't hurt.

blaisb · 2024-05-22T12:26:55Z

include/fem-dem/cfd_dem_simulation_parameters.h

@@ -51,6 +52,7 @@ class CFDDEMSimulationParameters
    void_fraction = std::make_shared<Parameters::VoidFraction<dim>>();
    void_fraction->declare_parameters(prm);
    Parameters::CFDDEM::declare_parameters(prm);
+    Parameters::CFDDEM_postprocessing::declare_parameters(prm);


I think it's better to put it into the regular post-processing parameters.

blaisb · 2024-05-22T12:27:34Z

include/fem-dem/parameters_cfd_dem.h

+
+  struct CFDDEM_postprocessing
+  {
+    bool calculate_total_volume;


In this case I think it is better to be even more specific.
calculate_fluid_and_particle_volumes

or something like that. Because here this function is only used in CFD-DEM, it's used anywhere elese.

blaisb · 2024-05-22T12:27:59Z

include/fem-dem/postprocessing_cfd_dem.h

+#  include <deal.II/lac/dynamic_sparsity_pattern.h>
+#  include <deal.II/lac/vector.h>
+
+// Dofs
+#  include <deal.II/dofs/dof_handler.h>
+
+// Fe
+#  include <deal.II/fe/fe.h>
+#  include <deal.II/fe/mapping_fe.h>
+
+// Lethe includes
+#  include <core/boundary_conditions.h>


Can you double check if you need all these includes ? I think not.

source/fem-dem/CMakeLists.txt

blaisb · 2024-05-22T12:30:15Z

source/fem-dem/cfd_dem_coupling.cc

+CFDDEMSolver<dim>::postprocess_cfd_dem()
+{
+  // Calculate total volume of fluid and solid 
+  if (this->cfd_dem_simulation_parameters.cfd_dem_postprocessing.calculate_total_volume)


Just put it straight in the post-processing parameters (the regular one)

blaisb · 2024-05-22T12:30:33Z

source/fem-dem/cfd_dem_coupling.cc

+          this->pcout << "Total volume of fluid: "
+                      << std::setprecision(
+                           this->simulation_control->get_log_precision())
+                      << this->simulation_parameters.physical_properties_manager
+                             .get_density_scale() *
+                           total_volume_fluid
+                      << " m^3" << std::endl;
+          this->pcout << "Total volume of fluid: "
+                      << std::setprecision(
+                           this->simulation_control->get_log_precision())
+                      << this->simulation_parameters.physical_properties_manager
+                             .get_density_scale() *
+                           total_volume_solid
+                      << " m^3" << std::endl;


blaisb · 2024-05-22T12:30:53Z

source/fem-dem/postprocessing_cfd_dem.cc

+#include <deal.II/base/quadrature_lib.h>
+
+// Lac
+#include <deal.II/lac/dynamic_sparsity_pattern.h>
+#include <deal.II/lac/vector.h>
+
+// grid
+#include <deal.II/grid/grid_tools.h>
+
+// Dofs
+#include <deal.II/dofs/dof_accessor.h>
+#include <deal.II/dofs/dof_handler.h>
+
+// Lac - Trilinos includes
+#include <deal.II/lac/trilinos_parallel_block_vector.h>
+#include <deal.II/lac/trilinos_vector.h>
+
+// Fe
+#include <deal.II/fe/fe_q.h>
+#include <deal.II/fe/fe_system.h>
+#include <deal.II/fe/fe_values.h>
+#include <deal.II/fe/mapping_q.h>
+
+// Lethe includes
+#include <core/boundary_conditions.h>
+#include <core/parameters.h>
+#include <core/rheological_model.h>
+#include <core/vector.h>
+
+#include <fem-dem/postprocessing_cfd_dem.h>


Do you need all these includes? I think not.

blaisb · 2024-05-22T12:32:18Z

source/fem-dem/postprocessing_cfd_dem.cc

+calculate_total_volume(const DoFHandler<dim> &void_fraction_dof_handler,
+                           const VectorType   &present_void_fraction_solution,
+                           const Quadrature<dim> &quadrature_formula,
+                           std::shared_ptr<Mapping<dim>>    mapping)


I am unsure what you mean by parse and dereference.
Here the argument could be a const reference to a mapping, that's for sure. Using a share_ptr is kinda equivalent except that it copies the pointer. If it's a shared_ptr, it cannot be const, because shared_ptr have a ref_count mechanism that is modified when you copy it.
Passing as a const reference here would be the best choice IMO. we don't do it for the other post-processing which is a mistake, it should be like that everywhere. Just things we became better at as time went on.

source/fem-dem/postprocessing_cfd_dem.cc

blaisb · 2024-05-22T12:33:18Z

source/fem-dem/postprocessing_cfd_dem.cc

+              // Get the void fraction at the quadrature point
+              fe_vf_values[void_fraction].get_function_values(
+                present_void_fraction_solution, void_fraction_values);


This should be done outside of the loop on gauss points.

yashuuzi · 2024-05-22T15:01:50Z

Thank you so much for reviewing! I'll fix them and let you know when I've done

yashuuzi · 2024-05-30T19:14:21Z

I'm ready for second review!
Actually I could't come up with good name of parameter which is better than "volume phases", but I think it's understandable.

blaisb

1-2 last comment.
One more thing, can you add an application test, this would be very important to ensure the feature is stable as a function of time :).

blaisb · 2024-05-31T13:47:31Z

doc/source/parameters/cfd/post_processing.rst

@@ -252,5 +259,23 @@ This subsection controls the post-processing other than the forces and torque on

 * ``phase energy name``: name of the output file containing phase energies from Cahn-Hilliard simulations. The default file name is ``phase_energy``.

+* ``calculate volume phases``: outputs total volume of fluid phase and total volume of solid phase in CFD-DEM simulation. These volumes are computed as follow:


should be phase volumes and not volume phases
like we calculate the volume of the phases and not the phase volume
:)

doc/source/parameters/cfd/post_processing.rst

include/core/parameters.h

blaisb · 2024-05-31T13:48:50Z

include/core/parameters.h

@@ -916,6 +916,13 @@ namespace Parameters
    /// Prefix for the energy output in Cahn-Hilliard simulations
    std::string phase_energy_output_name;

+    /// Enable calculation of total fluid volume and total particles volume in
+    /// cfd-dem simulation
+    bool calculate_volume_phases;


would rename phase_volumes instead of volume_phases, but that's minor

blaisb · 2024-05-31T13:49:39Z

include/fem-dem/cfd_dem_coupling.h

+  // Post-processing variables to output total fluid volume and total particles
+  // volume


use /// instead of // for these type of comments

voferreira

All good to me after applying Bruno's comments. Good job!

voferreira · 2024-06-03T00:38:53Z

Please, don't forget to rebase. Your branch is 20 commits behind master.

Co-authored-by: Bruno Blais <blais.bruno@gmail.com>

change "volume_phases" into "phase_volumes"

blaisb · 2024-06-05T07:39:44Z

@yashuuzi Is this ready for a final review / merge? Try to finish this as a priority :)

yashuuzi · 2024-06-05T16:15:03Z

I finished modifying and ready to final review!

blaisb and others added 17 commits May 10, 2024 16:49

Improved spouted-bed examples

f115704

Fix documentation

1d9e21c

create postprocessing_cfd_dem.h

c52dd50

postprocessing_cfd_dem.cc

edit gls_vans.h, gls_vans.cc

3912dab

edit CMakeLists.txt

8ebd17b

fixed bug on 1st complie

f47ca7d

q_points is missed

fix the bug related to postprocessing_cfd_dem file

793e59e

fixed bug related to "this->"

e8a2a84

Bruno modified something

00ba941

move postproseccing function file to fem-dem/

6bd18ca

Deleted file

46c9b6b

successed in compiling

84da1be

move everything from gls_vans to cfd_dem_coupling

f15d0e0

change mapping

d0c7267

successeded in calculating void fraction

21117dc

complete

e54157a

working properly

fixed conflict

e859d27

yashuuzi requested review from blaisb and voferreira May 17, 2024 19:08

yashuuzi added Ready for review Hackathon labels May 17, 2024

yashuuzi changed the title ~~Create a framework for postprocessing of cfd-dem, and function which calculate the total fluid volume and total solid volume respectively~~ Create a postprocessing function for cfd-dem May 17, 2024

voferreira requested changes May 17, 2024

View reviewed changes

blaisb requested changes May 22, 2024

View reviewed changes

blaisb added Reviewed and waiting for changes and removed Ready for review labels May 22, 2024

yashuuzi force-pushed the postprocessing_void_fraction branch from 6f0e23a to 6bff7eb Compare May 29, 2024 18:54

yashuuzi added 7 commits May 29, 2024 15:22

reindent using vim shortcut

2d7c533

reindent again

748925e

add a line at the end of file gls_vans.h

3796990

add the description of function to document, parameter section

d98715c

Add parameter to the document

d3496ae

change the name of output file on doc

22856ad

small change

718da85

yashuuzi added Ready for review and removed Reviewed and waiting for changes labels May 30, 2024

yashuuzi requested review from voferreira and blaisb May 30, 2024 19:14

blaisb requested changes May 31, 2024

View reviewed changes

blaisb added Reviewed and waiting for changes and removed Ready for review labels May 31, 2024

voferreira approved these changes Jun 3, 2024

View reviewed changes

yashuuzi and others added 2 commits June 4, 2024 10:31

Apply suggestions from the 2nd review

2571a70

Co-authored-by: Bruno Blais <blais.bruno@gmail.com>

make application_test

6e74378

change "volume_phases" into "phase_volumes"

yashuuzi added 2 commits June 5, 2024 10:53

create application test, but ctest fail

0a7c2f3

passed the test

5c77e41

yashuuzi requested a review from blaisb June 5, 2024 16:15

yashuuzi added Ready for review and removed Reviewed and waiting for changes labels Jun 5, 2024

blaisb approved these changes Jun 6, 2024

View reviewed changes

blaisb merged commit 2c19eab into master Jun 6, 2024
8 checks passed

blaisb deleted the postprocessing_void_fraction branch June 6, 2024 08:27

		@@ -252,5 +259,23 @@ This subsection controls the post-processing other than the forces and torque on

		* ``phase energy name``: name of the output file containing phase energies from Cahn-Hilliard simulations. The default file name is ``phase_energy``.

		* ``calculate volume phases``: outputs total volume of fluid phase and total volume of solid phase in CFD-DEM simulation. These volumes are computed as follow:

		// Post-processing variables to output total fluid volume and total particles
		// volume

Create a postprocessing function for cfd-dem #1147

Create a postprocessing function for cfd-dem #1147

Conversation

yashuuzi commented May 17, 2024

Description of the problem

Description of the solution

How Has This Been Tested?

Documentation

voferreira 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

voferreira commented May 18, 2024

blaisb commented May 22, 2024

blaisb 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

yashuuzi commented May 22, 2024

yashuuzi commented May 30, 2024

blaisb 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

voferreira left a comment

Choose a reason for hiding this comment

voferreira commented Jun 3, 2024

blaisb commented Jun 5, 2024

yashuuzi commented Jun 5, 2024