Enable -Wconversion

This commit replaces the existing `Wfloat-conversion` flag with the more
strict `-Wconversion` and fixes related errors. These were mostly
harmless integer conversions, but certainly also some nasty improper
floating point-to-integer conversions which may constitute bugs.

Author: stephenswat

benchmarks/cpu/toy_detector_cpu.cpp

@@ -70,7 +70,7 @@ BENCHMARK_F(ToyDetectorBenchmark, CPU)(benchmark::State& state) {
 
 // Iterate over events
 #pragma omp parallel for
-        for (int i_evt = 0; i_evt < n_events; i_evt++) {
+        for (unsigned int i_evt = 0; i_evt < n_events; i_evt++) {
 
             auto& spacepoints_per_event = spacepoints[i_evt];
             auto& measurements_per_event = measurements[i_evt];

benchmarks/cuda/toy_detector_cuda.cpp

@@ -96,12 +96,9 @@ BENCHMARK_F(ToyDetectorBenchmark, CUDA)(benchmark::State& state) {
 
         for (int i = -10; i < n_events; i++) {
 
-            int i_evt = i;
-
             // First 10 events are for cold run
-            if (i < 0) {
-                i_evt = 0;
-            }
+            auto i_evt = static_cast<unsigned int>(std::max(i, 0));
+
             // Measure the time after the cold run
             if (i == 0) {
                 state.ResumeTiming();
@@ -112,13 +109,15 @@ BENCHMARK_F(ToyDetectorBenchmark, CUDA)(benchmark::State& state) {
 
             // Copy the spacepoint and module data to the device.
             traccc::spacepoint_collection_types::buffer spacepoints_cuda_buffer(
-                spacepoints_per_event.size(), mr.main);
+                static_cast<unsigned int>(spacepoints_per_event.size()),
+                mr.main);
             async_copy(vecmem::get_data(spacepoints_per_event),
                        spacepoints_cuda_buffer);
 
             traccc::measurement_collection_types::buffer
-                measurements_cuda_buffer(measurements_per_event.size(),
-                                         mr.main);
+                measurements_cuda_buffer(
+                    static_cast<unsigned int>(measurements_per_event.size()),
+                    mr.main);
             async_copy(vecmem::get_data(measurements_per_event),
                        measurements_cuda_buffer);
 

cmake/traccc-compiler-options-cpp.cmake

@@ -25,7 +25,7 @@ if( ( "${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU" ) OR
    traccc_add_flag( CMAKE_CXX_FLAGS "-pedantic" )
    traccc_add_flag( CMAKE_CXX_FLAGS "-Wold-style-cast" )
    if(PROJECT_IS_TOP_LEVEL)
-     traccc_add_flag( CMAKE_CXX_FLAGS "-Wfloat-conversion" )
+     traccc_add_flag( CMAKE_CXX_FLAGS "-Wconversion" )
    endif()
 
    # Fail on warnings, if asked for that behaviour.

cmake/traccc-compiler-options-cuda.cmake

@@ -19,6 +19,10 @@ if( "${CMAKE_CXX_COMPILER_ID}" MATCHES "MSVC" )
    traccc_add_flag( CMAKE_CUDA_FLAGS "-Xcompiler /Zc:__cplusplus" )
 endif()
 
+if( "${CMAKE_CUDA_COMPILER_ID}" MATCHES "NVIDIA" )
+   traccc_add_flag( CMAKE_CUDA_FLAGS "-Wconversion" )
+endif()
+
 # Set the CUDA architecture to build code for.
 set( CMAKE_CUDA_ARCHITECTURES "52" CACHE STRING
    "CUDA architectures to build device code for" )

cmake/traccc-compiler-options-sycl.cmake

@@ -20,6 +20,7 @@ foreach( mode RELEASE RELWITHDEBINFO MINSIZEREL DEBUG )
    traccc_add_flag( CMAKE_SYCL_FLAGS_${mode} "-Wno-unknown-cuda-version" )
    traccc_add_flag( CMAKE_SYCL_FLAGS_${mode} "-Wshadow" )
    traccc_add_flag( CMAKE_SYCL_FLAGS_${mode} "-Wunused-local-typedefs" )
+   traccc_add_flag( CMAKE_SYCL_FLAGS_${mode} "-Wconversion" )
 endforeach()
 
 if( NOT WIN32 )

core/include/traccc/clusterization/clustering_config.hpp

@@ -57,21 +57,21 @@ struct clustering_config {
     /**
      * @brief The maximum number of cells per partition.
      */
-    TRACCC_HOST_DEVICE constexpr std::size_t max_partition_size() const {
+    TRACCC_HOST_DEVICE constexpr unsigned int max_partition_size() const {
         return threads_per_partition * max_cells_per_thread;
     }
 
     /**
      * @brief The target number of cells per partition.
      */
-    TRACCC_HOST_DEVICE constexpr std::size_t target_partition_size() const {
+    TRACCC_HOST_DEVICE constexpr unsigned int target_partition_size() const {
         return threads_per_partition * target_cells_per_thread;
     }
 
     /**
      * @brief The total size of the scratch space, in number of cells.
      */
-    TRACCC_HOST_DEVICE constexpr std::size_t backup_size() const {
+    TRACCC_HOST_DEVICE constexpr unsigned int backup_size() const {
         return max_partition_size() * backup_size_multiplier;
     }
 };

core/include/traccc/clusterization/impl/measurement_creation.ipp

@@ -26,9 +26,11 @@ TRACCC_HOST_DEVICE inline vector2 position_from_cell(
     const auto module_dd = det_descr.at(cell.module_index());
     // Calculate / construct the local cell position.
     return {module_dd.reference_x() +
-                (scalar{0.5} + cell.channel0()) * module_dd.pitch_x(),
+                (scalar{0.5f} + static_cast<scalar>(cell.channel0())) *
+                    module_dd.pitch_x(),
             module_dd.reference_y() +
-                (scalar{0.5} + cell.channel1()) * module_dd.pitch_y()};
+                (scalar{0.5f} + static_cast<scalar>(cell.channel1())) *
+                    module_dd.pitch_y()};
 }
 
 template <typename T>
@@ -38,7 +40,7 @@ TRACCC_HOST_DEVICE inline void calc_cluster_properties(
     const silicon_detector_description::const_device& det_descr, point2& mean,
     point2& var, scalar& totalWeight) {
 
-    point2 offset{0., 0.};
+    point2 offset{0.f, 0.f};
     bool first_processed = false;
 
     // Loop over the cell indices of the cluster.

core/include/traccc/clusterization/impl/sparse_ccl.ipp

@@ -28,7 +28,7 @@ TRACCC_HOST_DEVICE inline unsigned int make_union(
     vecmem::device_vector<unsigned int>& labels, unsigned int e1,
     unsigned int e2) {
 
-    int e;
+    unsigned int e;
     if (e1 < e2) {
         e = e1;
         assert(e2 < labels.size());

core/include/traccc/edm/internal_spacepoint.hpp

@@ -78,10 +78,10 @@ struct internal_spacepoint {
     scalar phi() const { return m_phi; }
 
     TRACCC_HOST_DEVICE
-    scalar varianceR() const { return 0.; }
+    scalar varianceR() const { return 0.f; }
 
     TRACCC_HOST_DEVICE
-    scalar varianceZ() const { return 0.; }
+    scalar varianceZ() const { return 0.f; }
 };
 
 template <typename spacepoint_t>

core/include/traccc/edm/measurement.hpp

@@ -33,9 +33,9 @@ namespace traccc {
 struct measurement {
 
     /// Local 2D coordinates for a measurement on a detector module
-    point2 local{0., 0.};
+    point2 local{0.f, 0.f};
     /// Variance on the 2D coordinates of the measurement
-    variance2 variance{0., 0.};
+    variance2 variance{0.f, 0.f};
 
     /// Geometry ID
     detray::geometry::barcode surface_link;

core/include/traccc/edm/seed.hpp

@@ -30,17 +30,19 @@ struct seed {
         const spacepoint_collection_types::const_view& spacepoints_view) const {
         const spacepoint_collection_types::const_device spacepoints(
             spacepoints_view);
-        return {spacepoints.at(spB_link).meas, spacepoints.at(spM_link).meas,
-                spacepoints.at(spT_link).meas};
+        return {spacepoints.at(static_cast<unsigned int>(spB_link)).meas,
+                spacepoints.at(static_cast<unsigned int>(spM_link)).meas,
+                spacepoints.at(static_cast<unsigned int>(spT_link)).meas};
     }
 
     TRACCC_HOST_DEVICE
     std::array<spacepoint, 3> get_spacepoints(
         const spacepoint_collection_types::const_view& spacepoints_view) const {
         const spacepoint_collection_types::const_device spacepoints(
             spacepoints_view);
-        return {spacepoints.at(spB_link), spacepoints.at(spM_link),
-                spacepoints.at(spT_link)};
+        return {spacepoints.at(static_cast<unsigned int>(spB_link)),
+                spacepoints.at(static_cast<unsigned int>(spM_link)),
+                spacepoints.at(static_cast<unsigned int>(spT_link))};
     }
 };
 

core/include/traccc/edm/spacepoint.hpp

@@ -28,7 +28,7 @@ namespace traccc {
 struct spacepoint {
 
     /// The global position of the spacepoint in 3D space
-    point3 global{0., 0., 0.};
+    point3 global{0.f, 0.f, 0.f};
     /// The local measurement of the spacepoint on the detector surface
     measurement meas;
 

core/include/traccc/finding/candidate_link.hpp

@@ -20,7 +20,7 @@ namespace traccc {
 struct candidate_link {
 
     // Type of index
-    using link_index_type = thrust::pair<int, unsigned int>;
+    using link_index_type = thrust::pair<unsigned int, unsigned int>;
 
     // Index of link from the previous step
     link_index_type previous;

core/include/traccc/finding/finding_algorithm.ipp

@@ -28,6 +28,8 @@ finding_algorithm<stepper_t, navigator_t>::operator()(
     const measurement_collection_types::host& measurements,
     const bound_track_parameters_collection_types::host& seeds) const {
 
+    assert(m_cfg.min_track_candidates_per_track > 1);
+
     /*****************************************************************
      * Measurement Operations
      *****************************************************************/
@@ -42,15 +44,16 @@ finding_algorithm<stepper_t, navigator_t>::operator()(
 
     auto end = std::unique_copy(measurements.begin(), measurements.end(),
                                 uniques.begin(), measurement_equal_comp());
-    const unsigned int n_modules = end - uniques.begin();
+    const auto n_modules = static_cast<unsigned int>(end - uniques.begin());
 
     // Get upper bounds of unique elements
     std::vector<unsigned int> upper_bounds;
     upper_bounds.reserve(n_modules);
     for (unsigned int i = 0; i < n_modules; i++) {
         auto up = std::upper_bound(measurements.begin(), measurements.end(),
                                    uniques[i], measurement_sort_comp());
-        upper_bounds.push_back(std::distance(measurements.begin(), up));
+        upper_bounds.push_back(
+            static_cast<unsigned int>(std::distance(measurements.begin(), up)));
     }
     const auto n_meas = measurements.size();
 
@@ -88,8 +91,7 @@ finding_algorithm<stepper_t, navigator_t>::operator()(
 
     std::vector<bound_track_parameters> out_params;
 
-    for (int step = 0;
-         step < static_cast<int>(m_cfg.max_track_candidates_per_track);
+    for (unsigned int step = 0; step < m_cfg.max_track_candidates_per_track;
          step++) {
 
         // Iterate over input parameters
@@ -104,8 +106,8 @@ finding_algorithm<stepper_t, navigator_t>::operator()(
         out_params.reserve(n_in_params);
 
         // Previous step ID
-        const int previous_step =
-            (step == 0) ? std::numeric_limits<int>::max() : step - 1;
+        const unsigned int previous_step =
+            (step == 0) ? std::numeric_limits<unsigned int>::max() : step - 1;
 
         std::fill(n_trks_per_seed.begin(), n_trks_per_seed.end(), 0);
 
@@ -158,13 +160,14 @@ finding_algorithm<stepper_t, navigator_t>::operator()(
 
             if (lo2 == barcodes.begin()) {
                 range.first = 0u;
-                range.second = upper_bounds[bcd_id];
+                range.second = upper_bounds[static_cast<std::size_t>(bcd_id)];
             } else if (lo2 == barcodes.end()) {
                 range.first = 0u;
                 range.second = 0u;
             } else {
-                range.first = upper_bounds[bcd_id - 1];
-                range.second = upper_bounds[bcd_id];
+                range.first =
+                    upper_bounds[static_cast<std::size_t>(bcd_id - 1)];
+                range.second = upper_bounds[static_cast<std::size_t>(bcd_id)];
             }
 
             unsigned int n_branches = 0;
@@ -223,7 +226,7 @@ finding_algorithm<stepper_t, navigator_t>::operator()(
          * Propagate to the next surface
          *********************************/
 
-        const unsigned int n_links = links[step].size();
+        const std::size_t n_links = links[step].size();
         for (unsigned int link_id = 0; link_id < n_links; link_id++) {
 
             const unsigned int seed_idx = links[step][link_id].seed_idx;
@@ -276,7 +279,7 @@ finding_algorithm<stepper_t, navigator_t>::operator()(
             // Unless the track found a surface, it is considered a
             // tip
             else if (!s4.success &&
-                     step >= static_cast<int>(
+                     step >= static_cast<unsigned int>(
                                  m_cfg.min_track_candidates_per_track) -
                                  1) {
                 tips.push_back({step, link_id});
@@ -285,7 +288,8 @@ finding_algorithm<stepper_t, navigator_t>::operator()(
             // If no more CKF step is expected, current candidate is
             // kept as a tip
             if (s4.success &&
-                step == static_cast<int>(m_cfg.max_track_candidates_per_track) -
+                step == static_cast<unsigned int>(
+                            m_cfg.max_track_candidates_per_track) -
                             1) {
                 tips.push_back({step, link_id});
             }
@@ -319,7 +323,7 @@ finding_algorithm<stepper_t, navigator_t>::operator()(
                 break;
             }
 
-            const unsigned int link_pos =
+            const unsigned long link_pos =
                 param_to_link[L.previous.first][L.previous.second];
             L = links[L.previous.first][link_pos];
         }

core/include/traccc/geometry/silicon_detector_description.hpp

@@ -222,6 +222,6 @@ using silicon_detector_description = vecmem::edm::container<
     vecmem::edm::type::vector<geometry_id>, vecmem::edm::type::vector<scalar>,
     vecmem::edm::type::vector<scalar>, vecmem::edm::type::vector<scalar>,
     vecmem::edm::type::vector<scalar>, vecmem::edm::type::vector<scalar>,
-    vecmem::edm::type::vector<char> >;
+    vecmem::edm::type::vector<unsigned char> >;
 
 }  // namespace traccc

core/include/traccc/sanity/contiguous_on.hpp

@@ -54,9 +54,11 @@ is_contiguous_on(P&& projection, const CONTAINER& in) {
     // Compress adjacent elements
     for (std::size_t i = 0; i < n; ++i) {
         if (i == 0) {
-            iout[iout_size++] = projection(in.at(i));
+            iout[iout_size++] =
+                projection(in.at(static_cast<CONTAINER::size_type>(i)));
         } else {
-            projection_t v = projection(in.at(i));
+            projection_t v =
+                projection(in.at(static_cast<CONTAINER::size_type>(i)));
 
             if (v != iout[iout_size - 1]) {
                 iout[iout_size++] = v;

core/include/traccc/seeding/detail/seeding_config.hpp

@@ -55,19 +55,19 @@ struct seedfinder_config {
     // compatible
 
     // impact parameter in mm
-    float impactMax = 10. * unit<float>::mm;
+    float impactMax = 10.f * unit<float>::mm;
     // how many sigmas of scattering angle should be considered?
-    float sigmaScattering = 3.0;
+    float sigmaScattering = 3.0f;
     // Upper pt limit for scattering calculation
-    float maxPtScattering = 10 * unit<float>::GeV;
+    float maxPtScattering = 10.f * unit<float>::GeV;
 
     // for how many seeds can one SpacePoint be the middle SpacePoint?
-    int maxSeedsPerSpM = 10;
+    unsigned int maxSeedsPerSpM = 10;
 
     float bFieldInZ = 1.99724f * unit<float>::T;
     // location of beam in x,y plane.
     // used as offset for Space Points
-    vector2 beamPos{-.0 * unit<float>::mm, -.0 * unit<float>::mm};
+    vector2 beamPos{-.0f * unit<float>::mm, -.0f * unit<float>::mm};
 
     // average radiation lengths of material on the length of a seed. used for
     // scattering.

core/include/traccc/seeding/seed_selecting_helper.hpp

@@ -72,7 +72,8 @@ struct seed_selecting_helper {
         const spacepoint_collection_t& sp_collection, const seed& seed,
         const scalar& triplet_weight) {
 
-        const auto& spB = sp_collection.at(seed.spB_link);
+        const auto& spB =
+            sp_collection.at(static_cast<unsigned int>(seed.spB_link));
 
         return (triplet_weight > filter_config.seed_min_weight ||
                 spB.radius() > filter_config.spB_min_radius);

core/include/traccc/seeding/spacepoint_binning_helper.hpp

@@ -68,8 +68,9 @@ inline std::pair<detray::axis2::circular<>, detray::axis2::regular<>> get_axes(
         // seed making step. So each individual bin should cover
         // 1/config.phiBinDeflectionCoverage of the maximum expected azimutal
         // deflection
-        scalar deltaPhi = (outerAngle - innerAngle + deltaAngleWithMaxD0) /
-                          grid_config.phiBinDeflectionCoverage;
+        scalar deltaPhi =
+            (outerAngle - innerAngle + deltaAngleWithMaxD0) /
+            static_cast<scalar>(grid_config.phiBinDeflectionCoverage);
 
         // sanity check: if the delta phi is equal to or less than zero, we'll
         // be creating an infinite or a negative number of bins, which would be
@@ -82,7 +83,8 @@ inline std::pair<detray::axis2::circular<>, detray::axis2::regular<>> get_axes(
 
         // divide 2pi by angle delta to get number of phi-bins
         // size is always 2pi even for regions of interest
-        phiBins = std::llround(2 * M_PI / deltaPhi + 0.5);
+        phiBins = static_cast<detray::dindex>(
+            std::llround(2 * M_PI / deltaPhi + 0.5));
         // need to scale the number of phi bins accordingly to the number of
         // consecutive phi bins in the seed making step.
         // Each individual bin should be approximately a fraction (depending on
@@ -99,8 +101,9 @@ inline std::pair<detray::axis2::circular<>, detray::axis2::regular<>> get_axes(
 
     scalar zBinSize = grid_config.cotThetaMax * grid_config.deltaRMax;
     detray::dindex zBins = std::max(
-        1, static_cast<int>(
-               std::floor((grid_config.zMax - grid_config.zMin) / zBinSize)));
+        static_cast<detray::dindex>(1),
+        static_cast<detray::dindex>(
+            std::floor((grid_config.zMax - grid_config.zMin) / zBinSize)));
 
     detray::axis2::regular m_z_axis{zBins, grid_config.zMin, grid_config.zMax,
                                     mr};