Enable frozen points for boundaries #160
Conversation
|
@abisner tests added, mistakes fixed, and multiple sets of particles now possible |
streeve
force-pushed
the
frozen_points
branch
from
91d1a08 to
1a907b1
Compare
There was a problem hiding this comment.
Tried to give it another pretty thorough pass. Just a few minor suggestions/fixes is all. One big takeaway in createParticles() is that the localOffset should always be updated to the final resized size, whereas the frozenOffset can sometimes be updated to "point" to the same localOffset.
| return false; | ||
| }; | ||
| // Create more, starting from the current number of frozen points. | ||
| particles.createParticles( exec_space{}, init_top, particles.numFrozen() ); |
There was a problem hiding this comment.
| particles.createParticles( exec_space{}, init_top, particles.numFrozen() ); | |
| particles.createParticles( exec_space{}, init_top, particles.frozenOffset() ); |
| x = particles.sliceReferencePosition(); | ||
| rank = particles.sliceVolume(); | ||
| auto x_host = Cabana::slice<0>( aosoa_host ); | ||
| auto rank_host = Cabana::slice<1>( aosoa_host ); | ||
| Cabana::deep_copy( x_host, x ); | ||
| Cabana::deep_copy( rank_host, rank ); | ||
|
|
||
| EXPECT_EQ( particles.n_local, init_num_particles ); | ||
| EXPECT_EQ( particles.localOffset(), init_num_particles ); |
There was a problem hiding this comment.
What is this line actually testing? Do either particles.localOffset() or init_num_particles change from line 70? Are there side effects in the construction call comm( particles ) on line 82?
|
|
||
| _neigh_timer.start(); | ||
| _neigh_list.build( y, 0, n_local, Rc, 1.0, mesh_min, mesh_max ); | ||
| _neigh_list.build( y, n_frozen, n_local, Rc, 1.0, mesh_min, mesh_max ); |
There was a problem hiding this comment.
By design, this is now offset to build the neighbor lists for the local particles only, but all the particles (including the frozen ones) are still considered as potential neighbors during the construction, correct?
| // neigh_op_tag ); | ||
|
|
||
| // Forces only atomic if using team threading. | ||
| if ( std::is_same<decltype( neigh_op_tag ), Cabana::TeamOpTag>::value ) | ||
| force.computeForceFull( f_a, x, u, particles, n_local, neigh_op_tag ); | ||
| force.computeForceFull( f_a, x, u, particles, neigh_op_tag ); |
There was a problem hiding this comment.
Just a thought: the frozen particles will never accrue forces themselves, and so this is fundamentally different from an essential displacement BC, I think?
| @@ -239,7 +240,6 @@ template <class ForceType, class ParticleType, class ParallelType> | |||
| void computeForce( ForceType& force, ParticleType& particles, | |||
| const ParallelType& neigh_op_tag, const bool reset = true ) | |||
There was a problem hiding this comment.
Should the reset flag now have finer granularity as well, to distinguish between the frozen and local particles?
| @@ -496,15 +540,15 @@ class Particles<MemorySpace, PMB, TemperatureIndependent, BaseOutput, Dimension> | |||
| #ifdef Cabana_ENABLE_HDF5 | |||
| Cabana::Experimental::HDF5ParticleOutput::writeTimeStep( | |||
| h5_config, "particles", MPI_COMM_WORLD, output_step, output_time, | |||
| n_local, getPosition( use_reference ), sliceForce(), | |||
| local_offset, getPosition( use_reference ), sliceForce(), | |||
There was a problem hiding this comment.
| local_offset, getPosition( use_reference ), sliceForce(), | |
| localOffset(), getPosition( use_reference ), sliceForce(), |
| sliceDisplacement(), sliceVelocity(), | ||
| std::forward<OtherFields>( other )... ); | ||
| #else | ||
| #ifdef Cabana_ENABLE_SILO | ||
| Cabana::Grid::Experimental::SiloParticleOutput:: | ||
| writePartialRangeTimeStep( | ||
| "particles", local_grid->globalGrid(), output_step, output_time, | ||
| 0, n_local, getPosition( use_reference ), sliceForce(), | ||
| 0, local_offset, getPosition( use_reference ), sliceForce(), |
There was a problem hiding this comment.
| 0, local_offset, getPosition( use_reference ), sliceForce(), | |
| 0, localOffset(), getPosition( use_reference ), sliceForce(), |
| @@ -887,7 +917,7 @@ class Particles<MemorySpace, ModelType, ThermalType, EnergyOutput, Dimension> | |||
| void resize( int new_local, int new_ghost ) | |||
| { | |||
| base_type::resize( new_local, new_ghost ); | |||
| _aosoa_output.resize( new_local + new_ghost ); | |||
| _aosoa_output.resize( base_type::localOffset() ); | |||
There was a problem hiding this comment.
| _aosoa_output.resize( base_type::localOffset() ); | |
| _aosoa_output.resize( base_type::referenceOffset() ); |
| _aosoa_u.resize( referenceOffset() ); | ||
| _aosoa_y.resize( referenceOffset() ); | ||
| _aosoa_vol.resize( referenceOffset() ); | ||
| _plist_f.aosoa().resize( localOffset() ); |
There was a problem hiding this comment.
Is there a reason these two only use localOffset rather than referenceOffset like the rest?
| @@ -343,14 +357,16 @@ class Particles<MemorySpace, PMB, TemperatureIndependent, BaseOutput, Dimension> | |||
| memory_space, typename PositionType::execution_space>::value ); | |||
|
|
|||
| Kokkos::parallel_for( | |||
| "copy_to_particles", Kokkos::RangePolicy<ExecSpace>( 0, n_local ), | |||
| "copy_to_particles", | |||
| Kokkos::RangePolicy<ExecSpace>( num_previous, localOffset() ), | |||
There was a problem hiding this comment.
There should be an assert that num_previous does not exceed localOffset, unless this is caught by the RangePolicy? On second thought, there should also be an assert that the size of the x and vol slices is congruous with the index range (or at least as large).
Also adds particle class methods for owned, ghost, and global sizes (separate commit)