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

Fix HCAL premixing with QIE8 and uHTR #19047

Merged
merged 4 commits into from Jun 1, 2017

Conversation

kpedro88
Copy link
Contributor

In 9_1_0_pre3, it was observed that HCAL premixing did not match well with classic mixing for the 2017 Era. This was somewhat puzzling, since the problem was even evident in HB, which should be the exact same detector in 2016 and 2017.

However, there was one important change: the electronics map used in the global tag, which corresponds to the backend electronics. 2016 used the old VME/HTR setup, while 2017 uses the new uTCA/uHTR setup (but still the old QIE8 for HB and most of HE). There is a different packer/unpacker for uHTR, which discarded the per-sample error bit in the QIE8 format. Premixing uses the error bit to denote how the ADC counts in premixed digis should be interpreted, so this caused the observed regression.

To fix this, simulated QIE8 digis in premix stage 1 (creation of the premixed input file) will be packed into the old VME-style format, which keeps all the bits. To handle this in the uHTR code, flavor 7 datatype 15 (technical data, currently unused) is employed. The unpacker is correspondingly updated to handle this case correctly. (This was the agreed solution after some discussion between me and @jmmans.)

I also cleaned up some of the packer/unpacker code while I was there.

Proof that it works (ADC counts in HB; red = classic, blue = premix):

Before:
ADC counts before fix

After:
ADC counts after fix

@kpedro88
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 31, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20262/console Started: 2017/05/31 20:27

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

Configuration/StandardSequences
EventFilter/HcalRawToDigi

@perrotta, @cmsbuild, @franzoni, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @Martin-Grunewald, @ebrondol, @dgulhan this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

@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-19047/20262/summary.html

Comparison Summary:

  • You potentially added 51 lines to the logs
  • Reco comparison results: 3415 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 1786072
  • DQMHistoTests: Total failures: 33719
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1752180
  • DQMHistoTests: Total skipped: 173
  • DQMHistoTests: Total Missing objects: 0
  • Checked 94 log files, 14 edm output root files, 23 DQM output files

@davidlange6
Copy link
Contributor

merging as to have this in 921 as I see no changes in the monitoring. Please complain if there are issues

@davidlange6 davidlange6 merged commit 9746d95 into cms-sw:master Jun 1, 2017
@perrotta
Copy link
Contributor

perrotta commented Jun 2, 2017

+1
(post merging, just for the record)
No evident issues found in code and in jenkins outputs, no reason to complain

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