-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 payload inspector (v2) #18519
Ecal payload inspector (v2) #18519
Conversation
A new Pull Request was created by @depasse for master. It involves the following packages: CondCore/EcalPlugins @ggovi, @cmsbuild, @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. |
hello @depasse Should this PR supersede #18453 ? |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
… ________________________________
From: Giovanni Franzoni [notifications@github.com]
Sent: 01 May 2017 20:42
To: cms-sw/cmssw
Cc: Pierre Depasse; Mention
Subject: Re: [cms-sw/cmssw] Ecal payload inspector (v2) (#18519)
hello @depasse<https://github.com/depasse>
these 2 commits deliver to the master branch
nearly the same additions as the PR #18453<#18453> ( which is not yet merged)
Merging this PR in top of #18453<#18453> generates conflicts.
Should this PR supersede #18453<#18453> ?
If not, why don't you add an additional commit to #18453<#18453> , guaranteeing that way that all the changes you want can be consistently merged ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#18519 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AEaCW6bqxQ_ALf9fTZjpgftk7VnZrwKqks5r1iefgaJpZM4NLpfi>.
|
please close #18453 then. Thanks
… On May 2, 2017, at 8:05 AM, depasse ***@***.***> wrote:
Hi,
this PR #18519 supersed #18453.
So, #18453 must be ignored, must not be merged.
Thanks.
Pierre
________________________________
From: Giovanni Franzoni ***@***.***
Sent: 01 May 2017 20:42
To: cms-sw/cmssw
Cc: Pierre Depasse; Mention
Subject: Re: [cms-sw/cmssw] Ecal payload inspector (v2) (#18519)
hello @depasse<https://github.com/depasse>
these 2 commits deliver to the master branch
nearly the same additions as the PR #18453<#18453> ( which is not yet merged)
Merging this PR in top of #18453<#18453> generates conflicts.
Should this PR supersede #18453<#18453> ?
If not, why don't you add an additional commit to #18453<#18453> , guaranteeing that way that all the changes you want can be consistently merged ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#18519 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AEaCW6bqxQ_ALf9fTZjpgftk7VnZrwKqks5r1iefgaJpZM4NLpfi>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
tests are ok, no unexpected changes in the relval-based workflows @depasse |
Hi Giovanni,
yes please, put it in the DEMO. Thanks a lot.
Pierre
…________________________________
From: Giovanni Franzoni [notifications@github.com]
Sent: 02 May 2017 16:28
To: cms-sw/cmssw
Cc: Pierre Depasse; Mention
Subject: Re: [cms-sw/cmssw] Ecal payload inspector (v2) (#18519)
tests are ok, no unexpected changes in the relval-based workflows
@depasse<https://github.com/depasse>
do you need these commits to be deployed in the DEMO instance of the service,
for inspection ?
We can try and help w/ that in the next 24-48h
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#18519 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AEaCW7XuIUz0lpGDosMm9lPIAHxScGcxks5r1z2ggaJpZM4NLpfi>.
|
class EcalChannelStatusEEMap : public cond::payloadInspector::Histogram2D<EcalChannelStatus> { | ||
|
||
public: | ||
EcalChannelStatusEEMap() : cond::payloadInspector::Histogram2D<EcalChannelStatus>( "ECAL Barrel channel status - map ", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@depasse I think this is a copy paste error. It should be ECAL Endcap channel status. Can you please confirm and eventually change if its true?
hello @depasse The DEMO of payload inspector now points to 9_1_0_pre3+18519 Please do your detailed and content inspection, and let us know. |
Hi,
Ecal ChannelStatus : Yes, it is a wrong copy/paste title.
Ecal Pedestals : I have always "500 Internal Server Error : There was an error when trying to connect to payload inspector machine".. when I try with this part. Do you have the same ?
Pierre
________________________________
From: Arun Kumar [notifications@github.com]
Sent: 02 May 2017 21:27
To: cms-sw/cmssw
Cc: Pierre Depasse; Mention
Subject: Re: [cms-sw/cmssw] Ecal payload inspector (v2) (#18519)
@arunhep commented on this pull request.
________________________________
In CondCore/EcalPlugins/plugins/EcalChannelStatus_PayloadInspector.cc<#18519 (comment)>:
+ if (!(*payload)[rawid].getEncodedStatusCode()) continue;
+
+ // fill the Histogram2D here
+ fillWithValue( (EBDetId(rawid)).iphi() , (EBDetId(rawid)).ieta(), (*payload)[rawid].getEncodedStatusCode());
+ }// loop over cellid
+ }// if payload.get()
+ }// loop over IOV's (1 in this case)
+
+ return true;
+ }// fill method
+ };
+
+ class EcalChannelStatusEEMap : public cond::payloadInspector::Histogram2D<EcalChannelStatus> {
+
+ public:
+ EcalChannelStatusEEMap() : cond::payloadInspector::Histogram2D<EcalChannelStatus>( "ECAL Barrel channel status - map ",
@depasse<https://github.com/depasse> I think this is a copy paste error. It should be ECAL Endcap channel status. Can you please confirm and eventually change if its true?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#18519 (review)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AEaCW_bO4uvtckRWJgmzVhtC7e5dStW6ks5r14OigaJpZM4NLpfi>.
|
hello @depasse for me the https://cms-conddb.cern.ch/cmsDbBrowser/payload_inspector/Prod/DEMO responts ok and I can see plots ok |
Pull request #18519 was updated. @perrotta, @cmsbuild, @civanch, @ghellwig, @silviodonato, @arunhep, @mdhildreth, @Martin-Grunewald, @franzoni, @kpedro88, @cerminar, @slava77, @ggovi, @fwyzard, @mmusich, @davidlange6 can you please check and sign again. |
-1 |
Pull request #18519 was updated. @perrotta, @cmsbuild, @civanch, @ghellwig, @silviodonato, @arunhep, @mdhildreth, @Martin-Grunewald, @franzoni, @kpedro88, @cerminar, @slava77, @ggovi, @fwyzard, @mmusich, @davidlange6 can you please check and sign again. |
-1 |
-1 due to conflicts and unrelated merges |
-1 |
Tests for Ecal PayloadInspector for Pedestals and ChannelStatus (version 2)