ZDC unpacker, cleaner commit #12497

cferraio · 2015-11-19T14:10:30Z

Hi all,

I'm started a new commit with the ZDC unpacker that is much cleaner. Hopefully this helps.

Chris

cmsbuild · 2015-11-19T14:11:12Z

A new Pull Request was created by @cferraio (Chris Ferraioli) for CMSSW_7_5_X.

It involves the following packages:

Configuration/StandardSequences
EventFilter/CastorRawToDigi

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

Following commands in first line of a comment are recognized

+1|approve[d]|sign[ed]: L1/L2's to approve it
-1|reject[ed]: L1/L2's to reject it
assign <category>[,<category>[,...]]: L1/L2's to request signatures from other categories
unassign <category>[,<category>[,...]]: L1/L2's to remove signatures from other categories
hold: L1/all L2's/release manager to mark it as on hold
unhold: L1/user who put this PR on hold
merge: L1/release managers to merge this request
[@cmsbuild,] please test: L1/L2 and selected users to start jenkins tests
[@cmsbuild,] please test with cms-sw/cmsdist#<PR>: L1/L2 and selected users to start jenkins tests using externals from cmsdist

mmusich · 2015-11-19T14:16:16Z

-1
@cferraio you're still proposing to remove the changes I recently introduce in #12491. A further rebase is needed

mmusich · 2015-11-19T14:18:11Z

hold

cmsbuild · 2015-11-19T14:21:12Z

Pull request has been put on hold by @mmusich
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

cferraio · 2015-11-19T14:22:44Z

Hi @mmusich, how do I do this? I thought I had taken the most recent release.

Sorry...first time going through this progress...

abdoulline · 2015-11-19T14:38:54Z

The most recent IB is usually as good way to proceed...
scram list -a CMSSW | grep 7_5_X
-> CMSSW_7_5_X_2015-11-19-1100

cferraio · 2015-11-19T14:44:20Z

Ok, that's the one I used, however it looks like i DID add in AlCaReco...py
"git add Configuration/StandardSequences/python/AlCaRecoStreamsHeavyIons_cff.py"

I think I understand what happened now, I copied over a folder instead of an individual file, and then committed what git picked up as modified. If I change that file to what Marco committed, then recommit, would that work, or should I start over again and be more careful when copying files?

Thanks,
Chris

On Nov 19, 2015, at 15:38, Salavat Abdullin notifications@github.com wrote:

The most recent IB is usually as good way to proceed...
scramv1 list -a CMSSW | grep 7_5_X
-> CMSSW_7_5_X_2015-11-19-1100

—
Reply to this email directly or view it on GitHub.

mmusich · 2015-11-19T14:49:41Z

@cferraio you can edit the file and remove the changes you didn't mean to include. After that you can commit again with the --amend option (that will make your history look good).

mend

cferraio · 2015-11-19T15:09:20Z

hi @mmusich, i'm having some issues. i made the change and committed with amend, but i receive an error message when i push:
git push my-cmssw ZdcUnpacker_HI75X
Enter passphrase for key '/afs/cern.ch/user/c/cferraio/.ssh/id_rsa':
To git@github.com:cferraio/cmssw.git
! [rejected] ZdcUnpacker_HI75X -> ZdcUnpacker_HI75X (non-fast-forward)
error: failed to push some refs to 'git@github.com:cferraio/cmssw.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Merge the remote changes (e.g. 'git pull')
hint: before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

thanks again...

mmusich · 2015-11-19T15:10:51Z

git push my-cmssw +HEAD:ZdcUnpacker_HI75X

cferraio · 2015-11-19T15:11:57Z

great, that did it, thanks!

fingers crossed everything looks good...

On Nov 19, 2015, at 16:10, Marco Musich notifications@github.com wrote:

git push my-cmssw +HEAD:ZdcUnpacker_HI75X

mmusich · 2015-11-19T15:14:48Z

there is still one line difference, but should be harmless enough :)

mmusich · 2015-11-19T15:14:59Z

unhold

cmsbuild · 2015-11-19T15:16:23Z

Pull request #12497 was updated. @cmsbuild, @cvuosalo, @franzoni, @davidlange6, @slava77 can you please check and sign again.

slava77 · 2015-11-19T15:37:20Z

unless I'm missing something, this is still working only up to digi. The issue with rechits is still to be addressed, right?

