Dem volume insertion box parameters #1074

OGaboriault · 2024-03-19T21:54:11Z

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?

voferreira

I like the new shape of the parameters. It is indeed much cleaner! The only thing I would do would be to have it coherent with dim.
Apart from that, I believe your script has failed to capture some of the points. I have pointed them out and I think I have corrected most of them. Even so, better to double check.
Lastly, check the indentations on the documentation files. I know my comments are redundant, but I did it this way so you could have checkpoints while correcting it.

voferreira · 2024-03-19T22:19:00Z

applications_tests/lethe-particles/circle_restart.prm

-  set insertion box minimum y                        = -0.07
-  set insertion box maximum x                        = 0.05
-  set insertion box maximum y                        = 0.0
+  set insertion box points coordinates               = -0.02, -0.07, 0 : 0.05, 0.0, 0


When it is 2d, should it be like this?

I think you fogot the corrective...

I will put a if constexpr (dim == 3)

voferreira · 2024-03-19T22:27:04Z

doc/source/examples/dem/packing-in-circle/packing-in-circle.rst

-      set insertion box minimum y                        = 0
-      set insertion box maximum x                        = 0.05
-      set insertion box maximum y                        = 0.07
+      set insertion box points coordinates               = -0.05, 0,  : 0.05, 0.07,


doc/source/examples/dem/rectangular-hopper/rectangular-hopper.rst

voferreira · 2024-03-19T22:29:16Z

doc/source/examples/dem/small-scale-rotating-drum/small-scale-rotating-drum.rst

-      set initial refinement = 3
-    end
-
+subsection mesh


This should be indented

voferreira · 2024-03-19T23:14:50Z

examples/unresolved-cfd-dem/gas-solid-fluidized-bed/dem-packing-in-fluidized-bed-generator.prm

-  set insertion box maximum x                        = 0.018
-  set insertion box maximum y                        = 0.05
-  set insertion box maximum z                        = 0.018
+  set insertion box points coordinates               = -0.018, -0.05, 0 : 0.018, 0.05, 0


Suggested change

set insertion box points coordinates = -0.018, -0.05, 0 : 0.018, 0.05, 0

set insertion box points coordinates = -0.018, -0.05, -0.018 : 0.018, 0.05, 0.018

voferreira · 2024-03-19T23:15:28Z

examples/unresolved-cfd-dem/gas-solid-spouted-bed/dem-packing-in-spouted-bed-generator.prm

-  set insertion box maximum x                        = 0.075
-  set insertion box maximum y                        = 0.3
-  set insertion box maximum z                        = 0.015
+  set insertion box points coordinates               = -0.075, 0.0, 0 : 0.075, 0.3, 0


Suggested change

set insertion box points coordinates = -0.075, 0.0, 0 : 0.075, 0.3, 0

set insertion box points coordinates = -0.075, 0.0, 0. : 0.075, 0.3, 0.015

voferreira · 2024-03-19T23:16:08Z

examples/unresolved-cfd-dem/liquid-solid-fluidized-bed/dem-packing-in-lsfb-generator.prm

-  set insertion box maximum x                        = 0.53
-  set insertion box maximum y                        = 0.035
-  set insertion box maximum z                        = 0.035
+  set insertion box points coordinates               = -0.15, -0.035, 0 : 0.53, 0.035, 0


Suggested change

set insertion box points coordinates = -0.15, -0.035, 0 : 0.53, 0.035, 0

set insertion box points coordinates = -0.15, -0.035, -0.035 : 0.53, 0.035, 0.035

voferreira · 2024-03-19T23:16:42Z

performance_analyses/dem/packing_10k_particles/packing_10k_particles.prm

-  set insertion box maximum x                        = 0.029
-  set insertion box maximum y                        = 0.029
-  set insertion box maximum z                        = 0.09
+  set insertion box points coordinates               = -0.029, -0.029, 0 : 0.029, 0.029, 0


Suggested change

set insertion box points coordinates = -0.029, -0.029, 0 : 0.029, 0.029, 0

set insertion box points coordinates = -0.029, -0.029, 0.01 : 0.029, 0.029, 0.09

voferreira · 2024-03-19T23:17:22Z

performance_analyses/dem/short_packing_10k_particles/packing_10kparticles_dynamic.prm

-  set insertion box maximum x                        = 0.029
-  set insertion box maximum y                        = 0.029
-  set insertion box maximum z                        = 0.09
+  set insertion box points coordinates               = -0.029, -0.029, 0 : 0.029, 0.029, 0


Suggested change

set insertion box points coordinates = -0.029, -0.029, 0 : 0.029, 0.029, 0

set insertion box points coordinates = -0.029, -0.029, 0.01 : 0.029, 0.029, 0.09

OGaboriault

Apply some comment

voferreira · 2024-03-20T14:53:05Z

Hey Gabo, wouldn't it be a good idea to do the same for the insertion velocity and omega as well?

blaisb · 2024-03-21T12:49:42Z

