Maybe remove build warning in TrackerMap, but at least clean it up #42373

perrotta · 2023-07-25T20:48:34Z

PR description:

The possibility to return -1 from function nlayer(int det, int part, int lay) (which will never happen in practice, given its fixed input parameters) is generating build warnings in the IBs.

I imagine that if we return a non-negative number in that anyhow impossible case, those warnings can disappear.

EDIT: triggered by comment #42373 (comment) I reverted the modification of the return value in the nlayer function. I am not sure any more that the other change applied here, i.e. the loop boundaries defined as constexpr instead of being variables, will be enough to shut up the build warning: in any case I didn't want to add further useless checks in the caller code.
While doing so, I could not resist and applied a few very simple clean ups to the code. Therefore, l don't close this PR, but admittently it has a rather low priority...

PR validation:

It builds. No changes expected

cmsbuild · 2023-07-25T20:53:54Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42373/36398

This PR adds an extra 40KB to repository

cmsbuild · 2023-07-25T20:54:15Z

A new Pull Request was created by @perrotta (Andrea Perrotta) for master.

It involves the following packages:

CommonTools/TrackerMap (dqm, reconstruction)

@micsucmed, @nothingface0, @emanueleusai, @clacaputo, @cmsbuild, @pmandrik, @syuvivida, @tjavaid, @mandrenguyen, @rvenditti can you please review it and eventually sign? Thanks.
@missirol, @threus, @venturia 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

perrotta · 2023-07-25T20:58:30Z

please test

slava77 · 2023-07-25T21:47:20Z

The possibility to return -1 from function nlayer(int det, int part, int lay) (which will never happen in practice, given its fixed input parameters) is generating build warnings in the IBs.

it may help to quote the warning. Apparently the issue is in the calling code not the actual implementation here.

cmsbuild · 2023-07-25T23:28:24Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3a772b/33897/summary.html
COMMIT: 12efc98
CMSSW: CMSSW_13_3_X_2023-07-25-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/42373/33897/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 9 lines to the logs
Reco comparison results: 5 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3150117
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3150092
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 207 log files, 159 edm output root files, 48 DQM output files
TriggerResults: no differences found

emanueleusai · 2023-07-27T07:24:37Z

+1

cmsbuild · 2023-07-28T06:34:28Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42373/36416

This PR adds an extra 20KB to repository

cmsbuild · 2023-07-31T21:59:30Z

Pull request #42373 was updated. @micsucmed, @nothingface0, @emanueleusai, @clacaputo, @pmandrik, @syuvivida, @tjavaid, @mandrenguyen, @rvenditti can you please check and sign again.

cmsbuild · 2023-08-01T02:31:16Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3a772b/33996/summary.html
COMMIT: a419a13
CMSSW: CMSSW_13_3_X_2023-07-31-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42373/33996/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 17 lines from the logs
Reco comparison results: 9 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3150821
DQMHistoTests: Total failures: 8
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3150791
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 207 log files, 159 edm output root files, 48 DQM output files
TriggerResults: no differences found

mandrenguyen · 2023-08-01T05:21:19Z

+1

emanueleusai · 2023-08-01T09:16:38Z

+1

cmsbuild · 2023-08-01T09:17:02Z

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 · 2023-08-01T10:03:46Z

+1

iarspider · 2023-10-23T07:16:51Z

@perrotta @slava77 Any chance for a proper fix of this warning? Here is the log file: link

