chaos-polymtl / lethe

Repository for the open-source lethe CFD/DEM/CFD-DEM project
https://chaos-polymtl.github.io/lethe/index.html
GNU Lesser General Public License v2.1
269 stars 59 forks source link

Introduce the DEM action manager #1206

Closed acdaigneault closed 1 month ago

acdaigneault commented 1 month ago

Description

Introducing singleton design pattern with the DEM action manager. In DEM, we often check if an action has to be performed (insertion, load balancing, contact detection, etc), and those action result in resetting containers and/or do a particle contact search. The way it was handled was turning into a spaghetti code with the addition of new features. Before, when an event was performed, a flag was set to true. And then a few places in the code was checking the flag to know if the current action has to be executed. It was getting weird because some flags where nested into others if they triggered the same actions, but the flag has lost its meaning (ex: checkpoint in load balance flag, same for the solid object mapping).

In order to overcome this,, the action manager is introduced in the DEM and CFD-DEM to manage what event triggered what. How does it works? Any event that needs some following actions are handled by the action manager. The event communicates to the action manager that it happened and the action manager will change the trigger flags to the actions to do. When this action is about to be performed, it checks with the action manager if it should be trigger. Many events can triggered the same actions, but when the action is about to be performed, it only checks if it has to be performed, not what event have happened.

Many checks in the DEM and CFDDEM solvers were moved into the called function, even though the feature might not be implemented. The first thing in the function is a check with a return if so.

Testing [WIP]

The following tests have changed because the displacement vector was not reset after a contact search for every cases: restart-gas-solid-fluidized-bed.mpirun=2.output. dynamic_contact_search.mpirun=1.output.

load_balancing_solid_mobiliity_status=2.output: it seems also be related to the contact search that is shifted by one iteration. The difference between the old results with LB and without LB balancing are more different than the difference in this PR. load_balancing_solid_mobiliity_status=2.output

Miscellaneous (will be removed when merged)

I found a few bugs-ish where checks were incoherents There's also a lot of splitting of steps into functions, it was easier to handle, but I managed to make the DEM and the CFD-DEM almost the same that way. Then, I was able to add the action manager to the CFD-DEM also, and it would help to maybe add the DEM solver into the CFDDEM solver and only use the functions of the DEM.

Bugs found, but not fixed:

Refactoring suggestions:

I did most of the Doxygen doc of the CFD-DEM, but will continue in a other PR for the variables. I still have a small detail I want to change, but it is ready for review.

Checklist (will be removed when merged)

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

Code related list:

Pull request related list:

blaisb commented 1 month ago

@acdaigneault do you know what is the origin of the two failing tests?

acdaigneault commented 1 month ago

@acdaigneault do you know what is the origin of the two failing tests?

Yeah I fixed it last night, seems I did not push it correctly