Reclustering of SiPixelClusters

[ferencek]

This PR adds the necessary code to allow reclustering of already clustered SiPixelClusters. Its primary use case is to remake the pre-splitting SiPixelClusters from the split ones to allow re-running of the tracking sequence from the RECO data tier.

This PR slightly modifies the SiPixelCluster DataFormat code but without changing the format version. The only effect will be a slight increase in the size of the siPixelClusters collection. All other objects downstream from the pixel clusters should remain unchanged.

More details can be found in the following slides presented in the Pixel Offline meeting https://indico.cern.ch/event/536890/contributions/2361054/attachments/1365112/2067644/Proposed_SiPixelCluster_changes.pdf

Update (Nov. 7, 2016): Rebased to CMSSW_8_1_X_2016-11-07-1100

[cmsbuild]

A new Pull Request was created by @ferencek (Dinko Ferencek) for CMSSW_8_1_X.

It involves the following packages:

DataFormats/SiPixelCluster
RecoLocalTracker/SiPixelClusterizer

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @GiacomoSguazzoni, @rovere, @VinInn, @dkotlins, @gpetruc, @threus this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

[slava77]

@cmsbuild please test

@ferencek
Were you able to rerun the tracking sequence from RECO after this change?
Is your goal to stop at the generalTracks, or will you need more.

I plan to proceed with the review after TRK POG/DPG confirm the code changes are OK for them
@rovere, @VinInn, @dkotlins, @boudoul

[cmsbuild]

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

[ferencek]

@slava77, yes the tracking sequence runs. Here are the relevant bits of the configuration file I was running

process.load("Configuration.StandardSequences.MagneticField_cff")
process.load("Configuration.StandardSequences.GeometryRecoDB_cff")
process.load("Configuration.StandardSequences.Reconstruction_cff")

process.load("Configuration.StandardSequences.FrontierConditions_GlobalTag_cff")
from Configuration.AlCa.GlobalTag import GlobalTag
process.GlobalTag = GlobalTag(process.GlobalTag, 'auto:run2_mc', '')

process.siPixelClustersPreSplitting = process.siPixelClustersPreSplitting.clone(
    src = cms.InputTag('siPixelClusters::RECO'),
    ClusterMode = cms.untracked.string('PixelThresholdReclusterizer')
)

process.p = cms.Path(process.siPixelClustersPreSplitting*
                     process.siPixelRecHitsPreSplitting*
                     process.siStripMatchedRecHits*
                     process.MeasurementTrackerEventPreSplitting*
                     process.siPixelClusterShapeCachePreSplitting*
                     process.trackingGlobalReco*
                     process.vertexreco)

[cmsbuild]

-1

Tested at: 5471197

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

I found follow errors while testing this PR

Failed tests: UnitTests

• Unit Tests:

I found errors in the following unit tests:

---> test SiPixelCluster_t had ERRORS

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

[slava77]

the error looks related to this PR.
The failing unit test passes in the IB

[cmsbuild]

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

[ferencek]

@slava77, the failing unit test should be fixed now.

[cmsbuild]

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

[slava77]

@cmsbuild please test

[cmsbuild]

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

[cmsbuild]

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

[cvuosalo]

