InnerDetector clang-tidy related fixes

InnerDetector clang-tidy related fixes

WARNING: big files (>100K) are found in the changeset

156K in file InnerDetector/InDetCalibAlgs/SCT_CalibAlgs/src/SCTCalib.cxx

140K in file InnerDetector/InDetMonitoring/TRTMonitoringRun3/src/TRTMonitoringRun3RAW_Alg.cxx

112K in file InnerDetector/InDetDetDescr/TRT_GeoModel/src/TRTDetectorFactory_Full.cxx

156K in file InnerDetector/InDetRecAlgs/InDetLowBetaFinder/src/TrtToolsBetaLiklihood.cxx

112K in file InnerDetector/InDetMonitoring/InDetPerformanceMonitoring/src/IDPerfMonZmumu.cxx

This merge request affects 89 packages. Since this is a long list, it will not be printed.

Adding @cgrefe ,@jchapman ,@dshope ,@duperrin ,@kzoch ,@gavrilen ,@jsandesa ,@jajimene ,@stsuno ,@lshan ,@goetz ,@bouhova ,@amorley ,@ibragimo ,@martis ,@sroe ,@nagano ,@calfayan ,@sutt ,@battagl as watchers

added DQ Digitization ITk InnerDetector JetEtmiss Simulation Trigger analysis-review-required master review-pending-level-1 labels

CI Result SUCCESS (hash f4570463)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis	DetCommon
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 46431]

Other than a few instances of fabs needing to be changed to std::abs, the changes look good.

Jason (L1)

added review-user-action-required label and removed review-pending-level-1 label

Hi will not change std::fabs to std::abs not sure on the gain on that. fabs can be an issue so will change those.

But for floating types is a long discussion, I would prb imagine given past discussion one should update the "style" guide here. In the past I actually have attempted to change all std::fabs/fabs to std::abs (is not that difficult) but did not fly given feedback . So not 100% sure on the spirit of what we really want to "write" given actual feedback on the ground.

The actual issue(s) we have for "floating" types and this is where clang-tidy helps are indeed these these are the prints I was getting and the auto-fixes applied

warning: call to 'fabs' promotes float to double [performance-type-promotion-in-math-fn]
      r0Step = fabs(yStep)>fabs(xStep) ? rStep*m_effPrmDistance/fabs(yStep) : rStep*m_effPrmDistance/fabs(xStep);
               ^~~~
               std::fabs

/data/christos/Athena-git/athena/InnerDetector/InDetDigitization/BCM_Digitization/src/BCM_DigitizationTool.cxx:282:22: warning: call to 'exp' promotes float to double [performance-type-promotion-in-math-fn]
      effStep = 1/(1+exp((rStep-r0Step)/m_effPrmSharpness));
                     ^~~
                     std::exp

/data/christos/Athena-git/athena/InnerDetector/InDetDigitization/SCT_Digitization/src/SCT_FrontEnd.cxx:137:19: warning: call to 'sqrt' promotes float to double [performance-type-promotion-in-math-fn]
  float x1 = (A - sqrt(A)) / (2.0f * A);
                  ^~~~
                  std::sqrt

The std::abs makes some sense but also does the std::fabs here.

Using cmath and avoiding these issues btw is where one needs a "robot" as I am sure no-one believes I read 268 files ;)

added 2 commits

• e7ab128d - update copyright
• e002d527 - std::fabs

Compare with previous version

So lets wait for one CI round.

As I said changing all fabs/std::fabs to std::abs if we really really want that is a couple of sed commands and CI rounds.

The question is if we really want it as last attempt was not clear we actually want it.

WARNING: big files (>100K) are found in the changeset

156K in file InnerDetector/InDetCalibAlgs/SCT_CalibAlgs/src/SCTCalib.cxx

140K in file InnerDetector/InDetMonitoring/TRTMonitoringRun3/src/TRTMonitoringRun3RAW_Alg.cxx

112K in file InnerDetector/InDetDetDescr/TRT_GeoModel/src/TRTDetectorFactory_Full.cxx

156K in file InnerDetector/InDetRecAlgs/InDetLowBetaFinder/src/TrtToolsBetaLiklihood.cxx

112K in file InnerDetector/InDetMonitoring/InDetPerformanceMonitoring/src/IDPerfMonZmumu.cxx

This merge request affects 89 packages. Since this is a long list, it will not be printed.

Adding @cgrefe ,@jchapman ,@dshope ,@duperrin ,@kzoch ,@gavrilen ,@jsandesa ,@jajimene ,@stsuno ,@lshan ,@goetz ,@bouhova ,@amorley ,@ibragimo ,@martis ,@sroe ,@nagano ,@calfayan ,@sutt ,@battagl as watchers

added review-pending-level-1 label and removed review-user-action-required label

added 1 commit

• f9845243 - And now change all std::fabs --> std::abs

Compare with previous version

WARNING: big files (>100K) are found in the changeset

156K in file InnerDetector/InDetCalibAlgs/SCT_CalibAlgs/src/SCTCalib.cxx

140K in file InnerDetector/InDetMonitoring/TRTMonitoringRun3/src/TRTMonitoringRun3RAW_Alg.cxx

112K in file InnerDetector/InDetDetDescr/TRT_GeoModel/src/TRTDetectorFactory_Full.cxx

156K in file InnerDetector/InDetRecAlgs/InDetLowBetaFinder/src/TrtToolsBetaLiklihood.cxx

112K in file InnerDetector/InDetMonitoring/InDetPerformanceMonitoring/src/IDPerfMonZmumu.cxx

This merge request affects 89 packages. Since this is a long list, it will not be printed.

Adding @cgrefe ,@jchapman ,@dshope ,@duperrin ,@kzoch ,@gavrilen ,@jsandesa ,@jajimene ,@stsuno ,@lshan ,@goetz ,@bouhova ,@amorley ,@ibragimo ,@martis ,@sroe ,@nagano ,@calfayan ,@sutt ,@battagl as watchers

Anyhow I try again to batch replace things. As I said in the past std::fabs to std::abs received mixed feedback . Maybe will go through this time

if we really really want it is not hard to do, but has been unclear we really really want to do this .

added analysis-review-approved label and removed analysis-review-required label

CI Result SUCCESS (hash f9845243)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis	DetCommon
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 46452]

resolved all threads

Looks good. Approved

Sergei L1

added review-approved label and removed review-pending-level-1 label

Just to note that there were two CI jobs triggered: for hashes e7ab128d and e002d527, and for hash f9845243. The first job took much longer and eventually failed due to technical reasons. The second one was completed and posted results. I believe it should be OK because the latest commit has the full changeset.

mentioned in commit 0693884b

merged

added sweep:ignore label