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
Add more regularity into particle statistics #2004
Add more regularity into particle statistics #2004
Conversation
22e6c03
to
77b7e6e
Compare
Since so many tests actually test the particle positions, and we have no definitive proof yet that the new method is better, I reverted the changed default value, and instead added tests for both options of the new parameter. We can decide later on which value this parameter should have. |
cmake/generate_reference_output.sh
Outdated
@@ -13,7 +13,7 @@ echo "Overwriting test output with reference output ..." | |||
SRC_PATH=`dirname $0` | |||
SRC_PATH=`cd $SRC_PATH/..;pwd` | |||
OUT=$PWD/changes.diff | |||
ASPECT_GENERATE_REFERENCE_OUTPUT=1 ctest -j 4 >/dev/null | |||
ASPECT_GENERATE_REFERENCE_OUTPUT=1 ctest -j 4 -R particle_generator_random_ >/dev/null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change on purpose?
* particle numbers per cell follow exactly the particle density, | ||
* only the locations of particles are chosen randomly. | ||
*/ | ||
bool random_cell_selection; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I understand the comment. Can you document this in terms of the algorithm behind it, e.g. "first determine how many particles each cell should have based on the integral of the density over each of the cells, and then once we know how many particles we want on each cell, choose their locations randomly within each cell."
"This means particle numbers per cell can deviate statistically from " | ||
"the integral of the probability density. If false, " | ||
"particle numbers per cell follow exactly the particle density, " | ||
"only the locations of particles are chosen randomly."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
@bangerth and @gassmoeller I'm not sure whether this is the best place to insert myself into the discussion or not, but here goes. I would like to make a few general comments. Everything below refers to random particle generation and subsequent use.
Finally, this is fun stuff. I would be delighted to participate in the design of the random parts of the particle algorithms. I have some experience in this area. In my PhD thesis I proved that a stochastic ('random') method converged to the true solution of the Kolmogorov equation. Anyway, please do include me in the discussion here. Also, @gassmoeller I would like to hear more about the seeds, especially how you are generating initial particles positions on difference processors. |
48a21af
to
f7fb793
Compare
Wolfgang I think I addressed all of your comments. About Gerry's points:
|
That sounds good ...
No, 1(a) was the algorithm as it was implemented last week; i.e., before the new implementation.
No, by 1(b) I meant what I think you just implemented; namely:
With regards to
This is just a suggestion, not something I think absolutely needs to be done. My idea was to add additional randomness to the procedure. I neglected to specify two things: (A) Just pick a cell on the current processor at random. (B) Continue to assign particles to a cell based on its area / volume as you are currently doing. (I was thinking only of cells with the same area / volume.
Again, just an idea ... |
When and why would anyone /*need*/ to start with an initially random
distribution of particles. I can imagine various scenarios, but do we
currently have a user community that has that need? I think that until we
have a specific user with a specific need for random particle generation,
it will be difficult to design the 'correct' random particle generation
algorithm, in the sense that it will converge as h -> 0 and possibly other
parameters become small or large (e.g., number of particles).
If you think of cases where you want to treat particles passively (for example
just to track where material goes), then there is really no reason to
associate particles (or the number of particles) with cells. Whether a cell is
large or small, or whether a cell already has a large number of particles or
not, is not important -- you just want particles *somewhere*. If you want to
do statistics, then choosing them randomly located within the domain,
regardless of a mesh, is the right choice.
@gassmoeller:
Though it is not distributed "over the whole domain", but rather "inside
of the local domain of a process". The global domain thing would be a tricky
implementation that might destroy the scaling of the algorithm, so I would
really want to avoid it.
I think this would actually quite simple to implement, and suspect that in
practice also won't have much of an effect on the scalability of the
algorithm. Come talk to me sometime if you're curious :)
|
f7fb793
to
e83085a
Compare
Ok, so can we conclude that the new method in this PR is a valid alternative to the old one (both are now selectable in the input file), and that we have some ideas for further alternatives at a later time?
From my side this PR is then ready to merge. |
Looks great, thanks for the work on this! |
This adds another option to the statistical particle generation. Instead of randomly choosing cells (according to their probability distribution), we assume every cell will get exactly the number of particles it should get (i.e. the integral of the probability density function over the cell times the number of particles), and only positions of particles are choosen randomly. This seems to be the preferred option (it also seems to decrease the likelihood of empty cells later in the computation), therefore I made it the new default. I am not sure if tests will fail, but the number of particles should stay constant, positions will change. @egpuckett @hlokavarapu you will be interested in this.