>> Compiling  /scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/tmp/BUILDROOT/b7795e3ac29b716738cd04366bdc996a/opt/cmssw/el8_ppc64le_gcc12/cms/cmssw/CMSSW_13_3_X_2023-10-22-2300/src/CommonTools/TrackerMap/src/TrackerMap.cc
CMSSW_CPU_TYPE= /scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/bin/c++ -c -DGNU_GCC -D_GNU_SOURCE -DEIGEN_DONT_PARALLELIZE -DTBB_USE_GLIBCXX_VERSION=120301 -DTBB_SUPPRESS_DEPRECATED_MESSAGES -DTBB_PREVIEW_RESUMABLE_TASKS=1 -DTBB_PREVIEW_TASK_GROUP_EXTENSIONS=1 -DBOOST_SPIRIT_THREADSAFE -DPHOENIX_THREADSAFE -DBOOST_MATH_DISABLE_STD_FPCLASSIFY -DBOOST_UUID_RANDOM_PROVIDER_FORCE_POSIX -DCMSSW_GIT_HASH='CMSSW_13_3_X_2023-10-22-2300' -DPROJECT_NAME='CMSSW' -DPROJECT_VERSION='CMSSW_13_3_X_2023-10-22-2300' -I/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/tmp/BUILDROOT/b7795e3ac29b716738cd04366bdc996a/opt/cmssw/el8_ppc64le_gcc12/cms/cmssw/CMSSW_13_3_X_2023-10-22-2300/src -I/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/pcre/8.43-37eb2e8b73bab83d6645ecfd5d73dcaa/include -isystem/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/boost/1.80.0-25dc4c664004df9af6a87cc561ac9af1/include -I/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/bz2lib/1.0.6-d065ccd79984efc6d4660f410e4c81de/include -isystem/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/clhep/2.4.7.1-5bfe601b6d65215d210a10fe9d6d7478/include -I/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/cuda/12.2.1-bdf3fff69eaec65abe18a7569592cab6/include -I/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/gsl/2.6-f316a321a173f181b66df52be79d894b/include -I/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/libuuid/2.34-27ce4c3579b5b1de2808ea9c4cd8ed29/include -isystem/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/lcg/root/6.26.11-cd568a953108f53546aa1c922b721a13/include -isystem/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/tbb/v2021.9.0-b095b6d0e5df24a3b5f9bdd21e523ee3/include -I/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/xz/5.2.5-6f3f49b07db84e10c9be594a1176c114/include -I/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/zlib/1.2.11-51072030b7f93c3ac6c4235f21e413cb/include -I/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/eigen/3bb6a48d8c171cf20b5f8e48bfb4e424fbd4f79e-8ad1a182dbaa9ab375b9d1d584a1b40f/include/eigen3 -I/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/fmt/8.0.1-54e94b39f5cf29341bb9c4765764e1ca/include -I/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/md5/1.0.0-5b594b264e04ae51e893b1d69a797ec6/include -I/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/OpenBLAS/0.3.15-15504a5c800a9ecbbc2befbb991bead5/include -I/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/el8_ppc64le_gcc12/external/tinyxml2/6.2.0-d17873b4d6a42a43226cf689f82ec1ef/include -O2 -pthread -pipe -Werror=main -Werror=pointer-arith -Werror=overlength-strings -Wno-vla -Werror=overflow -std=c++17 -ftree-vectorize -Werror=array-bounds -Werror=format-contains-nul -Werror=type-limits -fvisibility-inlines-hidden -fno-math-errno --param vect-max-version-for-alias-checks=50 -Xassembler --compress-debug-sections -Wno-error=array-bounds -Warray-bounds -fuse-ld=bfd -fsigned-char -fsigned-bitfields -mcpu=power8 -mtune=power8 --param=l1-cache-size=64 --param=l1-cache-line-size=128 --param=l2-cache-size=512 -felide-constructors -fmessage-length=0 -Wall -Wno-non-template-friend -Wno-long-long -Wreturn-type -Wextra -Wpessimizing-move -Wclass-memaccess -Wno-cast-function-type -Wno-unused-but-set-parameter -Wno-ignored-qualifiers -Wno-deprecated-copy -Wno-unused-parameter -Wunused -Wparentheses -Wno-deprecated -Werror=return-type -Werror=missing-braces -Werror=unused-value -Werror=unused-label -Werror=address -Werror=format -Werror=sign-compare -Werror=write-strings -Werror=delete-non-virtual-dtor -Werror=strict-aliasing -Werror=narrowing -Werror=unused-but-set-variable -Werror=reorder -Werror=unused-variable -Werror=conversion-null -Werror=return-local-addr -Wnon-virtual-dtor -Werror=switch -fdiagnostics-show-option -Wno-unused-local-typedefs -Wno-attributes -Wno-psabi -Wno-error=unused-variable -DBOOST_DISABLE_ASSERTS  -fPIC  -MMD -MF tmp/el8_ppc64le_gcc12/src/CommonTools/TrackerMap/src/CommonToolsTrackerMap/TrackerMap.cc.d /scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/tmp/BUILDROOT/b7795e3ac29b716738cd04366bdc996a/opt/cmssw/el8_ppc64le_gcc12/cms/cmssw/CMSSW_13_3_X_2023-10-22-2300/src/CommonTools/TrackerMap/src/TrackerMap.cc -o tmp/el8_ppc64le_gcc12/src/CommonTools/TrackerMap/src/CommonToolsTrackerMap/TrackerMap.cc.o
/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/tmp/BUILDROOT/b7795e3ac29b716738cd04366bdc996a/opt/cmssw/el8_ppc64le_gcc12/cms/cmssw/CMSSW_13_3_X_2023-10-22-2300/src/CommonTools/TrackerMap/src/TrackerMap.cc: In member function 'void TrackerMap::init()':
  /scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/tmp/BUILDROOT/b7795e3ac29b716738cd04366bdc996a/opt/cmssw/el8_ppc64le_gcc12/cms/cmssw/CMSSW_13_3_X_2023-10-22-2300/src/CommonTools/TrackerMap/src/TrackerMap.cc:498:29: warning: array subscript -2 is below array bounds of 'int [43]' [-Warray-bounds]
   498 |         ntotRing[layer_g - 1] = nrings;
      |         ~~~~~~~~~~~~~~~~~~~~^
