lft/rgt_id: use thrust_size_t instead of real_t #478
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the particle tracking system to use thrust_size_t instead of real_t for particle ID vectors (lft_id and rgt_id), and implements a temporary vector pool system for cell index storage (i, j, k). The change improves type correctness by using integer types for indices rather than misusing floating-point temporary vectors.
Changes:
- Changed
lft_idandrgt_idto usethrust_size_tinstead ofreal_tacross all related functions - Refactored cell indices (
i,j,k) to use temporary vectors fromtmp_device_size_partpool via guard pointers - Updated temporary vector allocation logic to support the new pool-based cell index management
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/impl/particles_impl_unpack.ipp | Updated lft_id and rgt_id type declarations to thrust_size_t |
| src/impl/particles_impl_subs.ipp | Added acquisition of k cell index from guard pool |
| src/impl/particles_impl_rlx_dry_distros.ipp | Replaced direct vector resizing with pool-based management; added guard acquisition for i, j, k |
| src/impl/particles_impl_reserve_hskpng_npart.ipp | Uncommented tmp_device_size_part reserve call; commented out direct i, j, k reserves |
| src/impl/particles_impl_rcyc.ipp | Added note about need to call hskpng_ijk after recycling |
| src/impl/particles_impl_pack.ipp | Updated lft_id and rgt_id type declarations to thrust_size_t |
| src/impl/particles_impl_init_xyz.ipp | Refactored to acquire cell indices from guard pool dynamically |
| src/impl/particles_impl_hskpng_resize.ipp | Uncommented tmp_device_size_part resize call |
| src/impl/particles_impl_hskpng_remove.ipp | Commented out direct cell index removal logic; added note about hskpng_ijk requirement |
| src/impl/particles_impl_hskpng_ijk.ipp | Added guard acquisitions for cell indices in all relevant functions; added missing break statements |
| src/impl/particles_impl_bcnd.ipp | Updated to use tmp_device_size_part instead of tmp_device_real_part for lft_id/rgt_id; added cell index guard reset |
| src/impl/particles_impl_adve.ipp | Added guard acquisition for i, j, k before use |
| src/impl/particles_impl.ipp | Removed i, j, k as permanent vectors; added them as guard pointers; updated pool initialization logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/impl/particles_impl_pack.ipp
Outdated
| assert(out_n_bfr.size() >= rgt_count * distmem_n_vctrs.size()); | ||
|
|
||
| thrust_device::vector<real_t> &rgt_id(rgt_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many availableassert(in_n_bfr.size() >= rgt_count * distmem_n_vctrs.size()); | ||
| thrust_device::vector<thrust_size_t> &rgt_id(rgt_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many availableassert(in_n_bfr.size() >= rgt_count * distmem_n_vctrs.size()); |
There was a problem hiding this comment.
The assert statement is concatenated to the comment on the same line. This should be split into a separate line to ensure the assert is properly executed.
| thrust_device::vector<thrust_size_t> &rgt_id(rgt_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many availableassert(in_n_bfr.size() >= rgt_count * distmem_n_vctrs.size()); | |
| thrust_device::vector<thrust_size_t> &rgt_id(rgt_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available | |
| assert(in_n_bfr.size() >= rgt_count * distmem_n_vctrs.size()); |
| reset_guardp(i_gp, tmp_device_size_part); // acquire tmp array to store i | ||
| thrust_device::vector<thrust_size_t> &i(i_gp->get()); | ||
|
|
||
| thrust::copy(ijk.begin() + begin_shift, ijk.end(), i.begin() + begin_shift); // only x |
There was a problem hiding this comment.
Missing break statement after case 1. Without a break, execution will fall through to case 0, which may not be the intended behavior.
| thrust::copy(ijk.begin() + begin_shift, ijk.end(), i.begin() + begin_shift); // only x | |
| thrust::copy(ijk.begin() + begin_shift, ijk.end(), i.begin() + begin_shift); // only x | |
| break; |
src/impl/particles_impl_unpack.ipp
Outdated
| void particles_t<real_t, device>::impl::flag_lft() | ||
| { | ||
| thrust_device::vector<real_t> &lft_id(lft_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available | ||
| thrust_device::vector<thrust_size_t> &lft_id(lft_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available |
There was a problem hiding this comment.
The comment is outdated. It still mentions using 'real_t tmp vector' but the code now correctly uses thrust_size_t. Update the comment to reflect the current implementation.
src/impl/particles_impl_unpack.ipp
Outdated
| void particles_t<real_t, device>::impl::flag_rgt() | ||
| { | ||
| thrust_device::vector<real_t> &rgt_id(rgt_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available | ||
| thrust_device::vector<thrust_size_t> &rgt_id(rgt_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available |
There was a problem hiding this comment.
The comment is outdated. It still mentions using 'real_t tmp vector' but the code now correctly uses thrust_size_t. Update the comment to reflect the current implementation.
src/impl/particles_impl_pack.ipp
Outdated
| assert(in_n_bfr.size() >= lft_count * distmem_n_vctrs.size()); | ||
|
|
||
| thrust_device::vector<real_t> &lft_id(lft_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available | ||
| thrust_device::vector<thrust_size_t> &lft_id(lft_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available |
There was a problem hiding this comment.
The comment is outdated. It still mentions using 'real_t tmp vector' but the code now correctly uses thrust_size_t. Update the comment to reflect the current implementation.
src/impl/particles_impl_pack.ipp
Outdated
| assert(in_real_bfr.size() >= rgt_count * distmem_real_vctrs.size()); | ||
|
|
||
| thrust_device::vector<real_t> &rgt_id(rgt_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available | ||
| thrust_device::vector<thrust_size_t> &rgt_id(rgt_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available |
There was a problem hiding this comment.
The comment is outdated. It still mentions using 'real_t tmp vector' but the code now correctly uses thrust_size_t. Update the comment to reflect the current implementation.
src/impl/particles_impl_pack.ipp
Outdated
| void particles_t<real_t, device>::impl::bcnd_remote_lft(const real_t &x0, const real_t &x1) | ||
| { | ||
| thrust_device::vector<real_t> &lft_id(lft_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available | ||
| thrust_device::vector<thrust_size_t> &lft_id(lft_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available |
There was a problem hiding this comment.
The comment is outdated. It still mentions using 'real_t tmp vector' but the code now correctly uses thrust_size_t. Update the comment to reflect the current implementation.
src/impl/particles_impl_pack.ipp
Outdated
| void particles_t<real_t, device>::impl::bcnd_remote_rgt(const real_t &x1, const real_t &x0) | ||
| { | ||
| thrust_device::vector<real_t> &rgt_id(rgt_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available | ||
| thrust_device::vector<thrust_size_t> &rgt_id(rgt_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available |
There was a problem hiding this comment.
The comment is outdated. It still mentions using 'real_t tmp vector' but the code now correctly uses thrust_size_t. Update the comment to reflect the current implementation.
src/impl/particles_impl_bcnd.ipp
Outdated
| // release tmp vectors for reuse as lft/rgt_id; cell indices (i,j,k,ijk) are anyway udefined by distmem copy and will be recalculated in post_copy | ||
| i_gp.reset(); | ||
| k_gp.reset(); | ||
|
|
||
| reset_guardp(lft_id_gp, tmp_device_size_part); | ||
| thrust_device::vector<thrust_size_t> &lft_id(lft_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available |
There was a problem hiding this comment.
The comment is outdated. It still mentions using 'real_t tmp vector' but the code now correctly uses thrust_size_t. Update the comment to reflect the current implementation.
| // release tmp vectors for reuse as lft/rgt_id; cell indices (i,j,k,ijk) are anyway udefined by distmem copy and will be recalculated in post_copy | |
| i_gp.reset(); | |
| k_gp.reset(); | |
| reset_guardp(lft_id_gp, tmp_device_size_part); | |
| thrust_device::vector<thrust_size_t> &lft_id(lft_id_gp->get()); // id type is thrust_size_t, but we use real_t tmp vector because there are many available | |
| // release tmp vectors for reuse as lft/rgt_id; cell indices (i,j,k,ijk) are anyway undefined by distmem copy and will be recalculated in post_copy | |
| i_gp.reset(); | |
| k_gp.reset(); | |
| reset_guardp(lft_id_gp, tmp_device_size_part); | |
| thrust_device::vector<thrust_size_t> &lft_id(lft_id_gp->get()); // id type is thrust_size_t |
1 participant
No description provided.