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 for ECAL selective readout #17936

Merged

Conversation

franzoni
Copy link

This PR deploys the fix to ECAL selective readout already introduced in the lat ocmmit of the 90x #17886

Summary of changes in Global Tags

Upgrade

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @franzoni (Giovanni Franzoni) for master.

It involves the following packages:

Configuration/AlCa

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

cms-bot commands are listed here #13028

@arunhep
Copy link
Contributor

arunhep commented Mar 14, 2017

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 14, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18427/console Started: 2017/03/14 21:54

@cmsbuild
Copy link
Contributor

-1

Tested at: 0639ee8

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

I found follow errors while testing this PR

Failed tests: RelVals

  • RelVals:

When I ran the RelVals I found an error in the following worklfows:
10024.0 step4

runTheMatrix-results/10024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017/step4_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017.log

@cmsbuild
Copy link
Contributor

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

@franzoni
Copy link
Author

the error looks unrelated to the GT change

@franzoni
Copy link
Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 15, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18441/console Started: 2017/03/15 10:54

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@franzoni
Copy link
Author

@cmkuo
@shervin86
@emanueledimarco

Ecal experts and the lack of impact in EE: may you please comment?
This condition change in this pr assesses only the difference between the C1-scenario-with-problem (provided two weeks ago)
To the tag you released yesterday with the fix.

Does the fix pertain only EB?

@amassiro
Copy link
Contributor

The fix can have slightly different effects in EE and in EB due to different pulse shapes.
It will be explained in details tomorrow: https://indico.cern.ch/event/622160/

@franzoni
Copy link
Author

Difference in EE occupancy are actually there.
The relmon comparisons ( as already explained here #17886 (comment) ) show differences when a loose bin-to-bin comparison, and the EE differences don't fail the check on the pseudo-chi2 which flags them as "difference.
Below the change in occupancy for EB and EE, red is target (i.e. the GT 2017 realistic of this PR, TTbar), blue is the reference (i.e. the SR/ZS affected by the bug which this PR intends to fix).
91x-eb-occupancy
91x-ee

@franzoni
Copy link
Author

+1

@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 requires discussion in the ORP meeting before it's merged. @Muzaffar, @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit f180458 into cms-sw:master Mar 16, 2017
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

5 participants