Change naming of ArborX labels

[aprokop]

@dalg24 noticed than when running ArborX integrated into an application, the performance timers for view construction/initialization/etc are using generic names like "counts". We should prefix ArborX labels. It could be as easy as prepending arborx_ to the names, or more complex like including function names to prevent overlap.

In addition, it is proposed that the region names follow the convention of using :: instead of : and naming second/third argument for the class/function they are in. For example, ArborX:BVH:spatial_queries:compute_permutation becomes ArborX::BVH::query::spatial::compute_permutation or similar.

Thoughts?

[dalg24]

Agreed. I think we should consider using the same convention for view labels.
For instance I would suggest ArborX::BVH::internal_and_leaf_nodes for

ArborX/src/ArborX_LinearBVH.hpp

Line 151 in 9a8ff40

Kokkos::ViewAllocateWithoutInitializing("internal_and_leaf_nodes"),

or ArborX::BVH::BVH::morton for

ArborX/src/ArborX_LinearBVH.hpp

Line 188 in 9a8ff40

Kokkos::ViewAllocateWithoutInitializing("morton"), size());

[aprokop]

I think we should consider using the same convention for view labels.

I would prefer starting with something simple, e.g. arborx_morton, and see if that's sufficient. We do that for parallel_* region, and it seems to work well so far. imho, it is easier to read without extra punctuation.

[dalg24]

Snippet from kernel timer on CabanaMD

- Cabana::createSteering
 (ParFor)   0.002666 36 0.000074 0.006673 0.006440
- Kokkos::View::initialization [Kokkos::SortImpl::BinSortFunctor::bin_count]
 (ParFor)   0.002647 12 0.000221 0.006625 0.006393
- Cabana::CommunicationPlan::finalCount
 (ParFor)   0.002065 36 0.000057 0.005170 0.004989
- CommMPI::halo_exchange_pack_wrap
 (ParFor)   0.001922 36 0.000053 0.004810 0.004642
- ArborX_compute_underflow
 (ParRed)   0.001767 6 0.000294 0.004422 0.004268
- Kokkos::View::destruction []
 (ParFor)   0.001696 108 0.000016 0.004244 0.004096
- Kokkos::View::initialization [pack_ranks_migrate]
 (ParFor)   0.001411 6 0.000235 0.003532 0.003409

[aprokop]

@dalg24 Could you please post a result for space-time stack? I think that one may not look as good.

[dalg24]

BEGIN KOKKOS PROFILING REPORT:
TOTAL TIME: 40.2551 seconds
TOP-DOWN TIME TREE:
<average time> <percent of total time> <percent time in Kokkos> <percent MPI imbalance> <remainder> <kernels per second> <number of calls> <name> [type]
===================
|-> 2.23e+01 sec 55.3% 100.0% 0.0% ------ 101 ForceLJCabanaNeigh::compute_full [for]
|-> 1.06e+01 sec 26.4% 96.3% 0.0% 0.1% 1.36e+01 6 ArborX:BVH:spatial_queries [region]
|   |-> 1.05e+01 sec 26.0% 96.5% 0.0% 0.0% 5.73e+00 6 ArborX:BVH:two_pass [region]
|   |   |-> 9.50e+00 sec 23.6% 100.0% 0.0% 0.0% 1.89e+00 6 ArborX:BVH:two_pass:first_pass [region]
|   |   |   |-> 9.49e+00 sec 23.6% 100.0% 0.0% ------ 6 ArborX_BVH:spatial_queries [for]
|   |   |-> 9.35e-01 sec 2.3% 59.7% 0.0% 40.3% 6.42e+00 6 ArborX:BVH:two_pass:copy_values [region]
|   |   |   |-> 5.58e-01 sec 1.4% 100.0% 0.0% ------ 6 ArborX_copy_valid_values [for]
|   |-> 1.30e-01 sec 0.3% 87.7% 0.0% 12.6% 5.07e+02 6 ArborX:BVH:spatial_queries:compute_permutation [region]
|-> 2.47e+00 sec 6.1% 100.0% 0.0% ------ 11 ForceLJCabanaNeigh::compute_half [reduce]
|-> 1.14e+00 sec 2.8% 100.0% 0.0% ------ 100 IntegratorNVE::initial_integrate [for]
|-> 6.63e-01 sec 1.6% 100.0% 0.0% ------ 100 IntegratorNVE::final_integrate [for]
|-> 6.06e-01 sec 1.5% 133.1% 0.0% 20.7% 1.27e+03 6 Comm::exchange_halo [region]
|   |-> 3.25e-01 sec 0.8% 200.0% 0.0% ------ 36 ""="" [copy]
|   |   |-> 3.25e-01 sec 0.8% 100.0% 0.0% ------ 36 Kokkos::Impl::host_space_deepcopy_double [for]
|   |-> 1.07e-01 sec 0.3% 100.0% 0.0% ------ 38 CommMPI::halo_exchange_pack [for]
|-> 5.08e-01 sec 1.3% 200.0% 0.0% ------ 101 ""="Scalar" [copy]
|   |-> 5.08e-01 sec 1.3% 100.0% 0.0% ------ 101 Kokkos::ViewFill-1D [for]
|-> 4.30e-01 sec 1.1% 99.6% 0.0% 0.0% 2.37e+02 6 ArborX:BVH:construction [region]
|   |-> 1.51e-01 sec 0.4% 99.9% 0.0% 0.1% 3.96e+01 6 ArborX:BVH:generate_hierarchy [region]
|   |   |-> 1.51e-01 sec 0.4% 100.0% 0.0% ------ 6 ArborX_generate_hierarchy [for]
|   |-> 8.45e-02 sec 0.2% 92.2% 0.0% 8.1% 7.10e+02 6 ArborX:BVH:sort_morton_codes [region]
|   |-> 8.17e-02 sec 0.2% 106.4% 0.0% 0.2% 2.20e+02 6 ArborX:BVH:calculate_bounding_volumes [region]
|   |   |-> 7.61e-02 sec 0.2% 100.0% 0.0% ------ 6 ArborX_calculate_bounding_boxes [for]
|   |-> 7.35e-02 sec 0.2% 100.0% 0.0% 0.0% 8.16e+01 6 ArborX:BVH:init_leaves [region]
|   |   |-> 7.35e-02 sec 0.2% 100.0% 0.0% ------ 6 ArborX_initialize_leaf_nodes [for]
|-> 3.79e-01 sec 0.9% 49.1% 0.0% 50.9% 4.51e+03 95 Comm::update_halo [region]
|   |-> 1.02e-01 sec 0.3% 100.0% 0.0% ------ 570 Cabana::gather::gather_send_buffer [for]
|   |-> 5.06e-02 sec 0.1% 100.0% 0.0% ------ 570 Cabana::gather::extract_recv_buffer [for]
|-> 1.70e-01 sec 0.4% 100.0% 0.0% ------ 36 Cabana::kokkosBinSort::permute_to_scratch [for]
|-> 1.23e-01 sec 0.3% 100.0% 0.0% ------ 36 Cabana::kokkosBinSort::copy_back [for]
|-> 9.62e-02 sec 0.2% 100.0% 0.0% ------ 18 Kokkos::Impl::host_space_deepcopy_double [for]