In file included from /scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/tmp/BUILDROOT/b7795e3ac29b716738cd04366bdc996a/opt/cmssw/el8_ppc64le_gcc12/cms/cmssw/CMSSW_13_3_X_2023-10-22-2300/src/CommonTools/TrackerMap/src/TrackerMap.cc:1:
/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/tmp/BUILDROOT/b7795e3ac29b716738cd04366bdc996a/opt/cmssw/el8_ppc64le_gcc12/cms/cmssw/CMSSW_13_3_X_2023-10-22-2300/src/CommonTools/TrackerMap/interface/TrackerMap.h:643:7: note: while referencing 'TrackerMap::ntotRing'
  643 |   int ntotRing[43];
      |       ^~~~~~~~
  /scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/tmp/BUILDROOT/b7795e3ac29b716738cd04366bdc996a/opt/cmssw/el8_ppc64le_gcc12/cms/cmssw/CMSSW_13_3_X_2023-10-22-2300/src/CommonTools/TrackerMap/src/TrackerMap.cc:499:30: warning: array subscript -2 is below array bounds of 'int [43]' [-Warray-bounds]
   499 |         firstRing[layer_g - 1] = 1;
      |         ~~~~~~~~~~~~~~~~~~~~~^
/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/tmp/BUILDROOT/b7795e3ac29b716738cd04366bdc996a/opt/cmssw/el8_ppc64le_gcc12/cms/cmssw/CMSSW_13_3_X_2023-10-22-2300/src/CommonTools/TrackerMap/interface/TrackerMap.h:644:7: note: while referencing 'TrackerMap::firstRing'
  644 |   int firstRing[43];
      |       ^~~~~~~~~
  /scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/tmp/BUILDROOT/b7795e3ac29b716738cd04366bdc996a/opt/cmssw/el8_ppc64le_gcc12/cms/cmssw/CMSSW_13_3_X_2023-10-22-2300/src/CommonTools/TrackerMap/src/TrackerMap.cc:498:29: warning: array subscript -2 is below array bounds of 'int [43]' [-Warray-bounds]
   498 |         ntotRing[layer_g - 1] = nrings;
      |         ~~~~~~~~~~~~~~~~~~~~^
/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/tmp/BUILDROOT/b7795e3ac29b716738cd04366bdc996a/opt/cmssw/el8_ppc64le_gcc12/cms/cmssw/CMSSW_13_3_X_2023-10-22-2300/src/CommonTools/TrackerMap/interface/TrackerMap.h:643:7: note: while referencing 'TrackerMap::ntotRing'
  643 |   int ntotRing[43];
      |       ^~~~~~~~
  /scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/tmp/BUILDROOT/b7795e3ac29b716738cd04366bdc996a/opt/cmssw/el8_ppc64le_gcc12/cms/cmssw/CMSSW_13_3_X_2023-10-22-2300/src/CommonTools/TrackerMap/src/TrackerMap.cc:499:30: warning: array subscript -2 is below array bounds of 'int [43]' [-Warray-bounds]
   499 |         firstRing[layer_g - 1] = 1;
      |         ~~~~~~~~~~~~~~~~~~~~~^
/scratch/cmsbuild/jenkins_b/workspace/build-any-ib/w/tmp/BUILDROOT/b7795e3ac29b716738cd04366bdc996a/opt/cmssw/el8_ppc64le_gcc12/cms/cmssw/CMSSW_13_3_X_2023-10-22-2300/src/CommonTools/TrackerMap/interface/TrackerMap.h:644:7: note: while referencing 'TrackerMap::firstRing'
  644 |   int firstRing[43];
      |       ^~~~~~~~~

perrotta · 2023-10-23T07:57:47Z

@iarspider the caller code in https://github.com/cms-sw/cmssw/blob/master/CommonTools/TrackerMap/src/TrackerMap.cc#L491 looks for the correct indices, they being limited by boudaries correctly coded as constexpr. I think that the compiler complains because it tries every possible output value of the nlayer(subdet, detpart, layer) function. But the value "-1" cannot ever be outputed by construction.
If this is really the origin of the compiler warning, a possible way out could be to replace https://github.com/cms-sw/cmssw/blob/master/CommonTools/TrackerMap/interface/TrackerMap.h#L614 with something like:

    std::cout << "Error in TrackerMap: this can never happen as det and part are comprised between 1 and 3, while they are " 
                   << det << " and " << part << " respectively" << std::endl;
    return lay;

