Refactor the way the DEM PropertiesIndex are managed using templates #1399

blaisb · 2025-01-01T19:55:19Z

Description

The deal.II ParticleHandler class supports storing a number of properties as double within the a PropertyPool. The issue is that this PropertyPool is nothing more than an array of double and associating the elements of that array to a physical meaning (e.g. knowing that property 8 is the diameter) can be challenging. To alleviate this issue, we have always used an enum of type int to do the link between the properties and their index. This is convenient, but it also hardcodes at compile time the properties. This has forced us to have the same number of properties in DEM and CFD-DEM. This is wasteful, because all of the hydrodynamic forces and torques, which are just zero in DEM, have the be stored anyway in a DEM simulation.

This PR changes this issue by templating ALL of the DEM classes that use the properties of the particle with a typename that is an enum. This typename PropertiesIndex is then used within the class to do the calculation with the adequate properties index. This essentially enable the contact models, the integrators, or whatever you actually want, to be reused with ParticleHandler that have a different amount of properties.

Some of the planned usage for this are notably heat transfer within particles, but also the addition of more properties on the particle within CFD-DEM (e.g. previous particle velocity like we discussed with @voferreira ). This PR opens all of these possibilities.

This is a major change because it changes everything in the code everywhere, but does not change the logic. The DEM results are unchanged, but the CFD-DEM results are changed.
This is because, now CFD-DEM requires translating a DEM ParticleHandler to a CFD-DEM ParticleHandler. To achieve this, I read the ParticleHandler from the DEM simulation and transform it into a CFD-DEM ParticleHandler using a new conversion function I wrote.

This essentially opens-up the possibility of reading particles that were created on a different mesh and converting it on a new one. This is what i will do in the next PR and this will essentially disconnect the mesh used for DEM and for CFD-DEM. Enabling us to change the CFD-DEM mesh without redoing a DEM insertion (which is cool). @voferreira this feature will be useful for your mesh convergence work

Testing

The CFD-DEM test results have changed because of the aforementioned issue

Documentation

This does not modify the documentation except for a minor parameter that was added.

Miscellaneous (will be removed when merged)

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)
Copyright headers are present and up to date
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 planned, an issue is opened
The PR description is cleaned and ready for merge

…to generate new restart files

…nput files.

OGaboriault · 2025-01-08T20:55:41Z

CHANGELOG.md

+
+### Changed
+
+- MAJOR The index of properties in the DEM and CFD-DEM simulations are now indexed using a template (PropertiesIndex) instead of using an hardcoded enum. This enables the different solvers to have different number of properties and different properties index. For large simulations, this gives minor performance gain for DEM simulations (e.g. around 5-10%), but this makes the solver significantly more flexible. This is a major change since it breaks DEM and CFD-DEM restart files from previous versions. [#1399](https://github.com/chaos-polymtl/lethe/pull/1399)


Maybe add something regarding the potential bug you found along the way regarding the particle handler and vans solver?

OGaboriault · 2025-01-08T21:01:01Z

doc/source/parameters/dem/model_parameters.rst

@@ -53,6 +53,10 @@ In this subsection, contact detection, force models, time integration, load bala
      set granular temperature threshold  = 1e-4
      set solid fraction threshold        = 0.4
    end
+
+    # Solver type
+    # Choices are dem|cfd_dem


I'm the one who wrote this, but I'm not sure if the cfd_dem should be written in the choices since it should always be set at dem.

voferreira

I think it is all clear. It is nice to have a separate particle property handler for the CFD-DEM. Nice refactoring :)

voferreira · 2025-01-09T22:59:06Z

applications_tests/lethe-particles/restart_file_insertion.prm

@@ -31,7 +31,7 @@ end
 #---------------------------------------------------

 subsection restart
-  set checkpoint = false      # = true to generate the restart files.
+  set checkpoint = false     # = true to generate the restart files.


little spacebar here.

voferreira · 2025-01-09T23:00:48Z

