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
HCALDQM: bugfix, QIE11DataFrame::size() should be samples() #19588
Conversation
A new Pull Request was created by @DryRun (David Yu) for master. It involves the following packages: DQM/HcalTasks @vazzolini, @kmaeshima, @dmitrijus, @cmsbuild, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
-1 Tested at: 98cd3b8 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: You can see the results of the tests here: I found follow errors while testing this PR Failed tests: UnitTests
I found errors in the following unit tests: The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
@smuzaffar @davidlange6 do you know why the bot is printing "Failed tests: UnitTests" with no list of failed tests afterwards? Is this a bug in the bot, or did something actually go wrong? |
I see in https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19588/21262/unitTests.log
which normally happens when unit tests hangs and killed by timeout |
Comparison is ready Comparison Summary:
|
It's hard for me to imagine how this change would cause the unit test to fail like that... could this be a transient issue? |
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
please test |
The tests are being triggered in jenkins. |
-1 Tested at: 98cd3b8 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: You can see the results of the tests here: I found follow errors while testing this PR Failed tests: AddOn
I found errors in the following addon tests: cmsDriver.py TTbar_Tauola_13TeV_TuneCUETP8M1_cfi -s GEN,SIM,DIGI,L1,DIGI2RAW --mc --scenario=pp -n 10 --conditions auto:run2_mc_GRun --relval 9000,50 --datatier "GEN-SIM-RAW" --eventcontent RAWSIM --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2017 --magField 38T_PostLS1 --fileout file:RelVal_Raw_GRun_MC.root : FAILED - time: date Mon Jul 10 15:04:15 2017-date Mon Jul 10 15:03:07 2017 s - exit: 21248 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
Looks like all AddOn tests failures (all with the same - exit: 21248) and the same error message: are not related to this PR. E.g. : => dropping patTrigger |
Yes, looks like a broken IB caused by #19469 |
Comparison is ready Comparison Summary:
|
merge |
As @slava77 pointed out in #18841, there was a bug introduced that caused a plot to be filled with random values (Hcal/DigiTask/TimingCut/depth/*, for QIE11 channels; the timing is basically the charge-weighted average time slice). QIE11DataFrame::size() should be QIE11DataFrame::samples(); size() includes the digi header/footer, so the charge lookup was going past the last element of the vector storing the charge per time slice.