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
Ecal Pedestals PCL improvements 92X version #20369
Conversation
…e beam (off in config)
A new Pull Request was created by @argiro for CMSSW_9_2_X. It involves the following packages: Calibration/EcalCalibAlgos @ghellwig, @arunhep, @cerminar, @cmsbuild, @franzoni, @lpernie can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
Backport of #20366 |
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
please test |
The tests are being triggered in jenkins. |
@fabozzi, @Arun
I am not sure I understand how to fix the conflict. The two lines that I'd like to change are:
L63 Run2017BTE=[299149]
L70 steps['TestEnableEcalHCAL2017B']={'INPUT':InputInfo(dataSet='/TestEnablesEcalHcal/Run2017B-v1/RAW',label='te2017B',run=Run2017BTE,events=100000,location='STD')}
can someone fix it or explain how to do it ?
thanks
S.
… On 7 Sep 2017, at 13:29, Arun Kumar ***@***.***> wrote:
@argiro there are conflicts. There must be some other PR open modifying the same package which recently got merged.
@fabozzi
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
… On 7 Sep 2017, at 14:00, Stefano Argiro' ***@***.***> wrote:
@fabozzi, @Arun
I am not sure I understand how to fix the conflict. The two lines that I'd like to change are:
L63 Run2017BTE=[299149]
L70 steps['TestEnableEcalHCAL2017B']={'INPUT':InputInfo(dataSet='/TestEnablesEcalHcal/Run2017B-v1/RAW',label='te2017B',run=Run2017BTE,events=100000,location='STD')}
can someone fix it or explain how to do it ?
thanks
S.
> On 7 Sep 2017, at 13:29, Arun Kumar ***@***.***> wrote:
>
> @argiro there are conflicts. There must be some other PR open modifying the same package which recently got merged.
> @fabozzi
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
>
|
please test |
The tests are being triggered in jenkins. |
@argiro |
@fabozzi
meaning you think something is wrong with the conflict resolution I proposed ? It seems quite straight forward to me. As I say, I have changed two lines
S.
… On 7 Sep 2017, at 15:22, fabozzi ***@***.***> wrote:
@argiro
There are many changes in relval_steps.py. It seems you have not yet unders control the PR. If you have troubles to re-base, can you start from the most recent 92X IB and make just the changes you want?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@fabozzi
you are right, something is wrong, the diffs show much more than the two lines of changes, I am fixing it
… On 7 Sep 2017, at 15:25, Stefano Argiro' ***@***.***> wrote:
@fabozzi
meaning you think something is wrong with the conflict resolution I proposed ? It seems quite straight forward to me. As I say, I have changed two lines
S.
> On 7 Sep 2017, at 15:22, fabozzi ***@***.***> wrote:
>
> @argiro
> There are many changes in relval_steps.py. It seems you have not yet unders control the PR. If you have troubles to re-base, can you start from the most recent 92X IB and make just the changes you want?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
>
|
-1 Tested at: 4c24a3d 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: RelVals
The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped) |
4c24a3d
to
c7fc64f
Compare
it still seems I cannot get relval_steps.py right. I think I'll close this PR and start over
… On 7 Sep 2017, at 15:51, cmsbuild ***@***.***> wrote:
Pull request #20369 was updated. @ghellwig, @kkousour, @arunhep, @cerminar, @fabozzi, @cmsbuild, @franzoni, @kpedro88, @GurpreetSinghChahal, @lpernie can you please check and sign again.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Replaced by #20418 |
Same as #20366 , for 92X
fix for Nans, added histos, added check, added exclusion of non-stable beam (off in config)