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

Extending HCAL unpacking to uTCA HBHE FEDs #13389

Merged
merged 2 commits into from
Feb 19, 2016

Conversation

abdoulline
Copy link

Late (alas...) step in preparations for MWGR#2:

(1) Emap submitted to AlCaDB validation
https://hypernews.cern.ch/HyperNews/CMS/get/calibrations/2228.html

(2) this small unpacker update needs 800 patch, if possible

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @abdoulline (Salavat Abdullin) for CMSSW_8_0_X.

It involves the following packages:

EventFilter/HcalRawToDigi

@cmsbuild, @cvuosalo, @davidlange6, @slava77 can you please review it and eventually sign? Thanks.
@Martin-Grunewald this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@@ -37,6 +37,16 @@ HcalRawToDigi::HcalRawToDigi(edm::ParameterSet const& conf):
fedUnpackList_.push_back(1118);
fedUnpackList_.push_back(1120);
fedUnpackList_.push_back(1122);
// HBHE uTCA
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we instead of this change to a loop from MINHCALuTCAFEDID to MAXHCALuTCAFEDID ?
that's from 1100 to 1199.
The only reason not to would be that you'd expect the FEDs not listed here already to come and go and produce corrupt data.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those explicitly listed are certainly what we need.
Let me clarify it with HCAL OPS guys whether there could be some
unnecessary "extra" FEDs still connected (presumably no)

On Thu, 18 Feb 2016, Slava Krutelyov wrote:

In EventFilter/HcalRawToDigi/plugins/HcalRawToDigi.cc:

@@ -37,6 +37,16 @@ HcalRawToDigi::HcalRawToDigi(edm::ParameterSet const& conf):
fedUnpackList_.push_back(1118);
fedUnpackList_.push_back(1120);
fedUnpackList_.push_back(1122);

  • // HBHE uTCA

can we instead of this change to a loop from MINHCALuTCAFEDID to MAXHCALuTCAFEDID ?
that's from 1100 to 1199.
The only reason not to would be that you'd expect the FEDs not listed here already to come and go and produce corrupt
data.


Reply to this email directly or view it on GitHub.[AEx02iVbeFXR0hFWg0luflN0Hz6WF_7Yks5plcrwgaJpZM4HdCHO.gif]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

making this inclusive loop will

  • prevent these unpacker changes (apparently forgotten until the last minute)
  • get rid of the hardcoded FED numbers (FedNumbering is preferred)

@abdoulline
Copy link
Author

Slava, I still prefer to keep FEDs list in two parts separately (VME + uTCA).
Viktor Khristenko
@vkhristenko
will hopefully give it a try with a new emap tomorrow.

@slava77
Copy link
Contributor

slava77 commented Feb 18, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/11348/console

@slava77
Copy link
Contributor

slava77 commented Feb 18, 2016

... comparisons got stuck and the new IB is somewhat broken.
I'll test locally.

@slava77
Copy link
Contributor

slava77 commented Feb 19, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Feb 19, 2016

+1

for #13389 fa80a0e

  • code changes are in line with the description
  • jenkins tests passed runtime and compilation
  • tested locally in CMSSW_8_0_0 with an equivalent of the matrix tests (more events, actually) as the ones checked in jenkins. There are no changes in monitored quentities.

belatedly, this has to go to T0 and online ASAP, to match the needs for MWGR2 for HCAL (tests of uTCA readout of HE .. or is it HB as well?)

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@abdoulline
Copy link
Author

We have now both HB and HE moved to uTCA electronics (as reflected in 

aforementioned emap and the relevant FEDs included in this minimal unpacker
update).
NB: current tests use current (still VME for HB/HE) emap, so what happens
now with this PR activated: uTCA FEDs are unpacked (good) but Digi from
them aren't produced. They are produced still from (still present in
RAW) VME HB/HE FEDs.

But we're going now to make additional private tests to make sure
explicitly that this combination/tandem of this PR + new emap works OK.
Well, there were already some tests ofuTCA HBHE FEDs unpacking
(cherrypicking) proved to be OK.

On Fri, 19 Feb 2016, Slava Krutelyov wrote:

+1

for #13389 fa80a0e

  • code changes are in line with the description
  • jenkins tests passed runtime and compilation
  • tested locally in CMSSW_8_0_0 with an equivalent of the matrix tests (more events, actually) as the ones checked in
    jenkins. There are no changes in monitored quentities.

belatedly, this has to go to T0 and online ASAP, to match the needs for MWGR2 for HCAL (tests of uTCA readout of HE .. or
is it HB as well?)


Reply to this email directly or view it on GitHub.[AEx02lh1bsJNYAC8EtF_okKlmnmbGHfxks5plqMCgaJpZM4HdCHO.gif]

@slava77
Copy link
Contributor

slava77 commented Feb 19, 2016

Salavat,

In case you have a reference file with HB/HE taken from uTCA and a new emap,
it would be useful to have as a reference.

Thanks

On 2/18/16 10:42 PM, Salavat Abdullin wrote:

We have now both HB and HE moved to uTCA electronics (as reflected in
aforementioned emap and the relevant FEDs included in this minimal unpacker
update).
NB: current tests use current (still VME for HB/HE) emap, so what happens
now with this PR activated: uTCA FEDs are unpacked (good) but Digi from
them aren't produced. They are produced still from (still present in
RAW) VME HB/HE FEDs.

