-
Notifications
You must be signed in to change notification settings - Fork 7
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 uniform surface particle initialization #81
base: master
Are you sure you want to change the base?
add uniform surface particle initialization #81
Conversation
src/Picasso_ParticleInit.hpp
Outdated
if( particles_per_facet == 3 ) | ||
px = abc_centroid[ip+1]; | ||
// ppf = 1 or 4 | ||
else |
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.
ppf = 2
is not disallowed here, or for that matter, ppf > 4
, which would lead to a runtime error.
We might want a better way to restrict to these special cases for now.
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.
Can we make this ppf
more like the particles per dimension in the interior uniform - number of particles per median (or some combination of median and space between for better coverage)? That way we don't have to restrict so many cases
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 updated here to get the style that matches the internal particle uniform init. For ppf=0
you get one on the centroid, then the intent is to add 1 per facet median for each ppf>0
, but those are wrong.
@abisner can you take a look to fix since you wrote the random version? Mostly looking for anything reasonable we can merge for now, even if we only really use the one on the centroid for now
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.
@streeve I fixed it to work with internal division points with particles_per_facet_median if this is your intention
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.
No, your recursive version is better
src/Picasso_ParticleInit.hpp
Outdated
if( particles_per_facet == 3 ) | ||
px = abc_centroid[ip+1]; | ||
// ppf = 1 or 4 | ||
else |
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.
Can we make this ppf
more like the particles per dimension in the interior uniform - number of particles per median (or some combination of median and space between for better coverage)? That way we don't have to restrict so many cases
f52d377
to
58de92a
Compare
add uniform surface particle initialization.
when ppf = 1 : create on the centroid
ppf = 3 : create on the center between centroid and vertex
ppf = 4 : ppf = 1 + ppf = 3