cferraio · 2015-11-19T15:39:24Z

Hi Slava,

Sorry, maybe I was confused, I thought this was something we were to work on after we got this up and running and into the digi level. Is it necessary to fix through reco at this stage?

On Nov 19, 2015, at 16:37, Slava Krutelyov notifications@github.com wrote:

unless I'm missing something, this is still working only up to digi. The issue with rechits is still to be addressed, right?

—
Reply to this email directly or view it on GitHub.

slava77 · 2015-11-19T15:44:29Z

Chris,
up to your last message and the PR updates just recently I was anticipating the local reco is going to be updated.
High hopes, I guess.

cmsbuild · 2015-11-19T18:03:34Z

+1
Tested at: 8156022
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12497/9866/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
eebd997
fa361f8
f883935
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12497/9866/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12497/9866/git-merge-result

cmsbuild · 2015-11-19T19:30:59Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12497/9866/summary.html

slava77 · 2015-11-19T22:54:52Z

EventFilter/CastorRawToDigi/src/ZdcUnpacker.cc

+
+    //////////////////////////////////////////////
+    qie_begin=(const HcalQIESample*)daq_first;
+    qie_end=(HcalQIESample*)(daq_last+1); // one beyond last..


this generates a complaint from the static analyzer as well
(you can see it in the link next to "Tested at: " after automated test report is produced, they come out in "Static checks outputs")
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12497/9866/llvm-analysis/report-5dee98.html#EndPath

the code in fact doesn't use these casted non-consts, no harm.
Still, better fix it.

Ah, ok, I missed the const on the second line. Fixed.

On Nov 19, 2015, at 23:54, Slava Krutelyov notifications@github.com wrote:

In EventFilter/CastorRawToDigi/src/ZdcUnpacker.cc:

if (!silent) edm::LogWarning("ZdcUnpacker") << "Skipping data on spigot " << spigot << " of DCC with source id " << dccHeader->getSourceId() << " which is of unknown flavor " << htr.getFirmwareFlavor();

continue;

}

// calculate "real" number of presamples

//int nps=htr.getNPS()-startSample_;

// get pointers

htr.dataPointers(&daq_first,&daq_last,&tp_first,&tp_last);

unsigned int smid=htr.getSubmodule();

int htr_tb=smid&0x1;

int htr_slot=(smid>>1)&0x1F;

int htr_cr=(smid>>6)&0x1F;

//////////////////////////////////////////////

qie_begin=(const HcalQIESample*)daq_first;

qie_end=(HcalQIESample*)(daq_last+1); // one beyond last..

this generates a complaint from the static analyzer as well
(you can see it in the link next to "Tested at: " after automated test report is produced, they come out in "Static checks outputs")
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12497/9866/llvm-analysis/report-5dee98.html#EndPath

the code in fact doesn't use these casted non-consts, no harm.
Still, better fix it.

—
Reply to this email directly or view it on GitHub.

slava77 · 2015-11-19T23:31:39Z

I'm confused now: I was expecting that after we move to castor raw2digi, only one ZDC FED will be unpacked (the 693).
[this picture doesn't quite match the slides from today. What I see in the code now is that both FED 693 and FED 722 are unpacked in one place]

What's FED 722? it's the old ZDC VME FED index, when it was "attached" to HF, right?

I have a feeling I understand even less now.

Let me start with "basic" questions:

can the hcal raw2digi unpack new ZDC data if it was coming from the FED 722? (was it a mere FED number change or was there really a change in the format?)
if it's just a FED numbering, can't you just add FED 693 to https://github.com/cms-sw/cmssw/blob/CMSSW_7_5_6/EventFilter/HcalRawToDigi/plugins/HcalRawToDigi.cc#L39 and be done?

Looking at the HcalUnpacker and HcalRawToDigi, it wouldn't care which VME FED number it is as long as it's in the list to try.
@abdoulline ?

slava77 · 2015-11-19T23:32:35Z

sorry, I meant

can the hcal raw2digi unpack new ZDC data if it was coming from the FED 693? (was it a mere FED number change or was there really a change in the format?)

abdoulline · 2015-11-19T23:47:36Z

Slava,
it was HCAL request to start "phasing out" ZDC from HCAL ("a la" CASTOR), as new ZDC is sitting in CASTOR crate/FED
http://cmslxr.fnal.gov/source/DataFormats/FEDRawData/interface/FEDNumbering.h

ZDC entries were removed from HCAL emap since April. This was no-return point unpacking-wise.

NB: in principle, HCAL unpacker still could have managed new ZDC in-CASTOR FED
(693) , but HCAL OPS didn't want to maintain ZDC emap and insisted on its
removal from HCAL emap early in 2015.
More so, the request was made about ZDC QIE electronics properties to be
removed from HCAL DB table in particular , and possibly other ZDC conditions from HCAL ones in general (like in conditions-self-consistent CASTOR).
But this was unrealistic to accomplish given very limited manpower on ZDC
side (and HCAL overwhelmed by many HCAL-specific issues). So at the moment
ZDC is still in HCAL DIGI/RECO code (even ZDC snippets in HCAL
packer/unpacker), but not in new (2015) data e-map.

cferraio · 2015-11-20T09:07:40Z

Hi Slava,

Unfortunately Hcal has kicked out the Zdc :-).