(where I'd use cout rather than LogError because cout's are already heavily used in that code, of which I don't know the actual usage inside CMSSW)

iarspider · 2023-10-23T08:50:56Z

@perrotta thanks!

slava77 · 2023-10-23T14:49:31Z

If this is really the origin of the compiler warning, a possible way out could be to replace https://github.com/cms-sw/cmssw/blob/master/CommonTools/TrackerMap/interface/TrackerMap.h#L614 with something like:

would the warning go away if we put an assert there?

mmusich · 2023-10-23T15:02:18Z

out of curiosity, can someone remind me if a recipe exists to get ppc64le builds on publicly accessible machines at CERN?

perrotta · 2023-10-23T15:07:54Z

would the warning go away if we put an assert there?

I think so...

cmsbuild added this to the CMSSW_13_3_X milestone Jul 25, 2023

cmsbuild added reconstruction-pending dqm-pending pending-signatures tests-pending orp-pending code-checks-pending trk labels Jul 25, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 25, 2023

cmsbuild added tests-started and removed tests-pending labels Jul 25, 2023

cmsbuild added tests-approved and removed tests-started labels Jul 25, 2023

cmsbuild added dqm-approved and removed dqm-pending labels Jul 27, 2023

perrotta force-pushed the removeBuildWarningInTrackerMap branch from 12efc98 to 3692913 Compare July 28, 2023 06:25

cmsbuild added dqm-pending tests-pending code-checks-pending and removed dqm-approved tests-approved code-checks-approved labels Jul 28, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 31, 2023

cmsbuild added tests-approved and removed tests-started labels Aug 1, 2023

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Aug 1, 2023

cmsbuild added dqm-approved fully-signed and removed dqm-pending pending-signatures labels Aug 1, 2023

cmsbuild added orp-approved and removed orp-pending labels Aug 1, 2023

cmsbuild merged commit 8037e53 into cms-sw:master Aug 1, 2023

perrotta deleted the removeBuildWarningInTrackerMap branch August 1, 2023 10:05

This was referenced Aug 2, 2023

[Do not merge] Test cmssw build image cms-sw/cmsdist#8626

Closed

AVXVec: replace 2*insertf128 with 1*_mm256_set_m128d #42446

Merged

fwyzard mentioned this pull request Aug 3, 2023

[GPU] Unit test testEigenGPUNoFit_t failing when testing inverse 5x5 #42460

Closed

iarspider mentioned this pull request Oct 23, 2023

Remove build warning in TrackerMap #43083

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Maybe remove build warning in TrackerMap, but at least clean it up #42373

Maybe remove build warning in TrackerMap, but at least clean it up #42373

perrotta commented Jul 25, 2023 •

edited

Loading

cmsbuild commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

perrotta commented Jul 25, 2023

slava77 commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

emanueleusai commented Jul 27, 2023

cmsbuild commented Jul 28, 2023

cmsbuild commented Jul 31, 2023

cmsbuild commented Aug 1, 2023

mandrenguyen commented Aug 1, 2023

emanueleusai commented Aug 1, 2023

cmsbuild commented Aug 1, 2023

perrotta commented Aug 1, 2023

iarspider commented Oct 23, 2023

perrotta commented Oct 23, 2023

iarspider commented Oct 23, 2023

slava77 commented Oct 23, 2023

mmusich commented Oct 23, 2023

perrotta commented Oct 23, 2023

Maybe remove build warning in TrackerMap, but at least clean it up #42373

Maybe remove build warning in TrackerMap, but at least clean it up #42373

Conversation

perrotta commented Jul 25, 2023 • edited Loading

PR description:

PR validation:

cmsbuild commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

perrotta commented Jul 25, 2023

slava77 commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

Comparison Summary

emanueleusai commented Jul 27, 2023

cmsbuild commented Jul 28, 2023

cmsbuild commented Jul 31, 2023

cmsbuild commented Aug 1, 2023

Comparison Summary

mandrenguyen commented Aug 1, 2023

emanueleusai commented Aug 1, 2023

cmsbuild commented Aug 1, 2023

perrotta commented Aug 1, 2023

iarspider commented Oct 23, 2023

perrotta commented Oct 23, 2023

iarspider commented Oct 23, 2023

slava77 commented Oct 23, 2023

mmusich commented Oct 23, 2023

perrotta commented Oct 23, 2023

perrotta commented Jul 25, 2023 •

edited

Loading