Add hemisphere shell particle cloud packing#1667
Conversation
|
I don't think this is implemented quite right. The example you added highlights this fact. It defines a hemispherical particle cloud using a cubic domain. I think instead you should add a Edit: Your |
|
Thanks Ben, that makes sense. I agree that the hemisphere shell should be treated as a particle-cloud geometry rather than a packing method. I’ll refactor this so that I’ll also remove or adjust the example so it does not imply that a cubic domain defines the hemispherical cloud. |
81e9038 to
933f66c
Compare
| do while (n_placed < particle_cloud(cloud_idx)%num_particles .and. n_attempts < max_attempts) | ||
| n_attempts = n_attempts + 1 | ||
|
|
||
| if (p == 0) then |
There was a problem hiding this comment.
num_dims < 3 peferred in general. This likely does not affect things, but num_dims is a case-optimization parameter, and therefore the compiler can optimize this away if you make it a check on num_dims.
There was a problem hiding this comment.
Updated this to use num_dims < 3 instead of checking p == 0.
| xdir = rho*cos(phi) | ||
| ydir = rho*sin(phi) | ||
| u = f_xorshift(seed) | ||
| r_shell = ((r_outer**3._wp - r_inner**3._wp)*u + r_inner**3._wp)**(1._wp/3._wp) |
There was a problem hiding this comment.
This seems like a lot fo compute for your QOI. You do not use the [xyz]dir parameter at all after computing it. Again, either use it or do not computed.
There was a problem hiding this comment.
That said, you using a probability distribution function to weight your random seed is correct here. Just you are computing redundant quantities here that seem pointless.
There was a problem hiding this comment.
-
Thanks for catching that! I completely missed that [xyz]dir wasn't being used later on. I've removed the redundant calculation to save compute.
-
Thanks for confirming the PDF logic. I've cleaned up all the redundant quantities you mentioned to optimize the compute.
933f66c to
4aff083
Compare
BCKim55
left a comment
There was a problem hiding this comment.
Thanks for the feedback. I updated the hemisphere-shell path to remove the redundant box-bounds logic and rely on the shell radii for the placement region. I also changed the dimensionality checks to use num_dims < 3 and removed the extra direction variables in the 3D sampling path.
| do while (n_placed < particle_cloud(cloud_idx)%num_particles .and. n_attempts < max_attempts) | ||
| n_attempts = n_attempts + 1 | ||
|
|
||
| if (p == 0) then |
There was a problem hiding this comment.
Updated this to use num_dims < 3 instead of checking p == 0.
| xdir = rho*cos(phi) | ||
| ydir = rho*sin(phi) | ||
| u = f_xorshift(seed) | ||
| r_shell = ((r_outer**3._wp - r_inner**3._wp)*u + r_inner**3._wp)**(1._wp/3._wp) |
There was a problem hiding this comment.
-
Thanks for catching that! I completely missed that [xyz]dir wasn't being used later on. I've removed the redundant calculation to save compute.
-
Thanks for confirming the PDF logic. I've cleaned up all the redundant quantities you mentioned to optimize the compute.
sbryngelson
left a comment
There was a problem hiding this comment.
Request changes
-
Missing regression coverage. This PR adds the hemisphere-shell execution path but removes the local example and adds no test fixture. The existing particle-cloud case is box-only, so CI never executes
geometry = 2. Please add a small deterministic 2D or 3D case that checks generated particle positions for shell/plane clearance and non-overlap. -
Documentation was not updated.
docs/documentation/case.mddoes not documentgeometry,shell_inner_radius, orshell_outer_radius, and itslength_[x,y,z]description is inaccurate forgeometry = 2, where those extents are ignored. Please update the Particle Clouds section accordingly.
Description
Adds a hemisphere-shell particle cloud packing option for immersed-boundary particle clouds.
This introduces
particle_cloud(i)%packing_method = 3, which randomly places spherical/circular IBM particles inside a hemisphere-shell region while enforcing:This also adds
shell_inner_radiusandshell_outer_radiusparticle cloud parameters. The local verification example was removed from this PR to avoid introducing a new golden test in the same change.Type of change
Testing
./mfc.sh format./mfc.sh validate examples/*/case.py./mfc.sh validate examples/3D_mibm_particle_cloud_hemi_shell/case.py./mfc.sh run examples/3D_mibm_particle_cloud_hemi_shell/case.py --clean --no-debugAdditional local checks:
ib_state_0.datparticle positions.min_spacing=0.02.min_spacing=0.0.Checklist
GPU changes (expand if you modified
src/simulation/)