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

Enable packing/unpacking for Phase2 HCAL #20920

Merged
merged 4 commits into from Oct 26, 2017

Conversation

kpedro88
Copy link
Contributor

With #20758, we have a Phase2 GT that contains a functioning emap with all the HB upgrade cells.

This means that we no longer have to skip packing/unpacking for HB in Phase2 workflows. Among other things, this will enable premixing to work for the Phase2 HB.

(These changes were also propagated to 2019/post-LS2, but had to be disabled because there isn't a 2019 GT queue yet.)

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20920/1455

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @kpedro88 (Kevin Pedro) for master.

It involves the following packages:

CalibCalorimetry/HcalPlugins
Configuration/Eras
RecoLocalCalo/HcalRecProducers

@perrotta, @ghellwig, @arunhep, @cerminar, @cmsbuild, @franzoni, @slava77, @lpernie, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @makortel, @tocheng, @Martin-Grunewald, @ebrondol, @mmusich, @mariadalfonso this is something you requested to watch as well.
@davidlange6, @slava77 you are the release manager for this.

cms-bot commands are listed here

@kpedro88
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 13, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/23764/console Started: 2017/10/13 23:30

@cmsbuild
Copy link
Contributor

-1

Tested at: 5e075ae

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
25793ba
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20920/23764/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20920/23764/git-merge-result

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20920/23764/summary.html

I found follow errors while testing this PR

Failed tests: RelVals AddOn

  • RelVals:

The relvals timed out after 2 hours.

  • AddOn:

I found errors in the following addon tests:

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
25793ba
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20920/23764/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20920/23764/git-merge-result

@cmsbuild
Copy link
Contributor

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

@kpedro88
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 14, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/23765/console Started: 2017/10/14 07:31

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 12 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2767139
  • DQMHistoTests: Total failures: 105
  • DQMHistoTests: Total nulls: 2
  • DQMHistoTests: Total successes: 2766861
  • DQMHistoTests: Total skipped: 171
  • DQMHistoTests: Total Missing objects: 0
  • Checked 107 log files, 10 edm output root files, 26 DQM output files

@kpedro88
Copy link
Contributor Author

  • No differences in 2019 comparison (after most recent commit)
  • 1 difference in 2023 comparisons: plot that involves emap-related quantities, change is expected since the emap is different
  • reco comparisons show missing ZDC collections. Previously, the ZDC digi collection was being passed through the fake HCAL unpacker, but there are no ZDC entries in the emap for the real unpacker. Right now ZDC is unpacked via the Castor unpacker, but Castor is removed in the phase2 detector. Probably the ZDC unpacking needs to be moved back into HCAL (maybe in conjunction with moving to QIE10 readout); I'm discussing this with experts now, and can post a GitHub issue to track the progress. @slava77 @perrotta please advise

@slava77
Copy link
Contributor

slava77 commented Oct 16, 2017 via email

@abdoulline
Copy link

abdoulline commented Oct 16, 2017

My understanding is that HCAL emap contained entries for old ZDC detector, which doesn't exist anymore (since unsuccessful attempt to use it for the very last time in 2016 for HI data taking).
Old ZDC unpacking has been moved to CASTOR (following ZDC FEDs move to it) early in 2016 :

8395990#diff-0b2dd580fc15df181abe8beefafaae43

Instead, ZDC group has been developed a hodoscope ("RPD") detector, for which HcalZDCDetId has been extended
https://github.com/cms-sw/cmssw/commits/master/DataFormats/HcalDetId/interface/HcalZDCDetId.h
but since that time there were no more published developments (SIM/RECO) to my knowledge...

@perrotta
Copy link
Contributor

@kpedro88 : my understanding after the reco meeting was that the ZDC group would have taken care in due time of the packing/unpacking of their detector inside HCal, and in the meanwhile we can go on with this PR and remain without the ZDC collections for Phase2 data.
Is this summary correct? Can we therefore ask for merging this PR now? Do you have some further info to be reported here after your inquiry (anticipated in #20920 (comment)) with the ZDC experts?

@kpedro88
Copy link
Contributor Author

@perrotta yes, this summary is correct. We should go forward with this PR. Here are some excerpts from the email discussion with @BetterWang:

  1. ZDC will be part of HCAL (ideally HF, since we use the same
    electronics, readout chain) for 2018 run and Phase2;
  2. ZDC will be unpacked and calibrated by HCAL software; the current
    ZDC unpacker should be discarded to give way to the HCAL unpacker.
  3. ZDC will not be simulated in any simulations (pp, or HI) at the
    moment (might update for HI in long term, but not before LS2), if has
    to, maybe just use a dummy packer?
  4. ZDC/RPD EMAP history can be found at [1]. We might want to double check it.
    [1] http://cmsdoc.cern.ch/cms/HCAL/document/Mapping/ZDC/

If the [ZDC] simulation is used in the future, it will need to be upgraded to use QIE10 readout instead of QIE8, in order to be consistent with data. (Hopefully it can just be packed like HF QIE10 digis to reduce duplication of effort.)

There was already some work done in the HCAL unpacker to support ZDC w/ QIE10, see #16150.

ZDC was read out from CASTOR in 2016 pPb run. It works really fine during data taking.
We couldn't use QIE10 to read out from HF due to the radiation damage to the fibers.
In 2018, we will reinstall new fibers before the HI run, and read out from HCal.

@perrotta
Copy link
Contributor

Thank you @kpedro88 for the confirmation

@perrotta
Copy link
Contributor

+1

@kpedro88
Copy link
Contributor Author

@ghellwig, @arunhep, @cerminar, @franzoni, @lpernie please sign

@lpernie
Copy link
Contributor

lpernie commented Oct 24, 2017

+1

@davidlange6
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit c4cd8ff into cms-sw:master Oct 26, 2017
makortel added a commit to makortel/cmssw that referenced this pull request Feb 12, 2018
…remixing_cff as unnecessary

Note that HCAL phase2 packing+unpacking was enabled in cms-sw#20920, so
there is no need to remove that for phase2 premixing (which anyway is
not being tested or working at the moment).

Also note that in L1TDigiToRaw_cff we have to reset the
rawDataCollector.RawCollectionList as if the L1TDigiToRaw_cff would
not have been loaded. This was not needed earlier because the file was
explicitly not loaded.
makortel added a commit to makortel/cmssw that referenced this pull request Feb 13, 2018
…zation

Note that HCAL phase2 packing+unpacking was enabled in cms-sw#20920, so
there is no need to remove that for phase2 premixing (which anyway is
not being tested or working at the moment).

Also note that in L1TDigiToRaw_cff we have to reset the
rawDataCollector.RawCollectionList as if the L1TDigiToRaw_cff would
not have been loaded. This was not needed earlier because the file was
explicitly not loaded.
makortel added a commit to makortel/cmssw that referenced this pull request Feb 14, 2018
…zation

Note that HCAL phase2 packing+unpacking was enabled in cms-sw#20920, so
there is no need to remove that for phase2 premixing (which anyway is
not being tested or working at the moment).

Also note that in L1TDigiToRaw_cff we have to reset the
rawDataCollector.RawCollectionList as if the L1TDigiToRaw_cff would
not have been loaded. This was not needed earlier because the file was
explicitly not loaded.
makortel added a commit to makortel/cmssw that referenced this pull request Feb 28, 2018
…zation

Note that HCAL phase2 packing+unpacking was enabled in cms-sw#20920, so
there is no need to remove that for phase2 premixing (which anyway is
not being tested or working at the moment).

Also note that in L1TDigiToRaw_cff we have to reset the
rawDataCollector.RawCollectionList as if the L1TDigiToRaw_cff would
not have been loaded. This was not needed earlier because the file was
explicitly not loaded.
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