Fix `SiStripCluster::size` return type #36669

mmusich · 2022-01-11T13:35:25Z

resolves #36336
resolves #35489

PR description:

Follow up on #35489, based on suggestion at #36336 (comment).

48fbd69: Change to SiStripCluster DF, use auto for return types;

PR validation:

cmssw compiles.

cmsbuild · 2022-01-11T13:42:43Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36669/27723

This PR adds an extra 12KB to repository

cmsbuild · 2022-01-11T13:43:07Z

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

RecoTracker/TkHitPairs (reconstruction)

@jpata, @cmsbuild, @clacaputo, @slava77 can you please review it and eventually sign? Thanks.
@felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @ebrondol, @gpetruc, @mmusich, @mtosi, @dgulhan this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

clacaputo · 2022-01-13T18:38:46Z

@cmsbuild please test

cmsbuild · 2022-01-13T22:07:10Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-968ef0/21701/summary.html
COMMIT: 004dc94
CMSSW: CMSSW_12_3_X_2022-01-13-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36669/21701/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: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3461659
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3461632
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

makortel · 2022-01-13T22:24:42Z

test parameters:

enable_test = threading
workflows = 11834.21,11834.13
release = CMSSW_12_3_ASAN_X

Adding few more 11834.X workflows that we've seen crashes (#36336), and enabling threading to increase chances for the "lucky" 10th event.

makortel · 2022-01-13T22:24:49Z

@cmsbuild, please test

makortel · 2022-01-13T22:26:21Z

test parameters:

enable_test = threading
workflows = 11834.21,11834.13
release = CMSSW_12_3_ASAN_X

makortel · 2022-01-13T22:26:28Z

@cmsbuild, please test

cmsbuild · 2022-01-14T03:15:06Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-968ef0/21704/summary.html
COMMIT: 004dc94
CMSSW: CMSSW_12_3_X_2022-01-13-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36669/21704/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: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3461659
DQMHistoTests: Total failures: 12
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3461625
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

cmsbuild · 2022-01-14T13:48:42Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-968ef0/21705/summary.html
COMMIT: 004dc94
CMSSW: CMSSW_12_3_ASAN_X_2022-01-12-2300/slc7_amd64_gcc11
Additional Tests: THREADING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36669/21705/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-968ef0/11834.13_TTbar_14TeV+2021PU_mlpf+TTbar_14TeV_TuneCP5_GenSim+DigiPU+RecoNanoPU+HARVESTNanoPU
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-968ef0/11834.21_TTbar_14TeV+2021PU_ProdLike+TTbar_14TeV_TuneCP5_GenSim+DigiPU+RecoNanoPU+MiniAODPU+NanoPU

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
Reco comparison had 73 failed jobs
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3461659
DQMHistoTests: Total failures: 1063569
DQMHistoTests: Total nulls: 64
DQMHistoTests: Total successes: 2398004
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 3.065 KiB( 42 files compared)
DQMHistoSizes: changed ( 10224.0 ): -0.117 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 11834.0 ): 2.895 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 250202.181 ): 0.533 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 25202.0 ): -0.246 KiB SiStrip/MechanicalView
Checked 181 log files, 42 edm output root files, 43 DQM output files

makortel · 2022-01-14T15:13:35Z

Ok, the additional workflows did not end up in the threading test, and the NaN didn't appear there this time.

clacaputo · 2022-01-18T09:28:48Z

I'm testing a recipe for reproducing the NAN and checking the PR, proposed in #36336 (comment) by @missirol

perrotta · 2022-02-01T09:57:50Z

test parameters:

enable_test = threading
workflows = 11834.21,11834.13

perrotta · 2022-02-01T09:58:48Z

please test

DataFormats/SiStripCluster/interface/SiStripCluster.h

cmsbuild · 2022-02-01T13:47:19Z

-1

Failed Tests: UnitTests RelVals-INPUT RelVals-THREADING
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-968ef0/22117/summary.html
COMMIT: 48fbd69
CMSSW: CMSSW_12_3_X_2022-01-31-2300/slc7_amd64_gcc10
Additional Tests: THREADING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36669/22117/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test testMakeTrackValidationPlots had ERRORS

RelVals-INPUT

1040.01040.0_RunZeroBias2017F+RunZeroBias2017F+TIER0RAWSIPIXELCAL+ALCASPLITSIPIXELCAL+ALCAHARVDSIPIXELCAL/step2_RunZeroBias2017F+RunZeroBias2017F+TIER0RAWSIPIXELCAL+ALCASPLITSIPIXELCAL+ALCAHARVDSIPIXELCAL.log

RelVals-THREADING

250202.181250202.181_TTbar_13UP18+TTbar_13UP18INPUT+PREMIXUP18_PU25+DIGIPRMXLOCALUP18_PU25+RECOPRMXUP18_PU25+HARVESTUP18_PU25/step3_TTbar_13UP18+TTbar_13UP18INPUT+PREMIXUP18_PU25+DIGIPRMXLOCALUP18_PU25+RECOPRMXUP18_PU25+HARVESTUP18_PU25.log

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-968ef0/11834.13_TTbar_14TeV+2021PU_mlpf+TTbar_14TeV_TuneCP5_GenSim+DigiPU+RecoNanoPU+HARVESTNanoPU
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-968ef0/11834.21_TTbar_14TeV+2021PU_ProdLike+TTbar_14TeV_TuneCP5_GenSim+DigiPU+RecoNanoPU+MiniAODPU+NanoPU

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3449612
DQMHistoTests: Total failures: 16
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3449574
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

