Resolved uninitialized value warning in PixelTrackFitting #33881
Conversation
…Fitting.
This resolves the warning:
Eigen/src/Core/functors/AssignmentFunctors.h:92:62: warning: '*((void*)& p3D +88)' is used uninitialized in this function [-Wuninitialized]
92 | EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE void assignCoeff(DstScalar& a, const SrcScalar& b) const { a *= b; }
| ~~^~~~
The last row of the matrix is not initialized yet, and gets initialized right after.
|
@rovere could you double check? |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33881/22917
|
|
A new Pull Request was created by @ericcano (Eric Cano) for master. It involves the following packages: RecoPixelVertexing/PixelTrackFitting @perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
enable gpu |
|
please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a4563d/15414/summary.html GPU Comparison SummarySummary:
Comparison SummarySummary:
|
|
Ciao @ericcano |
| @@ -515,7 +515,7 @@ namespace riemannFit { | |||
| // scale | |||
| const double tempQ = mc.squaredNorm(); | |||
| const double tempS = sqrt(n * 1. / tempQ); // scaling factor | |||
| p3D *= tempS; | |||
| p3D.block(0, 0, 2, n) *= tempS; | |||
There was a problem hiding this comment.
was the older code running, BTW?
If I understood the warning correctly, was there supposed to be a crash here before this fix?
There was a problem hiding this comment.
Older code was running fine.
It was not supposed to crash. We simply scaled uninitialized values in place in the last row of the matrix just to overwrite them on the next statement. It was totally harmless (besides wasting n multiplications, and being the last compilation warning to clear).
|
+reconstruction
|
cmsbuild
added
fully-signed
reconstruction-approved
and removed
pending-signatures
reconstruction-pending
labels
|
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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
This resolves the warning:
The last row of the matrix is not initialized yet, and gets initialized right after.
PR validation:
The fix is limited and well understood.
scram b runtestssucceeds except for 5x5 matrices intestEigenGPUNoFit_t, and this problem is already followed up in #33797.