@ferencek: I tried running your config snippet (#16393 (comment)), but I got the error shown below.
Could you please provide a complete config for running the clustering, along with instructions?
Thanks.

----- Begin Fatal Exception 14-Nov-2016 23:05:30 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=SeedGeneratorFromRegionHitsEDProducer label='globalPixelSeeds'
Exception Message:
MissingParameter: Parameter 'maxElement' not found.
----- End Fatal Exception -------------------------------------------------

[ferencek]

@cvuosalo, try

import FWCore.ParameterSet.Config as cms

from FWCore.ParameterSet.VarParsing import VarParsing

###############################
####### Parameters ############
###############################

options = VarParsing ('python')

options.register('reportEvery', 10,
    VarParsing.multiplicity.singleton,
    VarParsing.varType.int,
    "Report every N events (default is N=10)"
)
options.register('wantSummary', False,
    VarParsing.multiplicity.singleton,
    VarParsing.varType.bool,
    "Print out trigger and timing summary"
)

## 'maxEvents' is already registered by the Framework, changing default value
options.setDefault('maxEvents', 500)

options.parseArguments()

process = cms.Process("USER")

process.load("FWCore.MessageService.MessageLogger_cfi")
process.MessageLogger.cerr.FwkReport.reportEvery = options.reportEvery


## Events to process
process.maxEvents = cms.untracked.PSet( input = cms.untracked.int32(options.maxEvents) )

## Input files
process.source = cms.Source("PoolSource",
    fileNames = cms.untracked.vstring(
        '/store/relval/CMSSW_8_0_11/RelValMinBias_13/GEN-SIM-RECO/80X_mcRun2_asymptotic_v14-v1/10000/162CE8B7-C434-E611-9817-0025905B85CA.root',
        '/store/relval/CMSSW_8_0_11/RelValMinBias_13/GEN-SIM-RECO/80X_mcRun2_asymptotic_v14-v1/10000/22285CD5-C134-E611-859E-0025905A609E.root',
        '/store/relval/CMSSW_8_0_11/RelValMinBias_13/GEN-SIM-RECO/80X_mcRun2_asymptotic_v14-v1/10000/589D37C4-C434-E611-89A4-0025905B8582.root',
        '/store/relval/CMSSW_8_0_11/RelValMinBias_13/GEN-SIM-RECO/80X_mcRun2_asymptotic_v14-v1/10000/8C7E5CD3-C134-E611-B836-0CC47A78A360.root',
        '/store/relval/CMSSW_8_0_11/RelValMinBias_13/GEN-SIM-RECO/80X_mcRun2_asymptotic_v14-v1/10000/A06BBC16-C334-E611-9D60-0CC47A78A4A6.root',
        '/store/relval/CMSSW_8_0_11/RelValMinBias_13/GEN-SIM-RECO/80X_mcRun2_asymptotic_v14-v1/10000/C48212E8-BF34-E611-B2FA-0CC47A78A360.root',
        '/store/relval/CMSSW_8_0_11/RelValMinBias_13/GEN-SIM-RECO/80X_mcRun2_asymptotic_v14-v1/10000/DAB4A04D-C034-E611-9674-0CC47A74524E.root',
        '/store/relval/CMSSW_8_0_11/RelValMinBias_13/GEN-SIM-RECO/80X_mcRun2_asymptotic_v14-v1/10000/EE1AB5E1-C034-E611-8ACF-0CC47A78A3E8.root'
    )
)

## Options and Output Report
process.options   = cms.untracked.PSet(
    wantSummary = cms.untracked.bool(options.wantSummary),
    allowUnscheduled = cms.untracked.bool(False)
)

## Load the full reconstraction configuration, to make sure we're getting all needed dependencies
process.load("Configuration.StandardSequences.MagneticField_cff")
process.load("Configuration.StandardSequences.GeometryRecoDB_cff")
process.load("Configuration.StandardSequences.Reconstruction_cff")

process.load("Configuration.StandardSequences.FrontierConditions_GlobalTag_cff")
from Configuration.AlCa.GlobalTag import GlobalTag
process.GlobalTag = GlobalTag(process.GlobalTag, 'auto:run2_mc', '')

process.siPixelClustersPreSplitting = process.siPixelClustersPreSplitting.clone(
    src = cms.InputTag('siPixelClusters::RECO'),
    ClusterMode = cms.untracked.string('PixelThresholdReclusterizer')
)

## Let it run
process.p = cms.Path(process.siPixelClustersPreSplitting*
                     process.siPixelRecHitsPreSplitting*
                     process.siStripMatchedRecHits*
                     process.MeasurementTrackerEventPreSplitting*
                     process.siPixelClusterShapeCachePreSplitting*
                     process.trackingGlobalReco*
                     process.vertexreco)

[cvuosalo]

@ferencek: What is the output of the config? I ran it, but it produced no files. How can I compare Reco before and after clustering?

[ferencek]

@cvuosalo, you can keep the output the usual way. Just add the following lines to the above configuration file

process.load('Configuration.EventContent.EventContent_cff')

process.RECOSIMoutput = cms.OutputModule("PoolOutputModule",
    dataset = cms.untracked.PSet(
        dataTier = cms.untracked.string(''),
        filterName = cms.untracked.string('')
    ),
    eventAutoFlushCompressedSize = cms.untracked.int32(5242880),
    fileName = cms.untracked.string('file:RECO.root'),
    outputCommands = process.RECOSIMEventContent.outputCommands,
    splitLevel = cms.untracked.int32(0)
)

process.RECOSIMoutput_step = cms.EndPath(process.RECOSIMoutput)

[cvuosalo]

The clustering config described above was run successfully. It added the following RECO event content:

-----------------------------------------------------------------
   or, B         new, B      delta, B   delta, %   deltaJ, %    branch 
-----------------------------------------------------------------
      0.0 ->       155.9        156     NEWO   0.08     recoVertexs_offlinePrimaryVerticesWithBS__USER.
      0.0 ->        35.4         35     NEWO   0.02     recoVertexsedmAssociation_offlinePrimaryVertices__USER.
      0.0 ->        25.9         26     NEWO   0.01     recoVertexCompositeCandidates_generalV0Candidates_Lambda_USER.
      0.0 ->        35.4         35     NEWO   0.02     recoDeDxHitInfosedmAssociation_dedxHitInfo__USER.
      0.0 ->        36.3         36     NEWO   0.02     TrackingRecHitsOwned_conversionStepTracks__USER.
      0.0 ->       271.7        272     NEWO   0.14     recoTrackExtrapolations_trackExtrapolator__USER.
      0.0 ->       156.7        157     NEWO   0.08     recoVertexs_offlinePrimaryVertices__USER.
      0.0 ->       120.2        120     NEWO   0.06     floats_generalTracks_MVAValues_USER.
      0.0 ->       156.6        157     NEWO   0.08     recoDeDxDataedmValueMap_dedxHarmonic2__USER.
      0.0 ->      5383.6       5384     NEWO   2.79     recoTrackExtras_generalTracks__USER.
      0.0 ->        41.4         41     NEWO   0.02     recoTracks_conversionStepTracks__USER.
      0.0 ->      3387.9       3388     NEWO   1.75     SiPixelClusteredmNewDetSetVector_siPixelClusters__USER.
      0.0 ->        33.4         33     NEWO   0.02     recoVertexCompositeCandidates_generalV0Candidates_Kshort_USER.
      0.0 ->        37.6         38     NEWO   0.02     intedmValueMap_offlinePrimaryVertices__USER.
      0.0 ->        28.4         28     NEWO   0.01     floatedmValueMap_offlinePrimaryVerticesWithBS__USER.
      0.0 ->        56.8         57     NEWO   0.03     recoTrackExtras_conversionStepTracks__USER.
      0.0 ->        38.3         38     NEWO   0.02     intedmValueMap_offlinePrimaryVerticesWithBS__USER.
      0.0 ->        23.3         23     NEWO   0.01     edmTriggerResults_TriggerResults__USER.
      0.0 ->       156.9        157     NEWO   0.08     recoDeDxDataedmValueMap_dedxTruncated40__USER.
      0.0 ->        36.1         36     NEWO   0.02     recoVertexsedmAssociation_offlinePrimaryVerticesWithBS__USER.
      0.0 ->        27.8         28     NEWO   0.01     floatedmValueMap_offlinePrimaryVertices__USER.
      0.0 ->      3073.7       3074     NEWO   1.59     recoTracks_generalTracks__USER.
      0.0 ->        31.3         31     NEWO   0.02     recoVertexs_inclusiveSecondaryVertices__USER.
      0.0 ->      2683.8       2684     NEWO   1.39     TrackingRecHitsOwned_generalTracks__USER.
      0.0 ->        19.9         20     NEWO   0.01     recoDeDxHitInfos_dedxHitInfo__USER.
-------------------------------------------------------------
   193287 ->      210626      17339             9.0     ALL BRANCHES

[ferencek]

As far as I see, there are no open issues with this PR. Any reason not to sign it?

[cvuosalo]

@ferencek: Tests and evaluation are still in progress.

[cvuosalo]

@ferencek: I am having difficulty assessing the effects of the clustering. How would you suggest to compare values before and after clustering?

[ferencek]

@cvuosalo, the first and most important thing is to verify that the code changes leave the output of the standard reconstruction unchanged. The second part would be to verify that tracks from re-tracking are the same as those from the standard reco.

[cvuosalo]

@ferencek: Are you saying the re-clustering should not actually change any Reco quantities?

[ferencek]

I'm not sure what you mean. Re-clustering of pixel clusters and re-tracking from RECO are mostly meant for validation and development purposes. So in that sense they are decoupled from the standard reco.

However, to enable re-clustering, the pixel clusterizer code was modified but all these changes should be transparent to the standard reco.

[cvuosalo]

+1

For #16393 5fe7d97

Enabling re-clustering of SiPixelClusters.

#16623 is the 90X version of this PR.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_8_1_X_2016-11-07-1100 show only tiny, insignificant differences. A test of a re-clustering config using a baseline from workflow 25202.0 shows it does not change Reco quantities, but only adds event content (see #16393 (comment)).

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

[davidlange6]

Hi @ferencek @slava77 @VinInn -since we lost a few weeks on the 63->127 change over the summer- can someone remind me why this limit is there in the first place?

[VinInn]

because of packing in the previous version. the hard limit is std::numeric_limits::max<uint8>

[ferencek]

@davidlange6, I did not participate in that discussion so I'm afraid I can't really say much about it. I understand why there was a limit of 63 in 80X since 6 bits from a 16-bit uint were used to encode the cluster span. In 81X the span is actually stored in a separate 8-bit uint so I don't really understand why the limit was not set to 255 in the first place instead of 127. In the presentiation linked in the PR description I show one example of a pixel cluster in MinBias simulation where even 127 is not enough to describe the cluster span. This is a pathological example but it still shows one limitation of the existing DataFormat. Using 255 makes sense from another point of view as well since the cluster size is limited to 256 pixels so the cluster span can be described even in the most pathological case (a straight line of 256 pixels).