Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LTO] Add default initialization to fix LTO build warnings #40210

Merged
merged 1 commit into from Dec 7, 2022

Conversation

smuzaffar
Copy link
Contributor

Add the default initialization for ClusterParamGeneric data members to avoid the build warnings we are getting in the LTO IBs

  RecoLocalTracker/SiPixelRecHits/src/PixelCPEFast.cc:223:21: warning: 'cp.sx2' may be used uninitialized [-Wmaybe-uninitialized]
   223 |     g.sx2 = toMicron(cp.sx2);
      |                     ^
RecoLocalTracker/SiPixelRecHits/src/PixelCPEFast.cc:185:25: note: 'cp' declared here
  185 |     ClusterParamGeneric cp;
      |                         ^
  RecoLocalTracker/SiPixelRecHits/src/PixelCPEFast.cc:224:34: warning: 'cp.sy1' may be used uninitialized [-Wmaybe-uninitialized]
   224 |     g.sy1 = std::max(21, toMicron(cp.sy1));  // for some angles sy1 is very small
      |                                  ^
RecoLocalTracker/SiPixelRecHits/src/PixelCPEFast.cc:185:25: note: 'cp' declared here
  185 |     ClusterParamGeneric cp;
      |                         ^

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40210/33243

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2022

A new Pull Request was created by @smuzaffar (Malik Shahzad Muzaffar) for master.

It involves the following packages:

  • RecoLocalTracker/SiPixelRecHits (reconstruction)

@cmsbuild, @mandrenguyen, @clacaputo can you please review it and eventually sign? Thanks.
@mtosi, @VourMa, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @mroguljic, @missirol, @dkotlins, @ferencek, @gpetruc, @mmusich, @threus, @tvami 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

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2022

-1

Failed Tests: RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6f2265/29380/summary.html
COMMIT: 1e69f77
CMSSW: CMSSW_13_0_X_2022-11-30-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40210/29380/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

  • 5.15.1_TTbar+TTbarFS+HARVESTFS/step1_TTbar+TTbarFS+HARVESTFS.log
  • 4.224.22_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC/step2_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC.log
  • 135.4135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log
Expand to see more relval errors ...

RelVals-INPUT

  • 4.224.22_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC/step2_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC.log
  • 4.64.6_MinimumBias2010A+MinimumBias2010A+RECOSKIMALCA+HARVESTDR1/step2_MinimumBias2010A+MinimumBias2010A+RECOSKIMALCA+HARVESTDR1.log
  • 134.813134.813_RunCosmics2015C+RunCosmics2015C+RECOCOSDRUN2+ALCACOSDRUN2+HARVESTDCRUN2/step2_RunCosmics2015C+RunCosmics2015C+RECOCOSDRUN2+ALCACOSDRUN2+HARVESTDCRUN2.log
Expand to see more relval errors ...

AddOn Tests

[fastsim:1] cmsDriver.py TTbar_8TeV_TuneCUETP8M1_cfi  --conditions auto:run1_mc --fast  -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,VALIDATION  --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot Realistic8TeVCollision : FAILED - elapsed time: 3 sec (ended on Thu Dec  1 11:08:29 2022) - exit: 256
[fastsim1:1] cmsDriver.py TTbar_13TeV_TuneCUETP8M1_cfi --conditions auto:run2_mc_l1stage1 --fast  -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,VALIDATION  --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot NominalCollision2015 --era Run2_25ns : FAILED - elapsed time: 2 sec (ended on Thu Dec  1 11:08:30 2022) - exit: 256
[fastsim2:1] cmsDriver.py TTbar_13TeV_TuneCUETP8M1_cfi --conditions auto:run2_mc --fast  -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,VALIDATION  --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot NominalCollision2015 --era Run2_2016 : FAILED - elapsed time: 2 sec (ended on Thu Dec  1 11:08:34 2022) - exit: 256
Expand to see more addon errors ...

@smuzaffar
Copy link
Contributor Author

please test
new IBs with #40206 should be available now for PR tests

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2022

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6f2265/29385/summary.html
COMMIT: 1e69f77
CMSSW: CMSSW_13_0_X_2022-12-01-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40210/29385/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3421159
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3421137
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 206 log files, 158 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@tvami
Copy link
Contributor

tvami commented Dec 1, 2022

@cmsbuild , please test

@cms-sw cms-sw deleted a comment from cmsbuild Dec 4, 2022
@clacaputo
Copy link
Contributor

+reconstruction

  • technical PR
  • no reco changes

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 6, 2022

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)

@mmusich
Copy link
Contributor

mmusich commented Dec 6, 2022

hold

  • I am not sure it was really tested, the fast CPE is used only in the patatrack wfs

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 6, 2022

Pull request has been put on hold by @mmusich
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

@mmusich
Copy link
Contributor

mmusich commented Dec 7, 2022

test parameters:

  • enable_tests = gpu
  • workflows_gpu = 11634.503, 11634.506, 11634.583, 11634.587
  • workflow = 11634.501
  • relvals_opt= -w standard,highstats,pileup,generator,extendedgen,production,upgrade,cleanedupgrade,ged

@mmusich
Copy link
Contributor

mmusich commented Dec 7, 2022

@cmsbuild, please test

@VinInn
Copy link
Contributor

VinInn commented Dec 7, 2022

needless to day that this cures the symptoms not the possible bug (just hides it)

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 7, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6f2265/29495/summary.html
COMMIT: 1e69f77
CMSSW: CMSSW_13_0_X_2022-12-06-2300/el8_amd64_gcc11
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40210/29495/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: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3437220
  • DQMHistoTests: Total failures: 158
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3437040
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 210 log files, 161 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

GPU Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 7
  • DQMHistoTests: Total histograms compared: 76686
  • DQMHistoTests: Total failures: 118
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 76568
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 6 files compared)
  • Checked 24 log files, 18 edm output root files, 7 DQM output files
  • TriggerResults: no differences found

@mmusich
Copy link
Contributor

mmusich commented Dec 7, 2022

unhold

  • despite there are changes in the GPU workflows, they are never at the level of e.g. rechits, so the change proposed here should not be the cause of them.

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 7, 2022

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)

@rappoccio
Copy link
Contributor

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants