[DO NOT MERGE - TEST ONLY] Remove #31730 and #32021 from #31907 #32165

silviodonato · 2020-11-17T17:10:22Z

PR description:

This PR is a copy of #31907 after having removed commits

7884c35 (Backport of [HGCAL] CLUE and regional factors #31730)
02d8281 (Backport of [HGCAL] Corrections in noise vector in CLUE #32021)

The purpose of this PR is only the review of #31907

Dereferencing an uninitialized edm::Handle leads to dereference of a nullptr which is undefined behavior. Rearranged code to avoid using edm::Handles at all.

… the merged collection

…hits in the Tracker

…selection

…in EM iterations

This reverts commit 02d8281.

This reverts commit 7884c35.

cmsbuild · 2020-11-17T17:10:44Z

A new Pull Request was created by @silviodonato (Silvio Donato) for CMSSW_11_1_X.

It involves the following packages:

DataFormats/HGCalReco
RecoEgamma/EgammaTools
RecoHGCal/Configuration
RecoHGCal/TICL
RecoLocalCalo/HGCalRecAlgos
RecoLocalCalo/HGCalRecProducers
RecoParticleFlow/PFClusterProducer
Validation/HGCalValidation

@perrotta, @andrius-k, @kmaeshima, @ErnestaP, @kpedro88, @cmsbuild, @jfernan2, @fioriNTU, @slava77, @jpata can you please review it and eventually sign? Thanks.
@felicepantaleo, @jainshilpi, @argiro, @bsunanda, @pfs, @thomreis, @varuns23, @seemasharmafnal, @mmarionncern, @sethzenz, @lgray, @apsallid, @vandreev11, @Sam-Harper, @cbernet, @rovere, @cseez, @deguio, @hatakeyamak, @clelange, @rchatter, @edjtscott, @sobhatta, @lecriste, @afiqaize, @ram1123 this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

silviodonato · 2020-11-17T17:13:17Z

please test

cmsbuild · 2020-11-17T17:13:42Z

The tests are being triggered in jenkins.

CMSSW_11_1_X_2020-11-17-1100/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/10809/console Started: 2020/11/17 18:16

rovere · 2020-11-17T17:30:16Z

Thanks @silviodonato for this.
I briefly spoke with Andrea that alerted me about that.

cmsbuild · 2020-11-17T18:50:33Z

+1
Tested at: f9d0e3f
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d355e5/10809/summary.html
CMSSW: CMSSW_11_1_X_2020-11-17-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-11-17T18:50:36Z

Comparison job queued.

cmsbuild · 2020-11-17T20:39:52Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d355e5/10809/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 2552 differences found in the comparisons
DQMHistoTests: Total files compared: 36
DQMHistoTests: Total histograms compared: 2784828
DQMHistoTests: Total failures: 6002
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2778776
DQMHistoTests: Total skipped: 50
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
Checked 152 log files, 16 edm output root files, 36 DQM output files

jfernan2 · 2020-11-18T08:20:07Z

It seems only change sin EGamma and HGCAL folders are produced:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_11_1_X_2020-11-17-1100+d355e5/39938/dqm-histo-comparison-summary.html

rovere · 2020-11-18T08:49:20Z

Dear all,
this confirms the analysis I've posted in the original PR.
@silviodonato should we start the review process of #31907 or do you have additional checks you'd like to perform before doing that?
By the way, since the code changes in #31907 have been all reviewed, in single PRs, in 11_2, I imagine the process could be rather smooth.
Let me know how you would like to proceed. I'll be happy to help if it's needed.

perrotta · 2020-11-18T12:49:53Z

By the way, since the code changes in #31907 have been all reviewed, in single PRs, in 11_2, I imagine the process could be rather smooth.

This check was great, and certainly helps in the review of #31907. I'll do my best to go through it as quicly as possible. Please however take into account that #31907 includes the backport of 9 different PRs merged in the master, and for a few of them that backport is not even complete. In the masters there were other HGCal related PRs merged which were not included in that backport, and this also must be taken care of while comparing the codes in the two releases, I commit myself in speeding up the review, but do not expect it can be concluded too soon.

rovere · 2020-11-18T13:45:45Z

By the way, since the code changes in #31907 have been all reviewed, in single PRs, in 11_2, I imagine the process could be rather smooth.

This check was great, and certainly helps in the review of #31907. I'll do my best to go through it as quicly as possible. Please however take into account that #31907 includes the backport of 9 different PRs merged in the master, and for a few of them that backport is not even complete. In the masters there were other HGCal related PRs merged which were not included in that backport, and this also must be taken care of while comparing the codes in the two releases, I commit myself in speeding up the review, but do not expect it can be concluded too soon.

Thanks a lot @perrotta
In doing the backport, I included the very bare minimum to avoid the inclusion of un-necessary code.
I'll be glad to help the review process in all possible ways.

mariadalfonso and others added 30 commits November 15, 2020 21:15

Backport of cms-sw#29837, w/o Configuration/StandardSequences changes

e0fd35e

Backport cms-sw#30007

48fec35

Backport of cms-sw#30815

e46e054

Backport cms-sw#31031

8f2f836

Dereferencing an uninitialized edm::Handle leads to dereference of a nullptr which is undefined behavior. Rearranged code to avoid using edm::Handles at all.

Partial backport of cms-sw#31013

15b625e

Created a ClusterFilter by Algo, Size and Layer Range

e46b82f

include an Electromagnetic TrackSeeded iteration as first iteration

0f4894b

Add the range of layers to the filter parameters

f787aa2

Inject tracksters from the track seeded electromagnetic interation in…

b9ab9e7

… the merged collection

create empty multiclusters

4347004

Add TrkEM tracksters into the Merge collection

26aa9f5

Select trackster-PID also on EM/Total trackster energy ratio

bc34f00

Fix merge conflict.

4f31467

Fix input collection for TRK iteration to be the output of EM iteration

8ff5c2d

Sorting seeding regions by track momentum

ff3a586

Use only clusters with minimum size of 3

d928499

Filter only with a maxLayerId in TRKEM iteration.

70c6b79

Remove MIP iteration

58aea39

clang format

88b0d1f

Limit Global EM iteration to layer 30

12b8e60

Limit track seeded iterations to tracks with at most 2 missing outer …

621b988

…hits in the Tracker

Add TrkEM products to EventContent

3ededa0

Remove missing layers cut for EM based iterations

c14afde

Introducing a shower_start_max_layer cut

87e3a55

raising shower_start_max_layer from 3 to 7 in EM iterations

5bcbd52

Count usage of layer clusters, only for those tracksters passing the …

3ea65f4

…selection

Increasing missing outer hits from 3 to 5

c631951

increase missing layer from 0 to 1 and loosen min_cos_theta to 0.961 …

8b6864f

…in EM iterations

tighten min_cos_theta from 0.961 to 0.978 in EM iterations

a62c80e

Rename parameter missing_layers to skip_layers.

b4236de

silviodonato added 3 commits November 17, 2020 17:57

Merged TICLv2_11_1_X from repository rovere with cms-merge-topic

76c704b

Revert "Backport of cms-sw#32021"

4d19703

This reverts commit 02d8281.

Revert "Backport of cms-sw#31730"

f9d0e3f

This reverts commit 7884c35.

cmsbuild added this to the CMSSW_11_1_X milestone Nov 17, 2020

cmsbuild added comparison-pending dqm-pending orp-pending pending-signatures reconstruction-pending tests-pending upgrade-pending labels Nov 17, 2020

silviodonato mentioned this pull request Nov 17, 2020

TICL reconstruction update #31907

Merged

cmsbuild added tests-started and removed tests-pending labels Nov 17, 2020

cmsbuild added tests-approved and removed tests-started labels Nov 17, 2020

cmsbuild added comparison-available and removed comparison-pending labels Nov 17, 2020

silviodonato closed this Nov 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DO NOT MERGE - TEST ONLY] Remove #31730 and #32021 from #31907 #32165

[DO NOT MERGE - TEST ONLY] Remove #31730 and #32021 from #31907 #32165

Uh oh!

silviodonato commented Nov 17, 2020

Uh oh!

cmsbuild commented Nov 17, 2020

Uh oh!

silviodonato commented Nov 17, 2020

Uh oh!

cmsbuild commented Nov 17, 2020 •

edited

Loading

Uh oh!

rovere commented Nov 17, 2020

Uh oh!

cmsbuild commented Nov 17, 2020

Uh oh!

cmsbuild commented Nov 17, 2020

Uh oh!

cmsbuild commented Nov 17, 2020

Uh oh!

jfernan2 commented Nov 18, 2020

Uh oh!

rovere commented Nov 18, 2020

Uh oh!

perrotta commented Nov 18, 2020

Uh oh!

rovere commented Nov 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

[DO NOT MERGE - TEST ONLY] Remove #31730 and #32021 from #31907 #32165

[DO NOT MERGE - TEST ONLY] Remove #31730 and #32021 from #31907 #32165

Uh oh!

Conversation

silviodonato commented Nov 17, 2020

PR description:

Uh oh!

cmsbuild commented Nov 17, 2020

Uh oh!

silviodonato commented Nov 17, 2020

Uh oh!

cmsbuild commented Nov 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rovere commented Nov 17, 2020

Uh oh!

cmsbuild commented Nov 17, 2020

Uh oh!

cmsbuild commented Nov 17, 2020

Uh oh!

cmsbuild commented Nov 17, 2020

Uh oh!

jfernan2 commented Nov 18, 2020

Uh oh!

rovere commented Nov 18, 2020

Uh oh!

perrotta commented Nov 18, 2020

Uh oh!

rovere commented Nov 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

cmsbuild commented Nov 17, 2020 •

edited

Loading