Fix warning in RZLine

[Dr15Jones]

PR description:

The LTO build was warning about a possible uninitialized value usage. From inspection, that is not the case but the compiler was unable to reason about the placement new usage.

• Changed to use VLA directly
• Added unit test

PR validation:

Code compiles and new unit test passes.

[Dr15Jones]

@makortel

[Dr15Jones]

please test

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40498/33688

• This PR adds an extra 12KB to repository

[cmsbuild]

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

• RecoPixelVertexing/PixelTrackFitting (reconstruction)

@mandrenguyen, @clacaputo can you please review it and eventually sign? Thanks.
@VourMa, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @missirol, @mmusich, @mtosi, @dgulhan this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

[makortel]

Resolves #39242

[cmsbuild]

-1

Failed Tests: ClangBuild
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20c51f/29955/summary.html
COMMIT: b87a0d7
CMSSW: CMSSW_13_0_X_2023-01-12-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40498/29955/install.sh to create a dev area with all the needed externals and cmssw changes.

Clang Build

I found compilation warning while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

See details on the summary page.

[slava77]

type tracking

[smuzaffar]

thanks @Dr15Jones for the fix here, can you please take care of the clang warnings https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20c51f/29955/clang-new-warnings.log

[Dr15Jones]

please test

[cmsbuild]

Pull request #40498 was updated. @mandrenguyen, @clacaputo can you please check and sign again.

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20c51f/29976/summary.html
COMMIT: 429d96a
CMSSW: CMSSW_13_0_X_2023-01-13-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40498/29976/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 1 differences found in the comparisons
• DQMHistoTests: Total files compared: 49
• DQMHistoTests: Total histograms compared: 3555538
• DQMHistoTests: Total failures: 3
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3555513
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
• Checked 211 log files, 162 edm output root files, 49 DQM output files
• TriggerResults: no differences found

[makortel]

@cms-sw/reconstruction-l2 Could you please review and sign?

[clacaputo]

+reconstruction

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

[perrotta]

@Dr15Jones , to complete the fix, and with the same logic, L41 of RZLine.h should be replaced by

    float simpleCot2 = sqr((z[nSafe - 1] - z[0]) / (r[nSafe - 1] - r[0]));

(even though a division by 0 in case of nSafe = 1 could suggest to make the calculation above only if n>1 instead)

[Dr15Jones]

@perrotta this PR is just meant to fix the new clang warning. I'd prefer any further changes be done by the maintainers of the code.

[perrotta]

@perrotta this PR is just meant to fix the new clang warning. I'd prefer any further changes be done by the maintainers of the code.

Ok. I think that what I pointed out was strictly linked to your fix, i.e. the same protection against n=0 that you add here is also needed a few lines below. However, I understand that such an additional fix can also get applied later on: let merge this PR and make clang happy, then

[perrotta]

+1

[aandvalenzuela]

Hello @perrotta @Dr15Jones ,
Could it be that those changes caused failures in https://cmssdt.cern.ch/SDT/cgi-bin/logreader/el8_aarch64_gcc11/CMSSW_13_0_X_2023-01-23-2300/unitTestLogs/RecoPixelVertexing/PixelTrackFitting#/3289-3289? I have seen the follow-up PR #40592, but I am not sure if it will also cover this error.

Thanks!
Andrea.

[perrotta]

Thank you @aandvalenzuela

It is indeed the new test introduced by this PR. It fails in the IBs for aarch64, but not for the other architectures.

Looking at the log it looks to me that it can depend on the precision of the machine: I'd let @Dr15Jones comment in a more informative way, since he designed that test.

In any case, I don't think #40592 can fix it: that PR deals with some pathological case in which either 0- or 1-dimension arrays are fed to the routine, which is not what is tested by that unit test. I am running the tests for el8_aarch64_gcc1 in that PR nonetheless, just to see what happens.

[aandvalenzuela]

Hello @perrotta,
Thank you for the explanation. Just for the records, it is also failing in ppc64le arch (https://cmssdt.cern.ch/SDT/cgi-bin/logreader/el8_ppc64le_gcc11/CMSSW_13_0_X_2023-01-23-2300/unitTestLogs/RecoPixelVertexing/PixelTrackFitting#/3643-3643).

[Dr15Jones]

So all these CPUs/compilers should be using the IEEE standard for the computations so it is worrisome that on x86 we get a value of 0.125 while on ARM/PPC we get 0.1. Those are VERY large differences.

How do I log onto our available ARM and/or PPC machines?

[smuzaffar]

after logging in to lxplus, you can run ~cmsbuild/public/lxplus to login as cmsbuild@lxplus and from there you can ssh to arm/ppc nodes

[smuzaffar]

for arm, you can also use lxplus8-arm and lxplus9-arm

[Dr15Jones]

So I 'understand' the 0.1 different from 0.125 for the different CPUs for the one point linear fit. The way the code is written, the intercept calculation is basically doing a 0*infinity computation. If I do the algebra directly for one point linear fit, I find that the we get lots of cancelation of terms and the intercept reduces to y (which in this case is 0.1).

I'm thinking about modifying the linear fit code to add a case for 1 point to make it stable.

[Dr15Jones]

See #40606 for a fix to the CPU architecture differences.