Hey Gabo, wouldn't it be a good idea to do the same for the insertion velocity and omega as well?

Yeah, but in a next PR I think
Step by step

blaisb

1-2 few comments. Very good work btw. Much cleaner now

blaisb · 2024-03-21T20:02:29Z

examples/dem/3d-bunny-drill/bunny-drill-loading.prm

-  set insertion first direction                      = 0
-  set insertion second direction                     = 2
-  set insertion third direction                      = 1
+  set insertion order of direction                   = 0, 2, 1


I agree... i think insertion direction sequence is a good phrasing

blaisb · 2024-03-21T20:04:00Z

tests/dem/insertion_volume_1.cc

+  dem_parameters.insertion_info.axis_0                = 0;
+  dem_parameters.insertion_info.axis_1                = 1;
+  dem_parameters.insertion_info.axis_2                = 2;


Could the axis not be like a std::vector instead of three parameters ?

blaisb · 2024-03-25T00:18:09Z

Can you rebase @OGaboriault ? Then from my POV it would be good

OGaboriault · 2024-03-25T00:37:12Z

Rebase done

blaisb · 2024-03-25T12:20:54Z

once @voferreira accepts this will be ready for merge

voferreira

All clear! Good job!

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.

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 chaos-polymtl#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

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

OGaboriault added Enhancement New feature or request Ready for review labels Mar 19, 2024

OGaboriault requested review from blaisb and voferreira March 19, 2024 21:54

OGaboriault self-assigned this Mar 19, 2024

voferreira requested changes Mar 19, 2024

View reviewed changes

OGaboriault commented Mar 20, 2024

View reviewed changes

blaisb requested changes Mar 21, 2024

View reviewed changes

OGaboriault requested a review from blaisb March 23, 2024 16:32

OGaboriault added 18 commits March 24, 2024 20:34

Work, just need to change the doc and every prm

3f3a77d

Updated every test and example .prm

480b6c6

Doc is updated

c2c9721

Update tests

c4cdf8d

Update tests

f0fe8bf

Nw parameter works and updated test

5452cc1

Update the doc

3515b1d

Fix the example prm

1ed4161

works in dim=2

f519eb9

Updated 2d cases

ca6b399

Updated cfd-dem examples

e5645b8

Update doc/example

4a2d91f

Fm packing-in-ball

6b3da95

Fm rectangular hopper

7da7d3e

FM dem

e565a67

FM cfd-dem

cd9b005

FM doc/dem

bbb2b2d

perf_analy

634e41d

OGaboriault added 4 commits March 24, 2024 20:35

Changelog + fix debug

c91c00a

Changelog + fix debug

0b60eb3

Applied Bruno's comments

8b9b8e8

Renamed parameter

7dd8e16

OGaboriault force-pushed the dem_volume_insertion_box branch from ab06cad to 7dd8e16 Compare March 25, 2024 00:36

blaisb requested a review from voferreira March 25, 2024 12:20

blaisb approved these changes Mar 25, 2024

View reviewed changes

voferreira approved these changes Mar 25, 2024

View reviewed changes

Merge branch 'master' into dem_volume_insertion_box

1299dcb

blaisb merged commit 219ca30 into master Mar 26, 2024
8 checks passed

blaisb deleted the dem_volume_insertion_box branch March 26, 2024 15:34

OGaboriault mentioned this pull request Mar 28, 2024

Dem volume insertion initial velocities #1078

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dem volume insertion box parameters #1074

Dem volume insertion box parameters #1074

OGaboriault commented Mar 19, 2024

voferreira left a comment

voferreira Mar 19, 2024

OGaboriault Mar 19, 2024

voferreira Mar 19, 2024

voferreira Mar 19, 2024

voferreira Mar 19, 2024

voferreira Mar 19, 2024

voferreira Mar 19, 2024

voferreira Mar 19, 2024

voferreira Mar 19, 2024

OGaboriault left a comment

voferreira commented Mar 20, 2024

blaisb commented Mar 21, 2024

blaisb left a comment •

edited

Loading

blaisb Mar 21, 2024

blaisb Mar 21, 2024

blaisb commented Mar 25, 2024

OGaboriault commented Mar 25, 2024

blaisb commented Mar 25, 2024

voferreira left a comment

	set insertion box points coordinates = -0.018, -0.05, 0 : 0.018, 0.05, 0
	set insertion box points coordinates = -0.018, -0.05, -0.018 : 0.018, 0.05, 0.018

Dem volume insertion box parameters #1074

Dem volume insertion box parameters #1074

Conversation

OGaboriault commented Mar 19, 2024

Description of the problem

Description of the solution

Documentation

Comments

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

OGaboriault left a comment

Choose a reason for hiding this comment

voferreira commented Mar 20, 2024

blaisb commented Mar 21, 2024

blaisb left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blaisb commented Mar 25, 2024

OGaboriault commented Mar 25, 2024

blaisb commented Mar 25, 2024

voferreira left a comment

Choose a reason for hiding this comment

blaisb left a comment •

edited

Loading