Change of DigiToRaw code to keep SiStrip Error information during data repacking #12674

rkunnawa · 2015-12-04T20:11:06Z

This PR fixes the bug which caused empty histograms for SiStrip FED Errors in the DQM. This is prepared for the HIN data taking period after we saw the aforementioned issues.
thanks!
@abaty @icali

cmsbuild · 2015-12-04T20:11:29Z

A new Pull Request was created by @rkunnawa (Raghav Kunnawalkam Elayavalli) for CMSSW_7_5_X.

It involves the following packages:

EventFilter/SiStripRawToDigi

@cmsbuild, @cvuosalo, @davidlange6, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @Martin-Grunewald, @VinInn 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

abaty · 2015-12-04T20:39:41Z

Some elaboration about this PR. This code adds to the DigiToRaw repacking code the ability to copy the header of another rawDataCollector. This is important for Heavy Ion data taking, as nearly all our data stored on disk is run through the repacker. Without this fix we lose the information stored in the header about the error status of the SiStrips, and also are unable to fill some of the SiStrip FED DQM plots.

slava77 · 2015-12-04T21:27:33Z

EventFilter/SiStripRawToDigi/plugins/SiStripDigiToRaw.cc

+	    fedbuffer.reset(new sistrip::FEDBuffer(rawfedData.data(),rawfedData.size(),true));
+	    //bufferBase.reset(new sistrip::FEDBufferBase(rawfedData.data(),rawfedData.size()));
+	  } catch (const cms::Exception& e) {
+	    std::cerr << "[sistrip::DigiToRaw::createFedBuffers_]"


use LogError or LogWarning

line 143 here has "std::cerr"
please change to LogWarning or LogError

slava77 · 2015-12-04T21:33:50Z

@rkunnawa
please provide details how to test with a case where two fedRaw are read

slava77 · 2015-12-04T21:34:06Z

@cmsbuild please test

cmsbuild · 2015-12-04T21:34:11Z

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

cmsbuild · 2015-12-04T22:10:21Z

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

rkunnawa · 2015-12-04T22:12:57Z

@slava77 we do have the logWarning in the code. can you briefly explain what you wanted done? should we remove the

```
if ( edm::isDebugEnabled() ) {
```
thanks!

cmsbuild · 2015-12-04T22:20:41Z

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

rkunnawa · 2015-12-04T22:23:07Z

@slava77 thanks for pointing that out. im afraid i need to ask you again what you mean by " please provide details how to test with a case where two fedRaw are read". Do you want a config which we tested with the cout statements for a single fed (which you can find it here ~rkunnawa/public.testSiStripFEDMonitor_cfg.py on lxplus)? or im not sure?

slava77 · 2015-12-04T22:26:49Z

@rkunnawa
is this config what's supposed to run online or really just a small test setup?
I'd rather check the full setup.

cmsbuild · 2015-12-04T23:38:28Z

+1
Tested at: 803ef9c
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12674/10154/summary.html

amagnan · 2015-12-09T20:13:26Z

Just want to add that the exact error classification doesn't matter much in practice, we have checked the raw fed buffer and indeed both types could apply. I still want to understand why exactly they get reclassified but this should not prevent the PR to go ahead: it is much better to have this now than delay it any further.

cmsbuild · 2015-12-09T21:48:45Z

-1
Tested at: 8616a23
When I ran the RelVals I found an error in the following worklfows:
1000.0 step1

DAS Error

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12674/10217/summary.html

slava77 · 2015-12-09T21:55:47Z

@cmsbuild please test

cmsbuild · 2015-12-09T21:56:07Z

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

cmsbuild · 2015-12-09T23:41:39Z

+1
Tested at: 8616a23
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12674/10221/summary.html

cmsbuild · 2015-12-10T00:29:40Z

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

slava77 · 2015-12-10T01:46:43Z

+1

for #12674 8616a23

code changes are in the packer of sistrip data in the heavy ion data taking.
changes introduced up to 73f0c48 are dscussed above; the last commit didn't trigger any additional changes in the monitored quantities (including matrix with 2010 HI data and reHLT-reRECO workflow of 2015 HI data)

@rkunnawa @abaty
Please, make a pull request for 80X and also just in case for 76X.

@rkunnawa please use the "Edit" button at the top of the github page for this PR and update the subject to reflect changes made in the packer (for self-documentation)

rkunnawa · 2015-12-10T10:05:31Z

@slava77 done! hope we can get this now! we will prepare one for 80X and 76X as requested, with all the changes.

Change of DigiToRaw code to keep SiStrip Error information during data repacking

Martin-Grunewald · 2015-12-10T11:41:13Z

Is this also needed for MC production validation purposes? If so we'd need another PR from HLT as #12735 does not contain the new parameters, so would use the default setting from fillDescriptions for them, while I assume you'd need another setting for the added parameters.
Please advise!
BTW, for data I'd need to parse this and the menu used online needs to be migrated and then changing the added parameters. Please advise as well!

amagnan · 2015-12-10T12:10:54Z

Hi, no need to change anything for MC I think, default settings are fine. MC doesn't know anything about fed errors or buffer header as far as I understand....

icali · 2015-12-10T12:24:35Z

@Martin-Grunewald not an expert but I believe that we need to ask the parsing of the menu. This is used in only one module. I also agree with Anne Marie, nothing should be affected in MC.

Martin-Grunewald · 2015-12-10T12:40:31Z

Hi,

OK, I have patched it into ConfDB - code template CMSSW_7_5_7_patch1_HLT1.
Default migration to this new template leads to these changes in the HIon menu:

-------------------------------------------------------------------------------
Modules (2):
  -> hltSiStripRawDigiToVirginRaw [SiStripDigiToRawModule] CHANGED
       bool CopyBufferHeader = false [ADDED]
       InputTag RawDataTag = rawDataCollector [ADDED]
  -> hltSiStripDigiToZSRaw [SiStripDigiToRawModule] CHANGED
       bool CopyBufferHeader = false [ADDED]
       InputTag RawDataTag = rawDataCollector [ADDED]

Thus, migrate the online menu you have in ConfDBv2, modified since provided by STORM,
and then adjust the new parameters in the two instances above to your liking!
As an example, I have done so starting from the STORM menu in hltdev (ConfDBv1),
/online/collisions/2015/HeavyIons/v1.0/HLT/V6, migrated that and saved it as
/online/collisions/2015/HeavyIons/v1.0/HLT/V7. Simply do the same with the most
recent PbPb menu running online.

abaty · 2015-12-11T17:42:35Z

@slava77

We managed to run this change online today and it worked well. I have made a PR for 8_0_X (#12762).

I have not made the PR for 7_6_X, as it has some changes that depend on having PR (#12161) merged before, and that PR seems to be held up quite a bit. Should I just wait for that PR to merge first?

Fix for SiStripErrors in HI repacking (porting PR #12674)

rkunnawa and others added 3 commits December 3, 2015 17:36

fixing the FED dqm monitoring header errors

1339198

input tag changed to rawDataCollector

31feddc

gave correct fed buffer header

18b46dd

cmsbuild added this to the Next CMSSW_7_5_X milestone Dec 4, 2015

cmsbuild added reconstruction-pending pending-signatures tests-pending orp-pending comparison-pending labels Dec 4, 2015

Cleaned up some std::cout's used for debugging

803ef9c

slava77 reviewed Dec 4, 2015
View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Dec 4, 2015

Cleaned up some commented code

17295fd

cmsbuild added tests-pending and removed tests-started labels Dec 4, 2015

Update SiStripDigiToRaw.cc

847841c

cmsbuild added tests-rejected and removed tests-started labels Dec 9, 2015

cmsbuild added tests-pending and removed tests-rejected labels Dec 9, 2015

cmsbuild added tests-started and removed tests-pending labels Dec 9, 2015

cmsbuild added tests-approved and removed tests-started labels Dec 9, 2015

cmsbuild added comparison-available and removed comparison-pending labels Dec 10, 2015

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Dec 10, 2015

rkunnawa changed the title ~~SiStrip FED DQM error plot fix~~ Change of DigiToRaw code to keep SiStrip Error information during data repacking Dec 10, 2015

davidlange6 added a commit that referenced this pull request Dec 10, 2015

Merge pull request #12674 from rkunnawa/SiStrigFED_DQMFix

50c1aa6

Change of DigiToRaw code to keep SiStrip Error information during data repacking

davidlange6 merged commit 50c1aa6 into cms-sw:CMSSW_7_5_X Dec 10, 2015

abaty mentioned this pull request Dec 11, 2015

Fix for SiStripErrors in HI repacking (porting PR #12674) #12762

Merged

cmsbuild added a commit that referenced this pull request Dec 15, 2015

Merge pull request #12762 from abaty/CMSSW_8_0_0_pre2_SiStripErrors

313aba8

Fix for SiStripErrors in HI repacking (porting PR #12674)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change of DigiToRaw code to keep SiStrip Error information during data repacking #12674

Change of DigiToRaw code to keep SiStrip Error information during data repacking #12674

rkunnawa commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

abaty commented Dec 4, 2015

slava77 Dec 4, 2015

slava77 Dec 4, 2015

slava77 commented Dec 4, 2015

slava77 commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

rkunnawa commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

rkunnawa commented Dec 4, 2015

slava77 commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

amagnan commented Dec 9, 2015

cmsbuild commented Dec 9, 2015

slava77 commented Dec 9, 2015

cmsbuild commented Dec 9, 2015

cmsbuild commented Dec 9, 2015

cmsbuild commented Dec 10, 2015

slava77 commented Dec 10, 2015

rkunnawa commented Dec 10, 2015

Martin-Grunewald commented Dec 10, 2015

amagnan commented Dec 10, 2015

icali commented Dec 10, 2015

Martin-Grunewald commented Dec 10, 2015

abaty commented Dec 11, 2015

Change of DigiToRaw code to keep SiStrip Error information during data repacking #12674

Change of DigiToRaw code to keep SiStrip Error information during data repacking #12674

Conversation

rkunnawa commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

abaty commented Dec 4, 2015

slava77 Dec 4, 2015

Choose a reason for hiding this comment

slava77 Dec 4, 2015

Choose a reason for hiding this comment

slava77 commented Dec 4, 2015

slava77 commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

rkunnawa commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

rkunnawa commented Dec 4, 2015

slava77 commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

amagnan commented Dec 9, 2015

cmsbuild commented Dec 9, 2015

slava77 commented Dec 9, 2015

cmsbuild commented Dec 9, 2015

cmsbuild commented Dec 9, 2015

cmsbuild commented Dec 10, 2015

slava77 commented Dec 10, 2015

rkunnawa commented Dec 10, 2015

Martin-Grunewald commented Dec 10, 2015

amagnan commented Dec 10, 2015

icali commented Dec 10, 2015

Martin-Grunewald commented Dec 10, 2015

abaty commented Dec 11, 2015