That's correct, 722 is the Fed used for old datasets, and I'm using it to distinguish between pre- and post- April data sets. As Salavat mentioned, Hcal can't simply unpack Zdc with a change of fed id for run 2 data. I've set up the code so that when it has info in fed 722 it uses different electronics and detectors ids than when it unpacks info in fed 693.

On Nov 20, 2015, at 00:31, Slava Krutelyov notifications@github.com wrote:

I'm confused now: I was expecting that after we move to castor raw2digi, only one ZDC FED will be unpacked (the 693).
[this picture doesn't quite match the slides from today. What I see in the code now is that both FED 693 and FED 722 are unpacked in one place]

What's FED 722? it's the old ZDC VME FED index, when it was "attached" to HF, right?

I have a feeling I understand even less now.

Let me start with "basic" questions:

• can the hcal raw2digi unpack new ZDC data if it was coming from the FED 722? (was it a mere FED number change or was there really a change in the format?)
• if it's just a FED numbering, can't you just add FED 693 to https://github.com/cms-sw/cmssw/blob/CMSSW_7_5_6/EventFilter/HcalRawToDigi/plugins/HcalRawToDigi.cc#L39 and be done?
Looking at the HcalUnpacker and HcalRawToDigi, it wouldn't care which VME FED number it is as long as it's in the list to try.
@abdoulline ?

—
Reply to this email directly or view it on GitHub.

cmsbuild · 2015-11-20T09:11:21Z

Pull request #12497 was updated. @cmsbuild, @cvuosalo, @franzoni, @davidlange6, @slava77 can you please check and sign again.

slava77 · 2015-11-20T14:33:55Z

@cmsbuild please test
[will just check up to compilation]

cmsbuild · 2015-11-20T14:36:23Z

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

slava77 · 2015-11-20T15:27:14Z

+1

for #12497 1b2649d

code changes are OK for this emergency 75X situation (and I'd prefer not to forward port and instead to solve this appropriately; even better still I'd rather see the ZDC unpacking go back to hcalDigis where it was fitting well and will still fit OK from the code perspective)
jenkins tests pass and comparisons with baseline show no relevant differences
local tests with CMSSW_7_5_6 and run2 pp data and also recent runs (262163)
- show only a change in HcalUnpackerReport castorDigis errorFree among the monitored quantities (expected)
- zdc digis are not empty with a manual inspection in 262163
  Tests were done with ZDC unpacker, cleaner commit #12497 8156022

For post-75X perspective. If I understood the plans correctly, the 722 and 693 FED unpacking (now both in castorDigis) will be moved out to a separate zdcRawToDigi and also outputs of it will be picked up by the local reconstruction.

slava77 · 2015-11-20T15:27:30Z

merge

compilation is OK from the console in jenkins

ZDC unpacker, cleaner commit

cmsbuild · 2015-11-20T16:29:20Z

+1
Tested at: 1b2649d
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12497/9882/summary.html

cmsbuild · 2015-11-20T17:28:58Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12497/9882/summary.html

cmsbuild added this to the Next CMSSW_7_5_X milestone Nov 19, 2015

cmsbuild added reconstruction-pending operations-pending pending-signatures tests-pending orp-pending comparison-pending labels Nov 19, 2015

cferraio mentioned this pull request Nov 19, 2015

Zdc unpacker #12467

Closed

cmsbuild added the hold label Nov 19, 2015

cferraio added 2 commits November 19, 2015 15:55

more zdc unpacker fixes

0421f18

ammending AlCaRecoStreamsHeavyIons_cff.py

8156022

mend

cferraio force-pushed the ZdcUnpacker_HI75X branch from 3aca95b to 8156022 Compare November 19, 2015 15:11

cmsbuild removed the hold label Nov 19, 2015

cmsbuild added tests-approved and removed tests-started labels Nov 19, 2015

cmsbuild added comparison-available and removed comparison-pending labels Nov 19, 2015

slava77 reviewed Nov 19, 2015
View reviewed changes

fixed const cast issue

1b2649d

cmsbuild added tests-pending comparison-pending and removed comparison-available tests-approved labels Nov 20, 2015

cmsbuild added tests-started and removed tests-pending labels Nov 20, 2015

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Nov 20, 2015

cmsbuild added a commit that referenced this pull request Nov 20, 2015

Merge pull request #12497 from cferraio/ZdcUnpacker_HI75X

f590afb

ZDC unpacker, cleaner commit

cmsbuild merged commit f590afb into cms-sw:CMSSW_7_5_X Nov 20, 2015

slava77 mentioned this pull request Apr 4, 2016

Zdc unpacker digi reco fix81 x #13882

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ZDC unpacker, cleaner commit #12497

ZDC unpacker, cleaner commit #12497

cferraio commented Nov 19, 2015

cmsbuild commented Nov 19, 2015

mmusich commented Nov 19, 2015

mmusich commented Nov 19, 2015

cmsbuild commented Nov 19, 2015

cferraio commented Nov 19, 2015

abdoulline commented Nov 19, 2015

cferraio commented Nov 19, 2015

mmusich commented Nov 19, 2015

cferraio commented Nov 19, 2015

mmusich commented Nov 19, 2015

cferraio commented Nov 19, 2015

mmusich commented Nov 19, 2015

mmusich commented Nov 19, 2015

cmsbuild commented Nov 19, 2015

slava77 commented Nov 19, 2015

cferraio commented Nov 19, 2015

slava77 commented Nov 19, 2015

cmsbuild commented Nov 19, 2015

cmsbuild commented Nov 19, 2015

slava77 Nov 19, 2015

cferraio Nov 20, 2015

slava77 commented Nov 19, 2015

slava77 commented Nov 19, 2015

abdoulline commented Nov 19, 2015

cferraio commented Nov 20, 2015

cmsbuild commented Nov 20, 2015

slava77 commented Nov 20, 2015

cmsbuild commented Nov 20, 2015

slava77 commented Nov 20, 2015

slava77 commented Nov 20, 2015

cmsbuild commented Nov 20, 2015

cmsbuild commented Nov 20, 2015

ZDC unpacker, cleaner commit #12497

ZDC unpacker, cleaner commit #12497

Conversation

cferraio commented Nov 19, 2015

cmsbuild commented Nov 19, 2015

mmusich commented Nov 19, 2015

mmusich commented Nov 19, 2015

cmsbuild commented Nov 19, 2015

cferraio commented Nov 19, 2015

abdoulline commented Nov 19, 2015

cferraio commented Nov 19, 2015

mmusich commented Nov 19, 2015

cferraio commented Nov 19, 2015

mmusich commented Nov 19, 2015

cferraio commented Nov 19, 2015

mmusich commented Nov 19, 2015

mmusich commented Nov 19, 2015

cmsbuild commented Nov 19, 2015

slava77 commented Nov 19, 2015

cferraio commented Nov 19, 2015

slava77 commented Nov 19, 2015

cmsbuild commented Nov 19, 2015

cmsbuild commented Nov 19, 2015

slava77 Nov 19, 2015

Choose a reason for hiding this comment

cferraio Nov 20, 2015

Choose a reason for hiding this comment

slava77 commented Nov 19, 2015

slava77 commented Nov 19, 2015

abdoulline commented Nov 19, 2015

cferraio commented Nov 20, 2015

cmsbuild commented Nov 20, 2015

slava77 commented Nov 20, 2015

cmsbuild commented Nov 20, 2015

slava77 commented Nov 20, 2015

slava77 commented Nov 20, 2015

cmsbuild commented Nov 20, 2015

cmsbuild commented Nov 20, 2015