contrib/preprocessing/extract-particles-properties-from-vtu.py

 with open(output_file_name, 'w') as file:
    # Write content to the file
    file.write(
-        "p_x; p_y; p_z; v_x  ; v_y; v_z; w_x; w_y; w_z; diameters; fem_force_x; fem_force_y; fem_force_z; "
-        "fem_torque_x; fem_torque_y; fem_torque_z \n")
+        "p_x; p_y; p_z; v_x  ; v_y; v_z; w_x; w_y; w_z; diameters; \n")


Suggested change

"p_x; p_y; p_z; v_x ; v_y; v_z; w_x; w_y; w_z; diameters; \n")

"p_x; p_y; p_z; v_x ; v_y; v_z; w_x; w_y; w_z; diameters; \n")

I think this would change test files, so I am not sure it is worth changing such a small typo :)

include/core/dem_properties.h

voferreira · 2025-01-09T23:04:46Z

include/core/dem_properties.h

+      omega_z      = 7,
+      mass         = 8,
+      n_properties = 9,
+


Suggested change

include/core/lethe_grid_tools.h

voferreira · 2025-01-09T23:27:25Z

include/fem-dem/vans_assemblers.h

@@ -178,8 +178,8 @@ class VANSAssemblerBDF : public NavierStokesAssemblerBase<dim>
                -(3.7 - 0.65 * exp(-pow((1.5 - log10(re)), 2) / 2)))
 *  and the momentum exchange coefficient
 *  beta =(0.5 * c_d * M_PI *
-         pow(particle_properties[DEM::PropertiesIndex::dp], 2) / 4) *
-        relative_velocity.norm()
+         pow(particle_properties[DEM::PropertiesIndex<DEM::SolverType::cfd_dem>::dp],


I am not sure, but could this be "DEM::CFDDEMProperties::PropertiesIndex::dp" instead? (Same for the other modifications in this file)

Co-authored-by: Victor Oliveira Ferreira <[email protected]>

OGaboriault and others added 11 commits January 2, 2025 16:47

Only the test remaining

dc2cd09

Test are working

19db9a5

Integrator, particle-wall, force_chains

407107d

Insertion works, doc and restart for the cfd_dem needs to be fixed

9787faf

Test are working, new parameter in the model parameter section, need …

d4b8649

…to generate new restart files

Remove template parameters from constructor

999598d

Fixed insertion_file_restart test

1075a34

Rebased tests with master

73b963b

Rebase

b4c9417

Add more comments on the dem_properties

5b2b4e0

Rebase + generated new restart files

77f2797

OGaboriault force-pushed the dem_refactor_properties_V2 branch from d91af1a to 77f2797 Compare January 2, 2025 22:22

OGaboriault added WIP When a PR is open but not ready for review Refactoring This PR is only refactoring or clean up labels Jan 2, 2025

OGaboriault changed the title ~~Dem refactor properties v2~~ Dem refactor properties Jan 3, 2025

Comments in include/dem/

15c4663

OGaboriault force-pushed the dem_refactor_properties_V2 branch from 41cc022 to 15c4663 Compare January 3, 2025 02:27

blaisb and others added 13 commits January 3, 2025 09:09

Fix compilation

b7bd1a2

Some briefs were added in the dem_properties

ca9e132

Update syntax and naming

d248bdf

Remove trailing ;

6f532bd

Fix tests to have DEM properties and not cfd_dem

c92c138

Lower version of boost archive

3b5b94d

File insertion documentation + preprocessing generator code for the i…

b284f74

…nput files.

Add new function to convert particle handlers

ad52a85

Add particle_handler_conversion and unit test

4a02d74

Add the temporary particle handler and cfd-dem

9106609

Update VANS restart files

58ba00a

Update lethe-fluid-vans results

703dc9f

Add exception check for communicator

7710eeb

OGaboriault and others added 13 commits January 6, 2025 09:40

Documenting the solver type in the .rst

75477bf

QCM Periodic works

427b534

Update QCM opposite flow test (particle switch)

54a6642

Update adaptive sparse contacts

49e44c3

Remove test that we do not have a generator

ca476a5

Fix CMakeList

d3dff77

Update the liquid fluidized bed test

a5120ae

Fix all tests now everything works

404bb66

Fix timeout

a9263aa

Got the monster to compile after the refactor

01b3ebc

Fix all unit test and indent

3ebe3c9

Changed tparam description and renamed PropertiesType to PropertiesIndex

69daace

Indent

ffaaee4

blaisb added Enhancement New feature or request Ready for review and removed WIP When a PR is open but not ready for review labels Jan 8, 2025

blaisb requested review from voferreira and OGaboriault January 8, 2025 19:12

blaisb changed the title ~~Dem refactor properties~~ Refactor the way the DEM PropertiesIndex are managed using templates Jan 8, 2025

Add CHANGELOG

ee2428d

blaisb marked this pull request as ready for review January 8, 2025 19:15

OGaboriault added 2 commits January 8, 2025 16:21

Erase the extra space after some "PropertiesIndex"

2e831c2

Indent

8fd042e

OGaboriault approved these changes Jan 9, 2025

View reviewed changes

voferreira approved these changes Jan 9, 2025

View reviewed changes

OGaboriault and others added 3 commits January 10, 2025 14:13

Applied Victor comment

0a2b395

Update include/core/lethe_grid_tools.h

eb4205e

Co-authored-by: Victor Oliveira Ferreira <[email protected]>

Update include/core/dem_properties.h

ba3a9bc

Co-authored-by: Victor Oliveira Ferreira <[email protected]>

blaisb merged commit b676be5 into master Jan 11, 2025
11 checks passed

blaisb deleted the dem_refactor_properties_V2 branch January 11, 2025 18:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor the way the DEM PropertiesIndex are managed using templates #1399

Refactor the way the DEM PropertiesIndex are managed using templates #1399

blaisb commented Jan 1, 2025 •

edited

Loading

OGaboriault Jan 8, 2025

OGaboriault Jan 8, 2025

voferreira left a comment

voferreira Jan 9, 2025

voferreira Jan 9, 2025

voferreira Jan 9, 2025

voferreira Jan 9, 2025

voferreira Jan 9, 2025


		### Changed

		- MAJOR The index of properties in the DEM and CFD-DEM simulations are now indexed using a template (PropertiesIndex) instead of using an hardcoded enum. This enables the different solvers to have different number of properties and different properties index. For large simulations, this gives minor performance gain for DEM simulations (e.g. around 5-10%), but this makes the solver significantly more flexible. This is a major change since it breaks DEM and CFD-DEM restart files from previous versions. [#1399](https://github.com/chaos-polymtl/lethe/pull/1399)

	"p_x; p_y; p_z; v_x ; v_y; v_z; w_x; w_y; w_z; diameters; \n")
	"p_x; p_y; p_z; v_x ; v_y; v_z; w_x; w_y; w_z; diameters; \n")

Refactor the way the DEM PropertiesIndex are managed using templates #1399

Refactor the way the DEM PropertiesIndex are managed using templates #1399

Conversation

blaisb commented Jan 1, 2025 • edited Loading

Description

Testing

Documentation

Miscellaneous (will be removed when merged)

Checklist (will be removed when merged)

OGaboriault Jan 8, 2025

Choose a reason for hiding this comment

OGaboriault Jan 8, 2025

Choose a reason for hiding this comment

voferreira left a comment

Choose a reason for hiding this comment

voferreira Jan 9, 2025

Choose a reason for hiding this comment

voferreira Jan 9, 2025

Choose a reason for hiding this comment

voferreira Jan 9, 2025

Choose a reason for hiding this comment

voferreira Jan 9, 2025

Choose a reason for hiding this comment

voferreira Jan 9, 2025

Choose a reason for hiding this comment

blaisb commented Jan 1, 2025 •

edited

Loading