mmusich · 2022-02-01T13:59:11Z

I think at least #36819 is required to pass tests in threading mode

cmsbuild · 2022-02-01T14:58:06Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36669/28078

This PR adds an extra 16KB to repository

cmsbuild · 2022-02-01T14:58:33Z

Pull request #36669 was updated. @jpata, @cmsbuild, @clacaputo, @slava77 can you please check and sign again.

mmusich · 2022-02-01T15:04:11Z

mmmh, I am not sure what happened here... gitHub decided to close the PR and I cannot re-open it...

makortel · 2022-02-01T15:12:17Z

I have no clue what caused GitHub to close the PR, but after a force-push to a closed PR (as the GH shows the actions here) it is not possible to re-open it. I suggest to open a new PR.

mmusich · 2022-02-01T15:53:40Z

it is not possible to re-open it. I suggest to open a new PR.

sigh... OK here it is: #36852.
Sorry for the mess, I think that during the rebase I pushed a no-op state and gitHub interpreted this as a signal to close the PR :(

cmsbuild added this to the CMSSW_12_3_X milestone Jan 11, 2022

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jan 11, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 11, 2022

missirol mentioned this pull request Jan 12, 2022

Segfaults in workflow 11834.21 step 2 #36336

Closed

cmsbuild added tests-started and removed tests-pending labels Jan 13, 2022

cmsbuild added tests-approved and removed tests-started labels Jan 13, 2022

cmsbuild added tests-started and removed tests-approved labels Jan 13, 2022

cmsbuild added tests-approved and removed tests-started labels Jan 14, 2022

mmusich marked this pull request as ready for review January 17, 2022 12:48

cmsbuild added the tests-rejected label Jan 25, 2022

cmsbuild added tests-started and removed tests-rejected labels Feb 1, 2022

fwyzard reviewed Feb 1, 2022

View reviewed changes

DataFormats/SiStripCluster/interface/SiStripCluster.h Outdated Show resolved Hide resolved

cmsbuild added tests-rejected and removed tests-started labels Feb 1, 2022

mmusich force-pushed the bufferOverFlowRechitsSortedInPhi branch from 48fbd69 to 6eae2ee Compare February 1, 2022 14:52

cmsbuild added code-checks-pending pending-signatures reconstruction-pending tests-pending and removed reconstruction-approved fully-signed tests-rejected code-checks-approved labels Feb 1, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 1, 2022

mmusich closed this Feb 1, 2022

mmusich force-pushed the bufferOverFlowRechitsSortedInPhi branch from 6eae2ee to 9c97405 Compare February 1, 2022 14:58

perrotta mentioned this pull request Feb 1, 2022

Change to SiStripCluster DataFormat: use auto for return types #36852

Merged

clacaputo mentioned this pull request Mar 28, 2022

[12.2.X] Change to SiStripCluster DataFormat: use auto for return types #37334

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `SiStripCluster::size` return type #36669

Fix `SiStripCluster::size` return type #36669

mmusich commented Jan 11, 2022 •

edited

cmsbuild commented Jan 11, 2022

cmsbuild commented Jan 11, 2022

clacaputo commented Jan 13, 2022

cmsbuild commented Jan 13, 2022

makortel commented Jan 13, 2022

makortel commented Jan 13, 2022

makortel commented Jan 13, 2022

makortel commented Jan 13, 2022

cmsbuild commented Jan 14, 2022

cmsbuild commented Jan 14, 2022

makortel commented Jan 14, 2022

clacaputo commented Jan 18, 2022

perrotta commented Feb 1, 2022

perrotta commented Feb 1, 2022

cmsbuild commented Feb 1, 2022

mmusich commented Feb 1, 2022

cmsbuild commented Feb 1, 2022

cmsbuild commented Feb 1, 2022

mmusich commented Feb 1, 2022

makortel commented Feb 1, 2022

mmusich commented Feb 1, 2022

Fix SiStripCluster::size return type #36669

Fix SiStripCluster::size return type #36669

Conversation

mmusich commented Jan 11, 2022 • edited

PR description:

PR validation:

cmsbuild commented Jan 11, 2022

cmsbuild commented Jan 11, 2022

clacaputo commented Jan 13, 2022

cmsbuild commented Jan 13, 2022

Comparison Summary

makortel commented Jan 13, 2022

makortel commented Jan 13, 2022

makortel commented Jan 13, 2022

makortel commented Jan 13, 2022

cmsbuild commented Jan 14, 2022

Comparison Summary

cmsbuild commented Jan 14, 2022

Comparison Summary

makortel commented Jan 14, 2022

clacaputo commented Jan 18, 2022

perrotta commented Feb 1, 2022

perrotta commented Feb 1, 2022

cmsbuild commented Feb 1, 2022

Unit Tests

RelVals-INPUT

RelVals-THREADING

Comparison Summary

mmusich commented Feb 1, 2022

cmsbuild commented Feb 1, 2022

cmsbuild commented Feb 1, 2022

mmusich commented Feb 1, 2022

makortel commented Feb 1, 2022

mmusich commented Feb 1, 2022

Fix `SiStripCluster::size` return type #36669

Fix `SiStripCluster::size` return type #36669

mmusich commented Jan 11, 2022 •

edited