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

Clean up particle-particle contact force class prior refactoring particle-wall contact force class in DEM #1237

Closed
wants to merge 1,550 commits into from

Conversation

acdaigneault
Copy link
Collaborator

@acdaigneault acdaigneault commented Aug 11, 2024

Description

The particle-particle contact force class had some redundancies, so I managed to implement functions that are now called.
Some cleanup was been done here and there, but most of it is related to the call to the calculation of the contact force for a pair of particle to the proper model.
Before this refactoring, each (5) loop over the adjacent pair had the same series of if constexpr (contact_model == ...), now each loop call execute_contact_calculation() according to the contact type as a template.

This clean up was done before applying the same architecture pattern for the particle-wall contact force class.

Testing

Tests are good.

Documentation

Miscellaneous (will be removed when merged)

I would like to have Gabo reviewing it before the merge since he has coded and may continue to code in the class and I want something that he agree on. However, I forbid him to do it during his mini-project hehe.

Checklist (will be removed when merged)

See this page for more information about the pull request process.

Code related list:

  • All in-code documentation related to this PR is up to date (Doxygen format)
  • Lethe documentation is up to date
  • The branch is rebased onto master
  • Code is indented with indent-all and .prm files (examples and tests) with prm-indent
  • If parameters are modified, the tests and the documentation of examples are up to date
  • Changelog (CHANGELOG.md) is up to date if the refactor affects the user experience or the codebase

Pull request related list:

  • No other PR is open related to this refactoring
  • Labels are applied
  • There are at least 2 reviewers (or 1 if small feature) excluding the responsible for the merge
  • If this PR closes an issue or is related to a project, it is linked in the "Projects" or "Development" section
  • If any future works is planed, an issue is opened
  • The PR description is cleaned and ready for merge