BOTTOM-UP TIME TREE:
<average time> <percent of total time> <percent time in Kokkos> <percent MPI imbalance> <number of calls> <name> [type]
===================
|-> 2.23e+01 sec 55.3% 100.0% 0.0% ------ 101 ForceLJCabanaNeigh::compute_full [for]
|-> 9.49e+00 sec 23.6% 100.0% 0.0% ------ 6 ArborX_BVH:spatial_queries [for]
|   |-> 9.49e+00 sec 23.6% 100.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:two_pass:first_pass [region]
|       |-> 9.49e+00 sec 23.6% 100.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:two_pass [region]
|           |-> 9.49e+00 sec 23.6% 100.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:spatial_queries [region]
|-> 2.47e+00 sec 6.1% 100.0% 0.0% ------ 11 ForceLJCabanaNeigh::compute_half [reduce]
|-> 1.14e+00 sec 2.8% 100.0% 0.0% ------ 100 IntegratorNVE::initial_integrate [for]
|-> 6.63e-01 sec 1.6% 100.0% 0.0% ------ 100 IntegratorNVE::final_integrate [for]
|-> 5.58e-01 sec 1.4% 100.0% 0.0% ------ 6 ArborX_copy_valid_values [for]
|   |-> 5.58e-01 sec 1.4% 100.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:two_pass:copy_values [region]
|       |-> 5.58e-01 sec 1.4% 100.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:two_pass [region]
|           |-> 5.58e-01 sec 1.4% 100.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:spatial_queries [region]
|-> 5.20e-01 sec 1.3% 100.0% 0.0% ------ 248 Kokkos::ViewFill-1D [for]
|   |-> 5.08e-01 sec 1.3% 100.0% 0.0% ------ 101 ""="Scalar" [copy]
|-> 4.31e-01 sec 1.1% 100.0% 0.0% ------ 60 Kokkos::Impl::host_space_deepcopy_double [for]
|   |-> 3.25e-01 sec 0.8% 100.0% 0.0% ------ 36 ""="" [copy]
|   |   |-> 3.25e-01 sec 0.8% 100.0% 0.0% 0.0% 0.00e+00 36 Comm::exchange_halo [region]
|-> 3.77e-01 sec 0.9% 0.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:two_pass:copy_values [region]
|   |-> 3.77e-01 sec 0.9% 0.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:two_pass [region]
|       |-> 3.77e-01 sec 0.9% 0.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:spatial_queries [region]
|-> 1.93e-01 sec 0.5% 0.0% 0.0% 0.0% 0.00e+00 95 Comm::update_halo [region]
|-> 1.70e-01 sec 0.4% 100.0% 0.0% ------ 36 Cabana::kokkosBinSort::permute_to_scratch [for]
|-> 1.51e-01 sec 0.4% 100.0% 0.0% ------ 6 ArborX_generate_hierarchy [for]
|   |-> 1.51e-01 sec 0.4% 100.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:generate_hierarchy [region]
|       |-> 1.51e-01 sec 0.4% 100.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:construction [region]
|-> 1.25e-01 sec 0.3% 0.0% 0.0% 0.0% 0.00e+00 6 Comm::exchange_halo [region]
|-> 1.23e-01 sec 0.3% 100.0% 0.0% ------ 36 Cabana::kokkosBinSort::copy_back [for]
|-> 1.10e-01 sec 0.3% 100.0% 0.0% ------ 642 Cabana::gather::gather_send_buffer [for]
|   |-> 1.02e-01 sec 0.3% 100.0% 0.0% 0.0% 0.00e+00 570 Comm::update_halo [region]
|-> 1.07e-01 sec 0.3% 100.0% 0.0% ------ 38 CommMPI::halo_exchange_pack [for]
|   |-> 1.07e-01 sec 0.3% 100.0% 0.0% 0.0% 0.00e+00 38 Comm::exchange_halo [region]
|-> 7.61e-02 sec 0.2% 100.0% 0.0% ------ 6 ArborX_calculate_bounding_boxes [for]
|   |-> 7.61e-02 sec 0.2% 100.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:calculate_bounding_volumes [region]
|       |-> 7.61e-02 sec 0.2% 100.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:construction [region]
|-> 7.35e-02 sec 0.2% 100.0% 0.0% ------ 6 ArborX_initialize_leaf_nodes [for]
|   |-> 7.35e-02 sec 0.2% 100.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:init_leaves [region]
|       |-> 7.35e-02 sec 0.2% 100.0% 0.0% 0.0% 0.00e+00 6 ArborX:BVH:construction [region]
|-> 5.64e-02 sec 0.1% 100.0% 0.0% ------ 642 Cabana::gather::extract_recv_buffer [for]
|   |-> 5.06e-02 sec 0.1% 100.0% 0.0% 0.0% 0.00e+00 570 Comm::update_halo [region]
|-> 5.46e-02 sec 0.1% 100.0% 0.0% ------ 12 Kokkos::Sort::BinBinning [for]

