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

70X mods to fix SiStrip Tracker PreMixing #5952

Conversation

mdhildreth
Copy link
Contributor

Add special 10-bit FED Readout Mode in simulation to preserve high pulse height hits in Tracker through the Digi to Raw step. Both Packer and Unpacker were modified to allow this (A.-M. Magnan). Added a new ZeroSuppression mode that merely passes any adc > 0 so that low-level hits are not lost. Because of all of this, the SiStrip Digitizer now needs to know that it is working in PreMixing mode, so as to not truncate all hits at ADC=256, so this switch was added. One customization added to ConfigBuilder so that the new PreMixing readout mode is enabled in the PreMixing DigiToRaw step.

@mdhildreth
Copy link
Contributor Author

Will do a few more tests before signing.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mdhildreth for CMSSW_7_0_X.

70X mods to fix SiStrip Tracker PreMixing

It involves the following packages:

Configuration/Applications
DataFormats/SiStripCommon
EventFilter/SiStripRawToDigi
RecoLocalTracker/SiStripZeroSuppression
SimGeneral/DataMixingModule
SimGeneral/MixingModule
SimTracker/SiStripDigitizer

@civanch, @nclopezo, @vlimant, @mdhildreth, @cmsbuild, @franzoni, @StoyanStoynev, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @nickmccoll, @jlagram, @wmtan, @makortel, @forthommel, @yduhm, @GiacomoSguazzoni, @gbenelli, @rovere, @VinInn, @appeltel, @cnuttens, @prolay, @gpetruc, @cerati, @venturia, @threus, @LBeck 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.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.


/// unpack -> add check to make sure strip < nstrips && strip > last strip......
while (unpacker.hasData()) {zs_work_digis_.push_back(SiStripDigi(unpacker.sampleNumber()+ipair*256,unpacker.adcPreMix()));unpacker++;}
} catch (const cms::Exception& e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wish we didn't do the try catch for things that wouldn't even become LogErrors.
Something to be handled in a more organized "campaign" at some point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree. This was the original structure, though, so we just added another block of code.

@slava77
Copy link
Contributor

slava77 commented Oct 22, 2014

@mdhildreth
Mike, is the logic for premixing development still "backwards", develop in an old release and forward port to a development release?

@cmsbuild
Copy link
Contributor

-1
Tested at: b1124a3
When I ran the RelVals I found an error in the following worklfows:
4.22 step2

runTheMatrix-results/4.22_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC/step2_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC.log

4.53 step2

runTheMatrix-results/4.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODreHLT+HARVESTDreHLT+MINIAODDATA/step2_RunPhoton2012B+RunPhoton2012B+HLTD+RECODreHLT+HARVESTDreHLT+MINIAODDATA.log

1000.0 step2

runTheMatrix-results/1000.0_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT/step2_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT.log

1001.0 step2

runTheMatrix-results/1001.0_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD/step2_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD.log

1003.0 step2

runTheMatrix-results/1003.0_RunMinBias2012A+RunMinBias2012A+RECODDQM+HARVESTDDQM/step2_RunMinBias2012A+RunMinBias2012A+RECODDQM+HARVESTDDQM.log

50101.0 step2

runTheMatrix-results/50101.0_SingleMuPt10+SingleMuPt10FSIdINPUT+SingleMuPt10FS_ID/step2_SingleMuPt10+SingleMuPt10FSIdINPUT+SingleMuPt10FS_ID.log

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

@davidlange6
Copy link
Contributor

for the moment, yes - then can validate and port in parallel.

On Oct 23, 2014, at 1:29 AM, Slava Krutelyov notifications@github.com wrote:

@mdhildreth
Mike, is the logic for premixing development still "backwards", develop in an old release and forward port to a development release?


Reply to this email directly or view it on GitHub.

@mdhildreth
Copy link
Contributor Author

I looked over the RelVals errors. I can't see any of them that relate to these changes. Several of them come from "input file not found" when looking at real data workflows. ??? Others happen in EM-ID shape fits in single muon events? What does this have to do with adc information in the tracker? Can we re-run these?

@mdhildreth
Copy link
Contributor Author

The runTheMatrix tests all pass in my release area.

@mdhildreth
Copy link
Contributor Author

I believe the runTheMatrix failures on this PR are spurious. See comments in git.

  • Mike

On Oct 23, 2014, at 6:58 AM, David Lange notifications@github.com wrote:

for the moment, yes - then can validate and port in parallel.

On Oct 23, 2014, at 1:29 AM, Slava Krutelyov notifications@github.com wrote:

@mdhildreth
Mike, is the logic for premixing development still "backwards", develop in an old release and forward port to a development release?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.


Mike Hildreth e-mail: mikeh@undhep.hep.nd.edu
Department of Physics mikeh@fnal.gov
225 Nieuwland Sciences Hall telephone: 574-631-6458 (office)
University of Notre Dame 574-631-5952 (FAX)
Notre Dame, IN 46556 http://www.hep.nd.edu/MikeHildreth

@slava77
Copy link
Contributor

slava77 commented Oct 24, 2014

+1

for #5952 b1124a3
tested in CMSSW_7_0_9_patch2 (test area sign709c)
no differences observed in the short matrix (not intended to test the premixing itself).

tests included

1000.0_RunMinBias2011A
1001.0_RunMinBias2011A
101.0_SingleElectronE120EHCAL
1102.0_RR
140.51_RunHI2010
200.0_ZEE
201.0_ZmumuJets_Pt_20_300
25.0_TTbar
4.17_RunMinBias2011A
4.22_RunCosmics2011A
4.29_RunMinBias2011B
4.37_ZMuSkim2011B
4.53_RunPhoton2012B
5.1_TTbar
50101.0_SingleMuPt10
8.0_BeamHalo

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