Apply suggestions from code review

Co-authored-by: Amishga Alphonius <107414376+AmishgaAlphonius@users.noreply.github.com>
chaos-polymtl · Dec 4, 2024 · fd49ba9 · fd49ba9
1 parent d7e3e42
commit fd49ba9
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 43 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,7 +8,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
 
 ### Removed
 
-- MAJOR The ability to bound the void fraction from below and above (using l2 lower bound and l2 upper bound parameters) has been removed. This bounding of the void fraction was highly problematic, since it could create discontinuities in the time derivative of the void fraction and contaminate the solution. In reality, this feature was never used. Importantly, this PR refactors the calculation of the void fraction outside of the VANS and CFD-DEM solvers of Lethe so that it exists only independently in a seperate subequation solver. This change is in preperation for the matrix free implementation of the VANS equations.
+- MAJOR The ability to bound the void fraction from below and above (using l2 lower bound and l2 upper bound parameters) has been removed. This bounding of the void fraction was highly problematic, since it could create discontinuities in the time derivative of the void fraction and contaminate the solution. In reality, this feature was never used. Importantly, this PR refactors the calculation of the void fraction outside of the VANS and CFD-DEM solvers of Lethe so that it exists only independently in a seperate subequation solver. This change is in preperation for the matrix free implementation of the VANS equations. [#1392](https://github.com/chaos-polymtl/lethe/pull/1392)
 
 ## Release of Lethe v1.0 - 2024-11-30
 

diff --git a/include/fem-dem/void_fraction.h b/include/fem-dem/void_fraction.h
@@ -29,13 +29,13 @@
 using namespace dealii;
 
 /**
- * @brief Calculates the area of intersection between a circular (2D) particle and a circle
+ * @brief Calculate the area of intersection between a circular (2D) particle and a circle.
  *
- * @param r_particle Radius of the particle
+ * @param[in] r_particle Radius of the particle
  *
- * @param r_circle Radius of the circle
+ * @param[in] r_circle Radius of the circle
  *
- * @param neighbor_distance Distance between the particle and the circle
+ * @param[in] neighbor_distance Distance between the particle and the circle
  */
 inline double
 particle_circle_intersection_2d(double r_particle,
@@ -58,13 +58,13 @@ particle_circle_intersection_2d(double r_particle,
 }
 
 /**
- * @brief Calculates the volume of intersection between a spherical (3D) particle and a sphere
+ * @brief Calculate the volume of intersection between a spherical (3D) particle and a sphere.
  *
- * @param r_particle Radius of the particle
+ * @param[in] r_particle Radius of the particle
  *
- * @param r_sphere Radius of the sphere
+ * @param[in] r_sphere Radius of the sphere
  *
- * @param neighbor_distance Distance between the particle and the sphere
+ * @param[in] neighbor_distance Distance between the particle and the sphere
  */
 
 inline double
@@ -129,15 +129,15 @@ class VoidFractionBase : public PhysicsLinearSubequationsSolver
   }
 
   /**
-   * @brief Setup the degrees of freedom
+   * @brief Setup the degrees of freedom.
    *
    */
   void
   setup_dofs() override;
 
 
   /**
-   * @brief Establishes the constraints of the void fraction systems.
+   * @brief Establish the constraints of the void fraction systems.
    *
    * @param boundary_conditions The boundary conditions of fluid dynamics. This is used to establish periodic boundary conditions for the void fraction.
    *
@@ -148,9 +148,9 @@ class VoidFractionBase : public PhysicsLinearSubequationsSolver
 
 
   /**
-   * @brief Assembles the diagonal of the mass matrix to impose constraints on the system
+   * @brief Assemble the diagonal of the mass matrix to impose constraints on the system
    *
-   *  @param diagonal_mass_matrix The matrix for which the diagonal entries will be filled with the diagonal of the mass matrix
+   *  @param[out] diagonal_mass_matrix The matrix for which the diagonal entries will be filled with the diagonal of the mass matrix
    *
    *  @todo Establish if it is really necessary to keep this as a matrix and not as a vector since a diagonal is nothing more than a vector.
    */
@@ -159,9 +159,9 @@ class VoidFractionBase : public PhysicsLinearSubequationsSolver
     TrilinosWrappers::SparseMatrix &diagonal_mass_matrix);
 
   /**
-   * @brief Calculates the void fraction
+   * @brief Calculate the void fraction.
    *
-   * @param time current time for which the void fraction is to be calculated.
+   * @param[in] time Current time for which the void fraction is to be calculated.
    *
    */
   void