But we're going now to make additional private tests to make sure
explicitly that this combination/tandem of this PR + new emap works OK.
Well, there were already some tests ofuTCA HBHE FEDs unpacking
(cherrypicking) proved to be OK.

On Fri, 19 Feb 2016, Slava Krutelyov wrote:

+1

for #13389 fa80a0e

  • code changes are in line with the description
  • jenkins tests passed runtime and compilation
  • tested locally in CMSSW_8_0_0 with an equivalent of the matrix tests (more events, actually) as the ones checked in
    jenkins. There are no changes in monitored quentities.

belatedly, this has to go to T0 and online ASAP, to match the needs for MWGR2 for HCAL (tests of uTCA readout of HE .. or
is it HB as well?)


Reply to this email directly or view it on GitHub.[AEx02lh1bsJNYAC8EtF_okKlmnmbGHfxks5plqMCgaJpZM4HdCHO.gif]


Reply to this email directly or view it on GitHub
#13389 (comment).


Vyacheslav (Slava) Krutelyov
UCSD: 9500 Gillman Dr, Physics UCSD 0354, La Jolla, CA 92093-0354
CERN: 42-R-027
AIM/Skype: siava16 googleTalk: slava77@gmail.com
(630) 291-5128 Cell (US) +41 76 275 7116 Cell (CERN)


@abdoulline
Copy link
Author

Slava,

Viktor has kindly agreed to make a test from scratch today:
to use explicitly new emap tag (not txt emap input as before) + this PR
(encompassing all the uTCA FEDs and not cherrypicking uTCA FEDs as before)

NB: uTCA HBHE data is in RAW since quite a while.

On Fri, 19 Feb 2016, Slava Krutelyov wrote:

Salavat,

In case you have a reference file with HB/HE taken from uTCA and a new emap,
it would be useful to have as a reference.

Thanks

On 2/18/16 10:42 PM, Salavat Abdullin wrote:

We have now both HB and HE moved to uTCA electronics (as reflected in
aforementioned emap and the relevant FEDs included in this minimal unpacker
update).
NB: current tests use current (still VME for HB/HE) emap, so what happens
now with this PR activated: uTCA FEDs are unpacked (good) but Digi from
them aren't produced. They are produced still from (still present in
RAW) VME HB/HE FEDs.

But we're going now to make additional private tests to make sure
explicitly that this combination/tandem of this PR + new emap works OK.
Well, there were already some tests ofuTCA HBHE FEDs unpacking
(cherrypicking) proved to be OK.

On Fri, 19 Feb 2016, Slava Krutelyov wrote:

+1

for #13389 fa80a0e

  • code changes are in line with the description
  • jenkins tests passed runtime and compilation
  • tested locally in CMSSW_8_0_0 with an equivalent of the matrix tests (more events, actually) as the ones checked in
    jenkins. There are no changes in monitored quentities.

belatedly, this has to go to T0 and online ASAP, to match the needs for MWGR2 for HCAL (tests of uTCA readout of HE ..
or
is it HB as well?)


Reply to this email directly or view it on GitHub.[AEx02lh1bsJNYAC8EtF_okKlmnmbGHfxks5plqMCgaJpZM4HdCHO.gif]


Reply to this email directly or view it on GitHub
#13389 (comment).


Vyacheslav (Slava) Krutelyov
UCSD: 9500 Gillman Dr, Physics UCSD 0354, La Jolla, CA 92093-0354
CERN: 42-R-027
AIM/Skype: siava16 googleTalk: slava77@gmail.com
(630) 291-5128 Cell (US) +41 76 275 7116 Cell (CERN)



Reply to this email directly or view it on GitHub.[AEx02mEcQupRN9FpIutF0ZfpLViarwHpks5plrPqgaJpZM4HdCHO.gif]

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@smuzaffar
Copy link
Contributor

@davidlange6, the last tests for this PR were run based on slc6_amd64_gcc530 IB and comparison were made using slc6_amd64_gcc493 baseline.

@slava77
Copy link
Contributor

slava77 commented Feb 19, 2016

So, there are small differences in tracking outputs between slc6_amd64_gcc530 and slc6_amd64_gcc493 based on these /11387/ diffs
#13389 (comment)
We may want to keep this in a better location than a side PR

@davidlange6
Copy link
Contributor

Right - I think from this we can schedule relvals and switchover. I need to look more carefully, but I did not find anything to worry about yet.

On Feb 19, 2016, at 3:04 PM, Slava Krutelyov notifications@github.com wrote:

So, there are small differences in tracking outputs between slc6_amd64_gcc530 and slc6_amd64_gcc493 based on these /11387/ diffs
#13389 (comment)
We may want to keep this in a better location than a side PR


Reply to this email directly or view it on GitHub.

davidlange6 added a commit that referenced this pull request Feb 19, 2016
Extending HCAL unpacking to uTCA HBHE FEDs
@davidlange6 davidlange6 merged commit aac9004 into cms-sw:CMSSW_8_0_X Feb 19, 2016
@abdoulline abdoulline deleted the uTCA_HBHE_FEDs branch April 13, 2017 11:00
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