For some time, we had a rather random usage of ActsScalar instead of double. This generated mismatches child classes which made it impossible to compile the project.

This PR starts to reanimate ActsScalar. For the start we try only a minimal build of Core. We provide:

a CI-job, that attempts the minimal build with ACTS_CUSTOM_SCALARTYPE=float, ignoring narrowing warnings
fixes for Core, to enable a build

In following PRs, we should aim at also building the unit tests and examples until we can actually run the tests.

Oct 17 '24 12:10 AJPfleger

@andiwand Could you maybe help me to adapt the SympyStepper? This is the only failing unit.

[ 53%] Building CXX object Core/CMakeFiles/ActsCore.dir/src/Propagator/SympyStepper.cpp.o
/__w/acts/acts/Core/src/Propagator/SympyStepper.cpp: In member function 'Acts::Result<double> Acts::SympyStepper::stepImpl(State&, Acts::Direction, double, double, std::size_t) const':
/__w/acts/acts/Core/src/Propagator/SympyStepper.cpp:151:12: error: no matching function for call to 'rk4(Eigen::PlainObjectBase<Eigen::Matrix<float, 3, 1> >::Scalar*, Eigen::PlainObjectBase<Eigen::Matrix<float, 3, 1> >::Scalar*, double&, double&, double&, double&, double&, Acts::SympyStepper::stepImpl(State&, Acts::Direction, double, double, std::size_t) const::<lambda(const double*)>&, double*, double, Eigen::MapBase<Eigen::Block<Eigen::Matrix<float, 8, 1, 0, 8, 1>, 3, 1, false>, 1>::ScalarWithConstIfNotLvalue*, Eigen::MapBase<Eigen::Block<Eigen::Matrix<float, 8, 1, 0, 8, 1>, 3, 1, false>, 1>::ScalarWithConstIfNotLvalue*, Eigen::MapBase<Eigen::Block<Eigen::Matrix<float, 8, 1, 0, 8, 1>, 1, 1, false>, 1>::ScalarWithConstIfNotLvalue*, Eigen::PlainObjectBase<Eigen::Matrix<float, 8, 1, 0, 8, 1> >::Scalar*, Eigen::PlainObjectBase<Eigen::Matrix<float, 8, 8, 0, 8, 8> >::Scalar*)'
  151 |         rk4(pos.data(), dir.data(), t, h, qop, m, p_abs, getB, &errorEstimate,
      |         ~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  152 |             4 * stepTolerance, state.pars.template segment<3>(eFreePos0).data(),
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  153 |             state.pars.template segment<3>(eFreeDir0).data(),
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  154 |             state.pars.template segment<1>(eFreeTime).data(),
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  155 |             state.derivative.data(),
      |             ~~~~~~~~~~~~~~~~~~~~~~~~
  156 |             state.covTransport ? state.jacTransport.data() : nullptr);
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /__w/acts/acts/Core/src/Propagator/SympyStepper.cpp:17:
/__w/acts/acts/Core/src/Propagator/codegen/sympy_stepper_math.hpp:19:20: note: candidate: 'template<class T, class GetB> Acts::Result<bool> rk4(const T*, const T*, T, T, T, T, T, GetB, T*, T, T*, T*, T*, T*, T*)'
   19 | Acts::Result<bool> rk4(const T* p, const T* d, const T t, const T h,
      |                    ^~~
/__w/acts/acts/Core/src/Propagator/codegen/sympy_stepper_math.hpp:19:20: note:   template argument deduction/substitution failed:
/__w/acts/acts/Core/src/Propagator/SympyStepper.cpp:151:12: note:   deduced conflicting types for parameter 'T' ('float' and 'double')
  151 |         rk4(pos.data(), dir.data(), t, h, qop, m, p_abs, getB, &errorEstimate,
      |         ~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  152 |             4 * stepTolerance, state.pars.template segment<3>(eFreePos0).data(),
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  153 |             state.pars.template segment<3>(eFreeDir0).data(),
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  154 |             state.pars.template segment<1>(eFreeTime).data(),
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  155 |             state.derivative.data(),
      |             ~~~~~~~~~~~~~~~~~~~~~~~~
  156 |             state.covTransport ? state.jacTransport.data() : nullptr);
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Oct 17 '24 13:10 AJPfleger

@andiwand Could you maybe help me to adapt the SympyStepper? This is the only failing unit.

I think the hardcoded doubles are fighting here with the Vector3 floats. This should be fixable by putting ActsScalar on the offending variables.

On a general note: I wonder if we should rather get rid of ActsScalar because for CPUs it does not make a difference if you use double or float in my experience. I rather observed cases where floats make things worse because of conversions or other CPU black magic.

I think we should use the type which makes sense which is by default double and float in cases someone has a benchmark to proof it is more optimal or for memory reasons like for the material mapping.

Oct 17 '24 13:10 andiwand

Thanks, I just read through the code generation and saw, that the relevant part is not generated by Sympy.

Oct 17 '24 14:10 AJPfleger

📊: Physics performance monitoring for 508be93aa5f7d144049fd5d31a05a9387a770a72

Full contents

physmon summary

Oct 17 '24 16:10 github-actions[bot]

Quality Gate passed

Issues
9 New issues
0 Accepted issues

Measures
0 Security Hotspots
61.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Oct 19 '24 16:10 sonarqubecloud[bot]

Like @andiwand says it might be worth thinking about if we want to retire this typedef entirely at this point, since it doesn't really serve a purpose anymore.

Oct 21 '24 08:10 paulgessinger

We will have a discussion during the workshop next week, how we should proceed on the topic ActsScalar. Currently it seems, that the cost of keeping it outweights the benefits.

Nov 11 '24 09:11 AJPfleger

Let's remove it:

https://github.com/acts-project/acts/pull/3873

Nov 19 '24 13:11 AJPfleger

fix: resurrect `ActsScalar` with `float`

📊: Physics performance monitoring for 508be93aa5f7d144049fd5d31a05a9387a770a72

physmon summary

Quality Gate passed