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

unpack HF uTCA FEDs #8223

Merged
merged 1 commit into from
Mar 12, 2015
Merged

unpack HF uTCA FEDs #8223

merged 1 commit into from
Mar 12, 2015

Conversation

elaird
Copy link
Contributor

@elaird elaird commented Mar 12, 2015

Starting today (March 12), the HF front-end data is fed to the three HF uTCA FEDs included in this commit.

CC: @abdoulline

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @elaird for CMSSW_7_3_X.

unpack HF uTCA FEDs

It involves the following packages:

EventFilter/HcalRawToDigi

@cmsbuild, @cvuosalo, @nclopezo, @slava77 can you please review it and eventually sign? Thanks.
@Martin-Grunewald this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@nclopezo you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@abdoulline
Copy link

To David: that's what I meant in person as missing component for uTCA unpacking.
We really need it in the data-taking...

@davidlange6
Copy link
Contributor

@cmsbuild, please test

On Mar 12, 2015, at 3:26 PM, cmsbuild notifications@github.com wrote:

A new Pull Request was created by @elaird for CMSSW_7_3_X.

unpack HF uTCA FEDs

It involves the following packages:

EventFilter/HcalRawToDigi

@cmsbuild, @cvuosalo, @nclopezo, @slava77 can you please review it and eventually sign? Thanks.
@Martin-Grunewald this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@nclopezo you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.


Reply to this email directly or view it on GitHub.

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2015

I'm looking at it

@davidlange6
Copy link
Contributor

will bypass this so that things get into the IB for testing. @slava77, let me know if you see any problem in your testing.

davidlange6 added a commit that referenced this pull request Mar 12, 2015
@davidlange6 davidlange6 merged commit aba5c75 into cms-sw:CMSSW_7_3_X Mar 12, 2015
@slava77
Copy link
Contributor

slava77 commented Mar 12, 2015

I actually need to understand what's going on here

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2015

the change is not what I was expecting to happen from earlier discussions

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2015

IB testing is pointless since they don't include the current data taking

@davidlange6
Copy link
Contributor

indeed….i’ll check in tomorrow am to see if things are ok or not for you.

On Mar 12, 2015, at 10:18 PM, Slava Krutelyov notifications@github.com wrote:

IB testing is pointless since they don't include the current data taking


Reply to this email directly or view it on GitHub.

@abdoulline
Copy link

Slava, this is to include uTCA FEDs in the unpacker processing (fix of
omission). By default empty FED list is supplied to unpacker, and then it
considers only VME FEDs. We want it by default (without providing
explicit FED list in dedicated configurable) to be back-compatible with Run
1 (VME only) and new mixture of VME and uTCA data, to be steered by
electronics map.

On Thu, 12 Mar 2015, David Lange wrote:

indeed….i’ll check in tomorrow am to see if things are ok or not for you.

On Mar 12, 2015, at 10:18 PM, Slava Krutelyov notifications@github.com wrote:

IB testing is pointless since they don't include the current data taking


Reply to this email directly or view it on GitHub.


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

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2015

