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

Create new collection for Hcal lasermon digis + Hcal unpacker updates #22544

Merged
merged 13 commits into from Mar 22, 2018

Conversation

jkunkle
Copy link

@jkunkle jkunkle commented Mar 9, 2018

This is a backport of this pull request #22385 aimed at 10_0_4. The intent is to have it available for testing at p5. The backport did not require any merging, so we expect no isues.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 9, 2018

A new Pull Request was created by @jkunkle for CMSSW_10_0_X.

It involves the following packages:

CalibFormats/HcalObjects
DataFormats/HcalDetId
EventFilter/HcalRawToDigi
RecoMET/METProducers

@perrotta, @ghellwig, @civanch, @arunhep, @mdhildreth, @cmsbuild, @franzoni, @cerminar, @slava77, @lpernie can you please review it and eventually sign? Thanks.
@ghellwig, @jdamgov, @TaiSakuma, @gouskos, @ahinzmann, @mmarionncern, @rovere, @Martin-Grunewald, @jdolen, @nhanvtran, @gkasieczka, @tocheng, @schoef, @mmusich, @mariadalfonso, @seemasharmafnal this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@perrotta
Copy link
Contributor

perrotta commented Mar 9, 2018

backport of #22385

@perrotta
Copy link
Contributor

perrotta commented Mar 9, 2018

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 9, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/26728/console Started: 2018/03/09 12:55

@perrotta
Copy link
Contributor

perrotta commented Mar 9, 2018

Please @jkunkle : edit the title with something more descriptive, even better if it is the same as you used for the PR in the master (so that they can get easily linked each other)

@jkunkle jkunkle changed the title Unpacker 10 0 x Create new collection for Hcal lasermon digis + Hcal unpacker updates Mar 9, 2018
@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 9, 2018

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 27
  • DQMHistoTests: Total histograms compared: 2468459
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2468289
  • DQMHistoTests: Total skipped: 169
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.709999999963 KiB( 22 files compared)
  • Checked 111 log files, 9 edm output root files, 27 DQM output files

@perrotta
Copy link
Contributor

+1

  • Identical to Create new collection for Hcal lasermon digis + Hcal unpacker updates #22385, which is now merged in the master
  • Backport to 10_0_X required for its usage foreseen in the commissioning data taking
  • Keep laser monitoring data in the event
  • In case of data with extra samples in FED, those extra samples are now also unpacked and saved for further private studies (not being used for the current workflows)
  • No change expected and observed in reco outputs in the standard workflows, while new output will get stored in case of lasermon events or extra samples in the FED

@lpernie
Copy link
Contributor

lpernie commented Mar 13, 2018

+1

@civanch
Copy link
Contributor

civanch commented Mar 13, 2018

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_10_0_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_10_1_X is complete. This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@deguio
Copy link
Contributor

deguio commented Mar 14, 2018

Hi @fabiocos
if possible it would be nice to have this one in 10_0_4. it would give the possibility to test extensively the changes in p5 during the cruzet.
thanks,
F.

@slava77
Copy link
Contributor

slava77 commented Mar 19, 2018

@fabiocos
what is the plan for the next build?

@fabiocos
Copy link
Contributor

@slava77 @jkunkle this is in 10_1_X already. If this should be tried at P5 before 10_1_0, we need to consider a 10_0_5 (DataFormats are changed here)

@deguio
Copy link
Contributor

deguio commented Mar 20, 2018

Hi @fabiocos
right. we would like to have a 10_0_5 available to test this indeed.
thanks,
F.

@fabiocos
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit eea83e1 into cms-sw:CMSSW_10_0_X Mar 22, 2018
@slava77
Copy link
Contributor

slava77 commented Mar 22, 2018

@fabiocos
just a warning in case someone will pay attention to the production logs,
this PR is expected to lead to ~2K lines of warnings per job (8 thread case).
This shouldn't be a big deal for logCollection and archiving at T0, but may become quite distracting for log file content inspection

more in #22706

@davidlange6
Copy link
Contributor

davidlange6 commented Mar 23, 2018 via email

@deguio
Copy link
Contributor

deguio commented Mar 23, 2018

Hi,
the warnings are triggered by new calibration channels being included in the readout in recent runs at p5. this PR should actually reduce the number of warnings that we would get with the old version of the code.

at the moment we (hcal DPG and hca OPS) are deciding how to configure readout/emap/unpacker in an optimal way to support transparently the use of the HF calib channels. a dedicated discussion will happen on Monday.

In the meantime it would be beneficial to have this PR included in the 10_0_5 build as it fixes the behaviour of the old unpacker when processing lasermon channels.

@slava77
Copy link
Contributor

slava77 commented Mar 23, 2018 via email

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

9 participants