AmishgaAlphonius and others added 30 commits March 13, 2024 12:14
Description
This PR adds a small section in the melting cavity example featuring the new constrain stasis feature from (Temperature-dependant solid domain constraints #1038).
Documentation
Parameter documentation was also updated : doc/source/parameters/cfd/constrain_stasis.rst

Former-commit-id: e843eae
Description of the problem
The GlobalVectorType was introduced in two prototypes in PR #1001.

Description of the solution
This change was reverted as this is just a prototype and we do not need it to work with Trilinos and L::d::V vectors.



Former-commit-id: 3d2ffd2
Description of the problem
When calculate mass conservation in the post-processing was enabled (default) and the verbosity was set to verbose, the mass conservation table was displayed on terminal at the end of the simulation. If a lot of time iterations were made, this could cover a significant chunk of the terminal window and might not be pertinent to have it their when .dat files is also outputted by the simulation.

Description of the solution
In a similar way to "VOF Barycenter", the output of the "VOF Mass Conservation" is now displayed at every time iteration on the terminal.
How Has This Been Tested?
Expected tests have been updated output :

applications_tests/lethe-fluid/gls_vof_1_isothermal_compressible_fluid.output
applications_tests/lethe-fluid/gls_vof_2_isothermal_compressible_fluids.output
applications_tests/lethe-fluid/gls_vof_dirichlet_boundary_condition.output
applications_tests/lethe-fluid/gls_vof_periodic_boundary_condition.output
applications_tests/lethe-fluid/time_dependent_boundaries_vof.output
Mass monitoring has been added to the following test to improve robustness:

applications_tests/lethe-fluid/heat_transfer_vof_phase_change_constrain_solid_domain.prm (.output)

Future changes
Change all set_precision in the make_table functions to set_scientific as default.

Comments
A new make_table_scalars_vectors function has been added in utilities.h/.cc.
A new boolean argument in make_table functions has been added, namely display_scientific_notation to decide if the display of table values is displayed at a fixed decimal or in scientific notation.

Former-commit-id: 82095ba
Description of the problem
Some physical and interface property models require certain fields to compute property values and partial derivatives. When these fields were not defined a not so debug-friendly message was printed out.
Description of the solution
To help the debugging process, if an error were to occur, assertions were added to the property models in functions where the fields were required.
How Has This Been Tested?
By commenting lines in unit tests and code, the throw of the exception was verified in DEBUG mode.
Comments
Doxygen descriptions were also added to specific_heat_model.h.

Former-commit-id: bc03310
Description of the problem
The eigenvalue estimation for the smoother of the multigrid preconditioners was done through a PreconditionChebyshev.

Description of the solution
Due to recent changes in deal.II (dealii/dealii#16742), now we are able to do it using the PreconditionRelaxation directly. This allows us to remove the estimate omega function since it is done internally by the smoother if the relaxation parameter is set to 1. This allows also to remove the degree parameter that was only used in the case of Chebyshev.

Former-commit-id: 30f1a30
Former-commit-id: b633f6f
#1072)

Description of the problem
The recently implemented vof stasis constraint (Implement temperature-dependant solid domain contraints in VOF #1048) was not compatible with adaptive mesh refinements and checkpointing system. When refining the mesh or when restarting a simulation, the filtered phase fraction was reinitialized, but not filled with the appropriate values.

Description of the solution
The apply_phase_filter() function was added to post_mesh_adaptation() and read_checkpoint() functions.

How Has This Been Tested?
The stasis constraint was tested using the 2D LPBF benchmark :
with restart and mesh adaptation;
without restart and with mesh adaptation, and;
with restart and without mesh adaptation.

Former-commit-id: bb7cea3
Description of the problem
The flow control subsection in the documentation for this example was not up to date.

Description of the solution
Update section in accordance to parameter file and new parameters for dynamic flow control.

Former-commit-id: 3e3f3f3
Description of the problem
No tutorial on how to use Pointwise the new meshing software used by the group.
Pointwise tutorial is not that useful for our uses of it
Description of the solution
Add Pointwise tutorial
Future work
Add examples

Co-authored-by: Bruno Blais <[email protected]>
Former-commit-id: b47f7d2
Description of the problem
For some cases we are running, it is relevant to monitor both the mass and the momentum in VOF simulations

Description of the solution
Add momentum monitoring to the mass monitoring. I don't think it's worth it to make multiple files, instead I appended the information to the existing file.

How Has This Been Tested?
Tested on some of our regular benchmark and the results make sense.

Documentation
Updated documentation accordingly
Future changes
The parameter monitor mass conservation is maybe not the most appropriate right now, but I don't want to break the prm file in this PR. We could have a parameter called monitor conservation instead, of maybe monitoring mass and momentum, but i 'd rather postpone this change because it's only esthetics

Co-authored-by: hepap <[email protected]>
Co-authored-by: Amishga Alphonius <[email protected]>
Former-commit-id: 73306f4
* fix lid driven cavity postprocessing

* update doc

Former-commit-id: 01be39f
Description of the problem
The insertion box for the volume insertion was being defined by a large number of parameters (9 to be exact)

Description of the solution
The box is defined by one parameter and the order of direction by an other.

Documentation
The documentation has been updated accordingly.

Comments
The list insertion parameters are now parsed with "convert_string_to_vector", which is cleaner to look at.
I have made (with the help of Amishga) a bash script to parse every prm file and change the old parameters into the new ones. We could do something with it... maybe?

Former-commit-id: 219ca30
Description of the problem
This is a follow up from Victor's comment from PR [Dem volume insertion box parameters #1074
The initial velocity and initial angular velocity are now define with one parameter each.

Really quick PR.

No example or application test (except for one) was using those parameters.



Former-commit-id: 698b063
Follow-up to PR #1078 ...

Former-commit-id: 13e9dcb
kinematic pressure: pressure/reference_state_density
reduced pressure: pressure/critical_pressure
The "kinematic pressure" in the documentation on incompressible Navier-Stokes equations was referred to as "reduced pressure"; this has been replaced.

Former-commit-id: 734f8da
Description of the problem
The naming scheme for the insertion files were not coherent with the rest of the code. (i.e., integrator_... , particle_wall_... , etc.). This was annoying when trying to find a specific insertion file. Now they are next to each other.
Description of the solution
The files are now named like this: insertion_method.cc or insertion_method.h
How Has This Been Tested?
Tests are running on my machine with these changes.

Former-commit-id: 16425e9
Description of the problem
The particle-wall nonlinear model wasn't following the same coulomb-limit as the particle-particle hertz-limit-overlap model
Comments
Tests outputs had to be updated since the position of the particles are slightly different.

Former-commit-id: d1d9373
Former-commit-id: 4990749
Former-commit-id: 4e45598
Description of the problem
A test seems to break on master version of deal.II
Description of the solution
Decrease tolerance to be more robust

Former-commit-id: 9f84d35
Description of the problem
On some clusters, it is recommend to specify OMP_NUM_THREADS = 1, otherwise Trilinos can start using multithreading.
Description of the solution
I added a line in the .dealii file with a small comment.

Former-commit-id: 469bbe3
Description of the problem
Using the Matrix Free solver, Joel cooked up a very cool 3d Taylor-Couette example that really showcases the power of the matrix free approach
Description of the solution
To keep this example for the future, we add it as an example test case.


Co-authored-by: Pierre Laurentin <[email protected]>
Co-authored-by: Laura Prieto Saavedra <[email protected]>
Former-commit-id: 95dbbc5
Description of the problem
The console output of the non-linear solver in the NS solver was showing the the norms of the newton update without distinction between the velocity and pressure contributions.
The final residual norm of the linear solver was not outputted, only the tolerance.
Description of the solution
Add the distinction between the velocity and pressure contributions in the non-linear solver output.
Add the final value of the linear residual when verbosity is at verbose. If it is at extra verbose, the solver output the residual for each iteration (already implemented but not documented).
How Has This Been Tested?
A unit test was added.
 vector_problem.cc

Co-authored-by: Pierre Laurentin <[email protected]>
Co-authored-by: Amishga Alphonius <[email protected]>
Former-commit-id: 4632e53
Description of the problem
The third argument of the MPI_InitFinalize(...) call in all applications in Lethe was set to numbers::invalid_unsigned_int; if this is the case, then the number of threads is determined automatically according to the system and set via MultithreadInfo::set_thread_limit(...).

Due to confusions this week in the lab, I though that it would be a good idea to set it explicitly to 1, which guarantees that it will start as many MPI processes per node as there are cores on each node avoiding multi-threading.

For the main applications, there was a call MultithreadInfo::set_thread_limit(1) in the respective solve() function, however, according to deal.II, it only works if it is called before any threads are created, therefore, it is safer to call it at the beginning of main() instead.

Description of the solution
Removed MultithreadInfo::set_thread_limit(1) from the applications and set the third argument of MPI_InitFinalize(...) to 1 in all cases across the library.

How Has This Been Tested?
All the tests pass without any issue.

Former-commit-id: 34ff2b6
Description of the problem
The GMG preconditioners were defined within two functions in the matrix-free solver. This was preventing us from reusing certain parts of the multigrid.

Description of the solution
The GMG preconditioners now have their own class and objects. This also allows to use the inexact newton solver for the lethe-fluid-matrix-free application. In addition, some corrections to the local smoothing algorithm were introduced by eliminating the interface out matrix.

How Has This Been Tested?
All the existent matrix-free tests pass without any issue.

Future changes
-This is the first step towards reusing parts of multigrid to improve the performance of the matrix-free solver.


Co-authored-by: Peter Munch <[email protected]>
Co-authored-by: Bruno Blais <[email protected]>
Former-commit-id: d7504a6
AmishgaAlphonius and others added 18 commits September 13, 2024 15:58
Description
The previous implementation to calculate cell diameter used the measure function which assumed a linear mapping.

Solution
This PR adds a function that calculates the volume by integrating 1 over the cell, by summing JxW values returned by the FEValues object.

Testing
Tests that were outdated have been updated.

Former-commit-id: 0430787
Remove include in ib that make it recompiled

Move periodic offset as member variable

Remove effective mass and radius as member variable

Modify all force except DMT

Put rolling torque calculation in function

Clean up set pp force models with predefined template abuse

First cleanup set pp force models

Upgrade p-p contact forces doc and intermediate variables


Former-commit-id: 794f832
Former-commit-id: a73797f
Modify back to each argument for container

Temporary merge other branch


Former-commit-id: 130c252
Former-commit-id: a537d03
Former-commit-id: 5b7f3fe
This reverts commit 37963ad3a6e1f5472827af9bac67dcee105cbc8b.


Former-commit-id: 93b3555
Former-commit-id: 99fafe2
Former-commit-id: 0f0afbf
Copy link
Collaborator

@voferreira voferreira left a comment

Choose a reason for hiding this comment

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

Nice refactoring! For me, it is good to go.

@@ -3,591 +3,243 @@

using namespace dealii;

using namespace Parameters::Lagrangian;


Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready for review Refactoring This PR is only refactoring or clean up
Projects
None yet
Development

Successfully merging this pull request may close these issues.