Skip to content

update on strip approximate cluster #47367

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

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Configuration/Eras/python/Era_Run2024_approxSiStripCluster.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import FWCore.ParameterSet.Config as cms

from Configuration.Eras.Era_Run3_2024_cff import Run3_2024
from Configuration.ProcessModifiers.approxSiStripClusters_cff import approxSiStripClusters
from Configuration.ProcessModifiers.trackingNoLoopers_cff import trackingNoLoopers

Run3_pp_on_PbPb_approxSiStripClusters_2024 = cms.ModifierChain(Run3_2024.copyAndExclude([trackingNoLoopers]), approxSiStripClusters)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Run3_pp_on_PbPb_approxSiStripClusters_2024 = cms.ModifierChain(Run3_2024.copyAndExclude([trackingNoLoopers]), approxSiStripClusters)
Run3_2024_approxSiStripClusters = cms.ModifierChain(Run3_2024.copyAndExclude([trackingNoLoopers]), approxSiStripClusters)

what does this have to do specifically with pp_on_PbPb ?

Copy link
Author

Choose a reason for hiding this comment

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

nothing have to do with pp_on_PbPb, I add the suggestion to the PR.

15 changes: 11 additions & 4 deletions DataFormats/SiStripCluster/interface/SiStripApproximateCluster.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,20 @@
#define DataFormats_SiStripCluster_SiStripApproximateCluster_h

#include "FWCore/Utilities/interface/typedefs.h"
#include "assert.h"

class SiStripCluster;
class SiStripApproximateCluster {
public:
SiStripApproximateCluster() {}

explicit SiStripApproximateCluster(cms_uint16_t barycenter,
explicit SiStripApproximateCluster(cms_uint16_t compBarycenter,
cms_uint8_t width,
cms_uint8_t avgCharge,
bool filter,
bool isSaturated,
bool peakFilter = false)
: barycenter_(barycenter),
: compBarycenter_(compBarycenter),
width_(width),
avgCharge_(avgCharge),
filter_(filter),
Expand All @@ -26,20 +27,26 @@ class SiStripApproximateCluster {
float hitPredPos,
bool peakFilter);

cms_uint16_t barycenter() const { return barycenter_; }
float barycenter() const {
float _barycenter = compBarycenter_ * maxBarycenter_ / maxRange_;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please avoid underscore as the first character (2.14 in https://cms-sw.github.io/cms_coding_rules.html#2--naming-rules-1)

assert(_barycenter <= maxBarycenter_ && "Returning barycenter > maxBarycenter");
return _barycenter;
}
cms_uint8_t width() const { return width_; }
cms_uint8_t avgCharge() const { return avgCharge_; }
bool filter() const { return filter_; }
bool isSaturated() const { return isSaturated_; }
bool peakFilter() const { return peakFilter_; }

private:
cms_uint16_t barycenter_ = 0;
cms_uint16_t compBarycenter_ = 0;
cms_uint8_t width_ = 0;
cms_uint8_t avgCharge_ = 0;
bool filter_ = false;
bool isSaturated_ = false;
bool peakFilter_ = false;
static constexpr double maxRange_ = 65535.; //16 bit
static constexpr double maxBarycenter_ = 770.;
Copy link
Contributor

Choose a reason for hiding this comment

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

why 770 and not 768?

static constexpr double trimMaxADC_ = 30.;
static constexpr double trimMaxFracTotal_ = .15;
static constexpr double trimMaxFracNeigh_ = .25;
Expand Down
7 changes: 5 additions & 2 deletions DataFormats/SiStripCluster/src/SiStripApproximateCluster.cc
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
#include "DataFormats/SiStripCluster/interface/SiStripApproximateCluster.h"
#include "DataFormats/SiStripCluster/interface/SiStripCluster.h"
#include <algorithm>
#include <cassert>
#include <cmath>

SiStripApproximateCluster::SiStripApproximateCluster(const SiStripCluster& cluster,
unsigned int maxNSat,
float hitPredPos,
bool peakFilter) {
barycenter_ = std::round(cluster.barycenter() * 10);
compBarycenter_ = std::round(cluster.barycenter() * maxRange_ / maxBarycenter_);
assert(cluster.barycenter() <= maxBarycenter_ && "Got a barycenter > maxBarycenter");
assert(compBarycenter_ <= maxRange_ && "Filling compBarycenter > maxRange");
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this condition always true? (maximum possible value of compBaryCenter_ being 2**16-1 = 65535 and maxRange_ being set to 65535.)

width_ = cluster.size();
avgCharge_ = cluster.charge() / cluster.size();
avgCharge_ = (cluster.charge() + cluster.size() / 2) / cluster.size();
filter_ = false;
isSaturated_ = false;
peakFilter_ = peakFilter;
Expand Down
2 changes: 1 addition & 1 deletion DataFormats/SiStripCluster/src/SiStripCluster.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ SiStripCluster::SiStripCluster(const SiStripDigiRange& range) : firstStrip_(rang
}

SiStripCluster::SiStripCluster(const SiStripApproximateCluster cluster, const uint16_t maxStrips) : error_x(-99999.9) {
barycenter_ = cluster.barycenter() / 10.0;
barycenter_ = cluster.barycenter();
charge_ = cluster.width() * cluster.avgCharge();
amplitudes_.resize(cluster.width(), cluster.avgCharge());
filter_ = cluster.filter();
Expand Down
3 changes: 2 additions & 1 deletion DataFormats/SiStripCluster/src/classes_def.xml
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
<class name="edm::Wrapper<edmNew::DetSetVector<edm::Ref<edmNew::DetSetVector<SiStripCluster>,SiStripCluster,edmNew::DetSetVector<SiStripCluster>::FindForDetSetVector> > >" />


<class name="SiStripApproximateCluster" ClassVersion="6">
<class name="SiStripApproximateCluster" ClassVersion="7">
<version ClassVersion="7" checksum="3059068941"/>
<version ClassVersion="6" checksum="132211472"/>
<version ClassVersion="5" checksum="3495825183"/>
<version ClassVersion="4" checksum="2854791577"/>
Expand Down