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

HCalTopology revision of HE-HF and HB-HE boundaries for |ieta|=16 and 29 #31384

Merged
merged 3 commits into from Sep 14, 2020

Conversation

hatakeyamak
Copy link
Contributor

@hatakeyamak hatakeyamak commented Sep 7, 2020

PR description:

Update HcalTopology for the ieta neighbor search of |ieta|=16 and 29, i.e. HB-HE boundary and HE-HF boundaries. These two |ieta| values correspind to both HB&HE towers and HE&HF towers. With this update, we specify different behaviors depending on if the tower belongs to HB or HE or to HE or HF [1].

The update will have implications on CaloTopoogy used in PF's navigator for HCAL.
https://cmssdt.cern.ch/lxr/source/RecoParticleFlow/PFClusterProducer/interface/PFHCALDenseIdNavigator.h
Before this update, some 2D layer-wise clusters consisted of rechits of different depths [2]. This will no longer happen with this update [3].

This unexpected behavior was attributed to the HcalTopology behavior as discussed above. In case one is interested in further details: for example, when we are looking for a lower ieta tower of HE ieta=29, we will want to simply decrease ieta by 1 while keeping depth. But, currently, regardless of HF or HE, ieta=29 invokes the special neighbor search which sets the neighbors’s depth to 1. Similarly, for example, when we are looking for the higher ieta for HF ieta=29, we will want to simply increase ieta by 1 while keeping depth. However, currently, regardless of HE or HF, ieta=29 invokes the special neighbor search which sets the neighbor’s depth to 1. These resulted in 2D layer-wise clusters with mixed depth before this PR.

We consulted with @bsunanda (primary author of HcalTopolgy) before this PR was made.

[1] for example, https://cms-docdb.cern.ch/cgi-bin/DocDB/RetrieveFile?docid=12962&filename=HCAL-Depth-Segmentation-Phase1-HE-HBOption1.pdf&version=6
[2] https://indico.cern.ch/event/949951/#12-studies-on-pf-clusters-in-h page-4, 18, 19
[3] https://indico.cern.ch/event/949951/contributions/3991032/attachments/2092837/3527202/plot_PFclusterSize_HFHAD.pdf

@bendavid @bsunanda @abdoulline @garvitaa @iashvili @jsalfeld

PR validation:

The validation is already discussed above. Also checked this update with matrix: 12634.99_TTbar_14TeV (2023PU) and 23434.99_TTbar (2026D49PU) to make sure it runs fine.

This PR creates widespread small changes related to the HE-HF and HB-HE boundaries to many reconstructed quantities.

The PF validation results can be found at:
http://hep.baylor.edu/hatake/PFval/HcalTopologyIeta16And29/
[Highlights]
http://hep.baylor.edu/hatake/PFval/HcalTopologyIeta16And29/OffsetStacks/stack_NuGun_test_vs_NuGun_ref.pdf
show changes consistent with the jenkin's test. Neutral hadrons have largest differences at |eta|=2.7-3.0, and it looks like they are a little more pulled toward loer |eta|. I think this is expected given no (unintended) energy sharing between depths at the 2D clustering stage, which is addressed in this PR.
http://hep.baylor.edu/hatake/PFval/HcalTopologyIeta16And29/FlatQCD_PU_2021_ParticleFlow/JetResponse/slimmedJetsPuppi/noJEC/reso_pt_rms.pdf
Jet energy resolution is either not negatively impacted or slightly improved at the high |eta| endcap.

if this PR is a backport please specify the original PR and why you need to backport that PR:

Not backport.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 7, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 7, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31384/18212

  • This PR adds an extra 28KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 7, 2020

A new Pull Request was created by @hatakeyamak (Kenichi Hatakeyama) for master.

It involves the following packages:

Geometry/CaloTopology

@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild can you please review it and eventually sign? Thanks.
@fabiocos 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

@cvuosalo
Copy link
Contributor

cvuosalo commented Sep 8, 2020

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2020

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2020