@abdoulline @elaird

  • Do I understand correctly that the uTCA unpacked data will end up in QIE10DigiCollection? (we can't have both 718..723 and 1118..1122 FEDs go into the same HFDigiCollection)
  • There is no "keep" statement for QIE10DigiCollection anywhere: what's the point of running unpacker for these feds if the data is not stored or used
  • there is no code that reads QIE10DataFrame or QIE10DigiCollection

@abdoulline
Copy link

I believe these 3 lines completely decoupled from QIE10 (preparations) from
Alex Gude. HF still uses requlr (old) QIE8.
Digis from uTCA stored in the same HFDigiCollection as from VME channels.

On Thu, 12 Mar 2015, Slava Krutelyov wrote:

@abdoulline @elaird

  • Do I understand correctly that the uTCA unpacked data will end up in QIE10DigiCollection? (we
    can't have both 718..723 and 1118..1122 FEDs go into the same HFDigiCollection)
  • There is no "keep" statement for QIE10DigiCollection anywhere: what's the point of running
    unpacker for these feds if the data is not stored or used
  • there is no code that reads QIE10DataFrame or QIE10DigiCollection
  • Where is the "consumer"


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

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2015

uhm, interesting.
This PR (#8223) crashed without Alex's PR (8013)

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2015

I was running on 237614 data file
/store/data/Commissioning2015/Cosmics/RAW/v1/000/237/614/00000/26A23E25-ACC8-E411-92A6-02163E012A42.root
hence my guess about QIE10 ... apparently something more subtle in Alex's PR made this code to run

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2015

So, if the target is the same QIE8-based HFDigiCollection collection, then this is apparently (to me) in conflict with earlier discussion that you will be producing a separate collection for uTCA or use a clone of the hcalDigis to make the uTCA hits.

@abdoulline
Copy link

Apparently I was too fast (not to say wrong) stating "completely
decoupled"...
I recall now this PR was awaiting for Alex's fix. Ted might provide more
details (tomorrow).

On Thu, 12 Mar 2015, Slava Krutelyov wrote:

I was running on 237614 data file
/store/data/Commissioning2015/Cosmics/RAW/v1/000/237/614/00000/26A23E25-ACC8-E411-92A6-02163E012A42
.root
hence my guess about QIE10 ... apparently something more subtle in Alex's PR made this code to run


Reply to this email directly or view it on
GitHub.[AEx02mSwZnhwVepkH-YFq2G3-ZfAcqlKks5n0gSxgaJpZM4DtqIv.gif]

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2015

Salavat,
Please clarify if this PR is just a hack that you want to use for CRUZET/CRAFT for the next few days [then we should stop forward porting of this PR to 74X]
or if your plan is to never mix both uTCA and VME readouts for HF (either on the runControl side or by supplying a cabling map that doesn't let both channels to get through; my understanding so far was that VME and uTCA readouts can be done in parallel on the electronics side).

@abdoulline
Copy link

as I've mentioned today in person - we considered in-unpacker making two
collections, but finally opted for single collection for HF.
And even for future HBHE migration we most probably stay with one
collection of Digis.

On Thu, 12 Mar 2015, Slava Krutelyov wrote:

So, if the target is the same QIE8-based HFDigiCollection collection, then this is apparently (to
me) in conflict with earlier discussion that you will be producing a separate collection for uTCA or
use a clone of the hcalDigis to make the uTCA hits.


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

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2015

I didn't realize all implications of this decision until this PR discussion.
(as implied in my previous email) I'd like to understand better how this one collection solution will actually work.
If electronics doesn't let simultaneous readout, then it's almost trivial.
If the parallel readout is possible, then we are up for possible (at least occasional) mess.

@abdoulline
Copy link

Ted would probably better describe the tech.details

I can say it's not a temporary hack.
Indeed, VME and uTCA readouts can be done in parallel.
Plan is not to mix them in one digi collection for production!
Just switch from one to another when comissioning is done (today for HF).

And to have both in RAW and able to unpack both with two different
instances of unpacker or monitoring purposes. During
transition/comissioning period.

On Thu, 12 Mar 2015, Slava Krutelyov wrote:

Salavat,
Please clarify if this PR is just a hack that you want to use for CRUZET/CRAFT for the next few days
[then we should stop forward porting of this PR to 74X]
or if your plan is to never mix both uTCA and VME readouts for HF (either on the runControl side or
by supplying a cabling map that doesn't let both channels to get through; my understanding so far
was that VME and uTCA readouts can be done in parallel on the electronics side).


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

@abdoulline
Copy link

No mess, regular (GT) emap (in preparation as we speak) will steer the
things right in production: only HF VME before switch to uTCA, only HF
uTCA channels from today's switch on.

On Thu, 12 Mar 2015, Slava Krutelyov wrote:

I didn't realize all implications of this decision until this PR discussion.
(as implied in my previous email) I'd like to understand better how this one collection solution
will actually work.
If electronics doesn't let simultaneous readout, then it's almost trivial.
If the parallel readout is possible, then we are up for possible (at least occasional) mess.


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

@abdoulline
Copy link

"The morning is wiser than the evening"(c)

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2015

uhm, ok, it still sounds like we are up for occasional mess.
Is the mechanism in place to swap out the cabling map in IOVs (like run-control/configuration generating it before the run start that would no allow both channels to be in)?

With this particular solution it sounds like you want to support a mixed readout varying this and that way every other day.

@abdoulline
Copy link

Yes, emap is for that.
But no, we only change emap in production (fully from VME to uTCA) once, when commissioning is done.
I must admit this time (HF "switch") )we're a bit late, as of today we still have in production VME-only emap (HF part). Hope to have HF uTCA map tomorrow.

NB: next major single-time move will be in 2016 (HBHE commissioned with uTCA)

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2015

Sure, we can wait.
You have the benefit of this being preemptively merged already

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2015

maybe we can chat some time tomorrow?
It sounds like things are most likely in order, but better know a bit more clearly.
How about 10:00 in B40?

@abdoulline
Copy link

OK, 10 am in cafeteria of bld.40.

On Thu, 12 Mar 2015, Slava Krutelyov wrote:

maybe we can chat some time tomorrow?
It sounds like things are most likely in order, but better know a bit more clearly.
How about 10:00 in B40?


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

@slava77
Copy link
Contributor

slava77 commented Mar 13, 2015

+1

for #8223 dbe0463
tested in CMSSW_7_3_X_2015-03-12-1400 /test area sign733a/
[the following is a mix of notes from testing and from the discussion earlier today]

I checked that a recent run with HF uTCA FEDs is processed:

cmsDriver.py step2  --conditions auto:run2_data -s RAW2DIGI,L1Reco,RECO,EI,ALCAPRODUCER:@allForPrompt,DQM,ENDJOB --process RECO --data  --eventcontent RECO,AOD,ALCARECO,DQM --scenario pp --datatier RECO,AOD,ALCARECO,DQMIO --customise Configuration/DataProcessing/RecoTLR.customisePromptRun2 -n 1000 --no_exec --filein /store/data/Commissioning2015/Cosmics/RAW/v1/000/237/614/00000/26A23E25-ACC8-E411-92A6-02163E012A42.root

this is a partial PR for HF readout, global tag changes are still expected (maybe already almost in, but certainly separate from this PR).
The auto global tag used in the test may stay outdated, since the operations have a freedom to pick another GT (the GT I mentioned to Salavat was actually this auto:run2_mc from 73X).

The default readout of HF changed from VME to uTCA FEDs. This PR adds both the VME and uTCA HF FEDs on the list for unpacking; the actual channels unpacked are controlled by the HcalElectronicsMap. If this payload has appropriate IOVs defined, then the current default configuration of the hcalDigis is going to work for both the Run1 data with VME readout and for the Run2 data with uTCA.

While the HF isn't actually unpacked in the test, there is a significant increase in the size of
HcalUnpackerReport_hcalDigis__RECO. We discussed today that this will be checked.

@elaird elaird deleted the CMSSW_7_3_X branch March 17, 2015 08:11
This was referenced Apr 9, 2015
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.

5 participants