@@ -180,7 +180,7 @@ class VoidFractionBase : public PhysicsLinearSubequationsSolver
 
 
   /**
-   * @brief Percolates the time vector for the void fraction. This operation is called at the end of a time-step.
+   * @brief Percolate the time vector for the void fraction. This operation is called at the end of a time-step.
    *
    */
   void
@@ -235,37 +235,37 @@ class VoidFractionBase : public PhysicsLinearSubequationsSolver
 
 private:
   /**
-   * @brief Calculates the void fraction using a function. This is a straightforward usage of VectorTools.
+   * @brief Calculate the void fraction using a function. This is a straightforward usage of VectorTools.
    *
-   * @param time current time for which the void fraction is to be calculated.
+   * @param[in] time Current time for which the void fraction is to be calculated.
    *
    */
   void
   calculate_void_fraction_function(const double time);
 
   /**
-   * @brief Calculates the void fraction using the particle centered method.
+   * @brief Calculate the void fraction using the particle centered method.
    *
    */
   void
   calculate_void_fraction_particle_centered_method();
 
   /**
-   * @brief Calculates the void fraction using the satellite point method.
+   * @brief Calculate the void fraction using the satellite point method.
    *
    */
   void
   calculate_void_fraction_satellite_point_method();
 
   /**
-   * @brief Calculates the void fraction using the Quadrature-Centered Method (QCM).
+   * @brief Calculate the void fraction using the Quadrature-Centered Method (QCM).
    *
    */
   void
   calculate_void_fraction_quadrature_centered_method();
 
   /**
-   * @brief Solve the linear system resulting from the assemblies
+   * @brief Solve the linear system resulting from the assemblies.
    *
    * @param is_post_mesh_adaptation Indicates if the equation is being solved during post_mesh_adaptation(),
    * for verbosity.
@@ -275,15 +275,15 @@ class VoidFractionBase : public PhysicsLinearSubequationsSolver
     const bool &is_post_mesh_adaptation = false) override;
 
   /**
-   * @brief This function calculates and returns the periodic offset distance of the domain which is needed
+   * @brief Calculate and return the periodic offset distance vector of the domain which is needed
    * for the periodic boundary conditions using the QCM or SPM for void fraction
    * with the GLS VANS/CFD-DEM solver. The distance is based on one of the
-   * periodic boundaries and all particle location shifted by this distance is
-   * according to this periodic boundary.
+   * periodic boundaries. This periodic boundary is then used as the reference 
+   * for all particle locations shifted by this offset vector.
    *
-   * @param boundary_id The id of one of the periodic boundaries
+   * @param[in] boundary_id The ID of one of the periodic boundaries
    *
-   * @return The periodic offset distance
+   * @return The periodic offset vector.
    */
   inline Tensor<1, dim>
   get_periodic_offset_distance(unsigned int boundary_id) const

diff --git a/source/fem-dem/void_fraction.cc b/source/fem-dem/void_fraction.cc
@@ -418,7 +418,7 @@ VoidFractionBase<dim>::calculate_void_fraction_satellite_point_method()
                   double translational_factor =
                     particle_properties[DEM::PropertiesIndex::dp] * 0.5;
 