+1
Tested at: 1561a1d
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-618d04/9208/summary.html
CMSSW: CMSSW_11_2_X_2020-09-08-1100
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 8, 2020

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2020

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 10607 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 2609667
  • DQMHistoTests: Total failures: 22136
  • DQMHistoTests: Total nulls: 8
  • DQMHistoTests: Total successes: 2587501
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.005 KiB( 34 files compared)
  • DQMHistoSizes: changed ( 136.731 ): 0.008 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 136.793 ): 0.004 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 136.874 ): -0.023 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 4.53 ): 0.016 KiB JetMET/SUSYDQM
  • Checked 149 log files, 22 edm output root files, 35 DQM output files

@cvuosalo
Copy link
Contributor

cvuosalo commented Sep 9, 2020

The large number of comparison differences need to be investigated and explained.

@hatakeyamak
Copy link
Contributor Author

hatakeyamak commented Sep 9, 2020

The large number of comparison differences need to be investigated and explained.

Right. This PR fixes an issue of how neighbors are being searched for in PF HCAL clustering around the HE-HF boundary and HB-HE boundary. I expected a little larger effect at HE-HF boundary.

Here are PF candidate eta distributions for any particleID.
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_11_2_X_2020-09-08-1100+618d04/38719/validateJR/all_OldVSNew_RunEGamma2018Cwf136p874/all_OldVSNew_RunEGamma2018Cwf136p874c_recoPFCandidates_particleFlow__reRECO_obj_eta.png
which shows most pronounced differences at HE-HF boundary (on the HE side), and differences mainly show up in neutral hadrons.
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_11_2_X_2020-09-08-1100+618d04/38719/validateJR/all_OldVSNew_RunEGamma2018Cwf136p874/all_OldVSNew_RunEGamma2018Cwf136p874c_recoPFCandidates_particleFlow__reRECO_obj_eta66.png
as expected. So, I think this is what we expect to see.

@cvuosalo
Copy link
Contributor

@hatakeyamak Please add to the PR description, in the Validation section, a statement to the effect that this PR creates widespread small changes related to the HE-HF and HB-HE boundaries to many reconstructed quantities.

@cvuosalo
Copy link
Contributor

@hatakeyamak I would also change the title of this PR to "HCalTopology revision of HE-HF and HB-HE boundaries for |ieta|=16 and 29". It is important to make clear to PdmV and others that this PR is making very noticeable changes; it is not just a minor update.

@cvuosalo
Copy link
Contributor

@slava77 @jpata FYI: This PR is making a change that slightly affects many quantities derived from the HCal. It is similar in that way to a change in Reco, but the change is happening in Geometry code.
No action is needed on your part, but I just wanted you to be aware of it.

@hatakeyamak
Copy link
Contributor Author

@hatakeyamak I would also change the title of this PR to "HCalTopology revision of HE-HF and HB-HE boundaries for |ieta|=16 and 29". It is important to make clear to PdmV and others that this PR is making very noticeable changes; it is not just a minor update.

Okay. I will do that after having one more look of jenkins test output. Thank you.

@hatakeyamak hatakeyamak changed the title HcalTopology ieta neighbor search update for |ieta|=16 and 29 HCalTopology revision of HE-HF and HB-HE boundaries for |ieta|=16 and 29 Sep 13, 2020
@hatakeyamak
Copy link
Contributor Author

hatakeyamak commented Sep 14, 2020

@cvuosalo I made suggested changes to the PR title and added a few more high stat validation results coming from PF validation workflow run on-the-fly. The change in offset quantities show changes mainly in neutral hadrons as expected and jet resolution is either slightly improved or at least not negatively impacted. To me this looks good to go.

@cvuosalo
Copy link
Contributor

+1

@cvuosalo
Copy link
Contributor

@hatakeyamak Thanks for the clarifications.

@cmsbuild
Copy link
Contributor

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)

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit a03967f into cms-sw:master Sep 14, 2020
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

4 participants