KOKKOS HOST SPACE:
===================
MAX MEMORY ALLOCATED: 2436576.1 kB
ALLOCATIONS AT TIME OF HIGH WATER MARK:
  32.8% ArborX:BVH:spatial_queries/ArborX:BVH:spatial_queries:init_and_alloc/indices
  25.6% ArborX:BVH:spatial_queries/ArborX:BVH:two_pass/ArborX:BVH:two_pass:copy_values/indices
  25.2% ArborX:BVH:spatial_queries/ArborX:BVH:two_pass/ArborX:BVH:two_pass:copy_values/indices
  6.0% internal_and_leaf_nodes
  2.2% Comm::exchange_halo/
  2.2% Comm::exchange_halo/
  2.2% Comm::exchange_halo/
  0.7% Comm::exchange_halo/
  0.4% Comm::exchange_halo/
  0.4% Comm::exchange_halo/
  0.4% Comm::exchange/pack_ranks_migrate
  0.3% ArborX:BVH:spatial_queries/ArborX:BVH:two_pass/ArborX:BVH:two_pass:first_pass_postprocess/offset
  0.3% ArborX:BVH:spatial_queries/ArborX:BVH:spatial_queries:init_and_alloc/offset
  0.3% ArborX:BVH:spatial_queries/ArborX:BVH:spatial_queries:init_and_alloc/offset
  0.3% ArborX:BVH:spatial_queries/ArborX:BVH:two_pass/counts
  0.3% ArborX:BVH:spatial_queries/ArborX:BVH:spatial_queries:compute_permutation/Kokkos::SortImpl::BinSortFunctor::sort_order

KOKKOS CUDA SPACE:
===================
MAX MEMORY ALLOCATED: 0.0 kB
ALLOCATIONS AT TIME OF HIGH WATER MARK:

Host process high water mark memory consumption: 3893694464 kB

END KOKKOS PROFILING REPORT.

[aprokop]

Thank you. I think I would just go with ArborX:: prefix for everything, including variables. Just use ARBORX_MARK_REGION everywhere, and set it to ArborX:: (maybe, rename it to ARBORX_LABEL. Leave labels as they are, leave loop cycles as they are, and rename regions to follow the scheme. Sounds good?

[aprokop]

We addressed all relevant issues (labels of parallel regions, profiling regions, views).