Scripts for evaluating code performance #57
Conversation
There was a problem hiding this comment.
Just a few non-critical suggestions that you could incorporate if you feel they are useful. There were also some Kokkos sections that I had a few questions on - particularly use of thread vector range and separating statements for creating mirror views and deep copies.
| @@ -225,6 +225,17 @@ int AerosolModelData::initChem(YAML::Node &root, | |||
| return 0; | |||
| } | |||
|
|
|||
| void AerosolModelData::setNumberofParticles(const ordinal_type number_of_particles) | |||
| { | |||
| printf("-------------------------------------------------------\n"); | |||
There was a problem hiding this comment.
We should consider using some sort of preprocessor directive for debug print statements
| // For scaling studies, we must execute this example many times. | ||
| // Thus, we do not want to write the solution to a file. | ||
| // In those cases, we pass write_time_profiles false. | ||
| bool write_time_profiles(true); |
There was a problem hiding this comment.
Perhaps we could include a preprocessor directive here as well so we only need to modify directives instead of inline code for timing runs?
| using SIMPOL_single_particle_type = TChem::Impl::SIMPOL_single_particle<real_type, device_type >; | ||
| Kokkos::parallel_for( | ||
| Kokkos::TeamThreadRange(member, amcd.nParticles), | ||
| Kokkos::ThreadVectorRange(member, amcd.nParticles), |
There was a problem hiding this comment.
Not suggesting this needs to change, but I am curious what the range policy thread vector range does instead of using team thread range in this parallel for?
| // For scaling studies, we must execute this example many times. | ||
| // Thus, we do not want to write the solution to a file. | ||
| // In those cases, we pass write_time_profiles false. | ||
| bool write_time_profiles(true); |
There was a problem hiding this comment.
Same suggestion for preprocessor directives as before
| i = amd.aerosol_sp_name_idx_.begin(); | ||
| i != amd.aerosol_sp_name_idx_.end(); ++i) | ||
| aero_idx_sp_name[i->second] = i->first; | ||
| for (ordinal_type k = 0; k < kmcd.nSpec; k++) |
There was a problem hiding this comment.
For clarity I think these two for loops should use brackets to encapsulate the iterated statements
| for (ordinal_type ipart = 0; ipart < amd.nParticles_; ipart++) | ||
| { | ||
| for (ordinal_type isp = 0; isp < amd.nSpec_; isp++) | ||
| for (ordinal_type k = 0; k < kmcd.nSpec; k++) |
There was a problem hiding this comment.
For clarity I think these two for loops should use brackets to encapsulate the iterated statements
| const ordinal_type stateVecDim = | ||
| TChem::Impl::getStateVectorSize(total_n_species); | ||
| const auto speciesNamesHost = Kokkos::create_mirror_view(kmcd.speciesNames); | ||
| Kokkos::deep_copy(speciesNamesHost, kmcd.speciesNames); |
There was a problem hiding this comment.
More of a conceptual question, but why do we create the mirror view and copy data in two separate steps instead of using Kokkos::create_mirror_view_and_copy?
…feature to modify the number of particles in the RHS and Jacobian example.
odiazib
force-pushed
the
oscar/wall_times
branch
from
66d9b41
to
a469448
Compare
No description provided.