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

Flag candidate merged clusters for relaxed CPE errors #6348

Closed
wants to merge 5 commits into from

Conversation

wmtford
Copy link
Contributor

@wmtford wmtford commented Nov 12, 2014

Add a function "refineCluster" in SiStripClusterizer to mark SiStrip clusters as merged. Use this information StripCPEfromTrackAngle to fall back to the larger "legacy" CPE errors for these clusters. The flag lives on the high bit of the firstStrip_ private data member of the SiStripCluster. Configurable thresholds in the sensor occupancy and strip width are used to discriminate between un- and merged candidates. The cluster refiner is turned off as configured for this PR.

@cmsbuild
Copy link
Contributor

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

Flag candidate merged clusters for relaxed CPE errors

It involves the following packages:

DataFormats/SiStripCluster
RecoLocalTracker/SiStripClusterizer
RecoLocalTracker/SiStripRecHitConverter

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

@wmtford
Copy link
Contributor Author

wmtford commented Nov 12, 2014

Closing in favor of PR #6349, which based on a newer release, and is mergeable.

@wmtford wmtford closed this Nov 12, 2014
SiStripDetInfoFileReader* reader = 0;
if (doRefineCluster_) {
es.get<SiStripQualityRcd>().get("", quality);
reader = edm::Service<SiStripDetInfoFileReader>().operator->();
Copy link
Contributor

Choose a reason for hiding this comment

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

@slava77 does the SiStripDetInfoFileReader Service affect the physics results? If so, then this is an illegal use of a Service.

Copy link
Contributor

Choose a reason for hiding this comment

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

Chris,
do you mean that we should use ES or some plugin factory?

Copy link
Contributor

Choose a reason for hiding this comment

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

Either of those two or it might be possible to just make it a member data of the producer itself.

@wmtford wmtford deleted the mergedCPE branch November 24, 2014 18:57
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

4 participants