-                  // Resize and translate reference sphere
+                  // Resize and translate the reference sphere
                   // to the particle size and position according the volume
                   // ratio between sphere and particle.
                   for (unsigned int l = 0;
@@ -565,7 +565,7 @@ VoidFractionBase<dim>::calculate_void_fraction_quadrature_centered_method()
   double quadrature_void_fraction;
   double qcm_sphere_diameter = void_fraction_parameters->qcm_sphere_diameter;
 
-  // If the reference sphere diameter is user defined, the radius is
+  // If the reference sphere diameter is user-defined, the radius is
   // calculated from it, otherwise, the value must be calculated while looping
   // over the cells.
   bool calculate_reference_sphere_radius = true;
@@ -595,7 +595,7 @@ VoidFractionBase<dim>::calculate_void_fraction_quadrature_centered_method()
   system_rhs_void_fraction    = 0;
   system_matrix_void_fraction = 0;
 
-  // Clear all contributions of particles from the previous timestep
+  // Clear all contributions of particles from the previous time-step
   for (const auto &cell : dof_handler.active_cell_iterators())
     {
       if (cell->is_locally_owned())
@@ -716,7 +716,7 @@ VoidFractionBase<dim>::calculate_void_fraction_quadrature_centered_method()
                       // Particle completely outside reference
                       // sphere. Do absolutely nothing.
 
-                      // Particle partially in reference sphere
+                      // Particle partially in the reference sphere
                       else if ((neighbor_distance > (r_sphere - r_particle)) &&
                                (neighbor_distance < (r_sphere + r_particle)))
                         {
@@ -781,7 +781,7 @@ VoidFractionBase<dim>::calculate_void_fraction_quadrature_centered_method()
                         particle_location.distance(
                           periodic_neighbor_quadrature_point_location[n][k]);
 
-                      // Particle completely in reference sphere
+                      // Particle completely in the reference sphere
                       if (periodic_neighbor_distance <= (r_sphere - r_particle))
                         {
                           particle_properties
@@ -792,10 +792,10 @@ VoidFractionBase<dim>::calculate_void_fraction_quadrature_centered_method()
                             (2.0 * dim);
                         }
 
-                      // Particle completely outside reference
+                      // Particle completely outside the reference
                       // sphere. Do absolutely nothing.
 
-                      // Particle partially in reference sphere
+                      // Particle partially in the reference sphere
                       else if ((periodic_neighbor_distance >
                                 (r_sphere - r_particle)) &&
                                (periodic_neighbor_distance <
@@ -913,7 +913,7 @@ VoidFractionBase<dim>::calculate_void_fraction_quadrature_centered_method()
                       distance = particle.get_location().distance(
                         quadrature_point_location[q]);
 
-                      // Particle completely in reference sphere
+                      // Particle completely in the reference sphere
                       if (distance <= (r_sphere - r_particle))
                         particles_volume_in_sphere +=
                           (M_PI *
@@ -924,10 +924,10 @@ VoidFractionBase<dim>::calculate_void_fraction_quadrature_centered_method()
                           particle_properties
                             [DEM::PropertiesIndex::volumetric_contribution];
 
-                      // Particle completely outside reference sphere. Do
+                      // Particle completely outside the reference sphere. Do
                       // absolutely nothing.
 
-                      // Particle partially in reference sphere
+                      // Particle partially in the reference sphere
                       else if ((distance > (r_sphere - r_particle)) &&
                                (distance < (r_sphere + r_particle)))
                         {
@@ -953,8 +953,8 @@ VoidFractionBase<dim>::calculate_void_fraction_quadrature_centered_method()
 
               // Execute same operations for periodic neighbors, if the
               // simulation has no periodic boundaries, the container is
-              // empty. Also, those operations can not be done in the previous
-              // loop because the particles on the periodic side needs a
+              // empty. Also, those operations cannot be done in the previous
+              // loop because the particles on the periodic side need a
               // correction with an offset for the distance with the
               // quadrature point
               for (unsigned int m = 0; m < active_periodic_neighbors.size();
@@ -992,7 +992,7 @@ VoidFractionBase<dim>::calculate_void_fraction_quadrature_centered_method()
                       distance = particle_location.distance(
                         quadrature_point_location[q]);
 
-                      // Particle completely in reference sphere
+                      // Particle completely in the reference sphere
                       if (distance <= (r_sphere - r_particle))
                         particles_volume_in_sphere +=
                           (M_PI *
@@ -1003,10 +1003,10 @@ VoidFractionBase<dim>::calculate_void_fraction_quadrature_centered_method()
                           particle_properties
                             [DEM::PropertiesIndex::volumetric_contribution];
 
-                      // Particle completely outside reference sphere. Do
+                      // Particle completely outside the reference sphere. Do
                       // absolutely nothing.
 
-                      // Particle partially in reference sphere
+                      // Particle partially in the reference sphere
                       else if ((distance > (r_sphere - r_particle)) &&
                                (distance < (r_sphere + r_particle)))
                         {
@@ -1134,7 +1134,7 @@ VoidFractionBase<dim>::solve_linear_system_and_update_solution(
   void_fraction_locally_relevant = completely_distributed_solution;
 }
 
-// Pre-compile the 2D and 3D Navier-Stokes solver to ensure that the
+// Pre-compile the 2D and 3D VoidFractionBase solver to ensure that the
 // library is valid before we actually compile the solver This greatly
 // helps with debugging
 template class VoidFractionBase<2>;