Skip to content

Commit a927dcb

Browse files
sihyunjeonsihyunjeon
committed
address comments
1 parent 63fae6f commit a927dcb

File tree

6 files changed

+68
-105
lines changed

6 files changed

+68
-105
lines changed

‎DataFormats/Phase2TrackerDigi/interface/Phase2ITChip.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,9 @@
88

99
class Phase2ITChip {
1010
// Quarter cores collected into a chip (only active quarter cores with hits gets collected)
11-
std::vector<Phase2ITDigiHit> hitList;
12-
int rocnum_;
1311

1412
public:
15-
Phase2ITChip(int rocnum, std::vector<Phase2ITDigiHit> hl);
13+
Phase2ITChip(int rocnum, const std::vector<Phase2ITDigiHit> hl);
1614

1715
unsigned int size();
1816
int rocnum() const { return rocnum_; }
@@ -21,6 +19,9 @@ class Phase2ITChip {
2119
std::vector<bool> get_chip_code();
2220

2321
private:
22+
std::vector<Phase2ITDigiHit> hitList_;
23+
int rocnum_;
24+
2425
std::pair<int, int> get_QCore_pos(Phase2ITDigiHit hit);
2526

2627
Phase2ITQCore get_QCore_from_hit(Phase2ITDigiHit pixel);

‎DataFormats/Phase2TrackerDigi/interface/Phase2ITQCore.h‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,22 @@
44

55
class Phase2ITQCore {
66
// Collects hits and creates a quarter core (16 pixel positions)
7-
private:
8-
std::vector<int> adcs; // Full array of adc values in a quarter core
9-
std::vector<int> hits; // Full array of hit occurrences
10-
bool islast_; // RD53 chip encoding bits
11-
bool isneighbour_; // RD53 chip encoding bits
12-
int rocid_; // Chip index number
13-
int ccol; // QCore position column
14-
int qcrow; // QCore position row
157

168
public:
179
Phase2ITQCore(int rocid,
1810
int ccol_in,
1911
int qcrow_in,
2012
bool isneighbour_in,
2113
bool islast_in,
22-
std::vector<int> adcs_in,
23-
std::vector<int> hits_in);
14+
const std::vector<int>& adcs_in,
15+
const std::vector<int>& hits_in);
2416

2517
Phase2ITQCore() {
2618
rocid_ = -1;
2719
islast_ = false;
2820
isneighbour_ = false;
29-
ccol = -1;
30-
qcrow = -1;
21+
ccol_ = -1;
22+
qcrow_ = -1;
3123
}
3224

3325
void setIsLast(bool islast) { islast_ = islast; }
@@ -36,22 +28,30 @@ class Phase2ITQCore {
3628
void setIsNeighbour(bool isneighbour) { isneighbour_ = isneighbour; }
3729

3830
int rocid() const { return rocid_; }
39-
int get_col() const { return ccol; }
40-
int get_row() const { return qcrow; }
31+
int get_col() const { return ccol_; }
32+
int get_row() const { return qcrow_; }
4133

4234
std::vector<bool> getHitmap();
4335
std::vector<int> getADCs();
4436
std::vector<bool> encodeQCore(bool is_new_col);
4537

4638
const bool operator<(const Phase2ITQCore& other) {
47-
if (ccol == other.ccol) {
48-
return (ccol < other.ccol);
39+
if (ccol_ == other.ccol_) {
40+
return (ccol_ < other.ccol_);
4941
} else {
50-
return (qcrow < other.qcrow);
42+
return (qcrow_ < other.qcrow_);
5143
}
5244
}
5345

5446
private:
47+
std::vector<int> adcs_; // Full array of adc values in a quarter core
48+
std::vector<int> hits_; // Full array of hit occurrences
49+
bool islast_; // RD53 chip encoding bits
50+
bool isneighbour_; // RD53 chip encoding bits
51+
int rocid_; // Chip index number
52+
int ccol_; // QCore position column
53+
int qcrow_; // QCore position row
54+
5555
std::vector<bool> toRocCoordinates(std::vector<bool>& hitmap);
5656
std::vector<bool> intToBinary(int num, int length);
5757
bool containsHit(std::vector<bool>& hitmap);

‎DataFormats/Phase2TrackerDigi/src/Phase2ITChip.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
#include "DataFormats/Phase2TrackerDigi/interface/Phase2ITChip.h"
77
#include "DataFormats/Phase2TrackerDigi/interface/Phase2ITDigiHit.h"
88

9-
Phase2ITChip::Phase2ITChip(int rocnum, std::vector<Phase2ITDigiHit> hl) {
10-
hitList = hl;
9+
Phase2ITChip::Phase2ITChip(int rocnum, const std::vector<Phase2ITDigiHit> hl) {
10+
hitList_ = hl;
1111
rocnum_ = rocnum;
1212
}
1313

14-
unsigned int Phase2ITChip::size() { return hitList.size(); }
14+
unsigned int Phase2ITChip::size() { return hitList_.size(); }
1515

1616
//Returns the position (row,col) of the 4x4 QCores that contains a hit
1717
std::pair<int, int> Phase2ITChip::get_QCore_pos(Phase2ITDigiHit hit) {
@@ -25,7 +25,7 @@ Phase2ITQCore Phase2ITChip::get_QCore_from_hit(Phase2ITDigiHit pixel) {
2525
std::vector<int> adcs(16, 0), hits(16, 0);
2626
std::pair<int, int> pos = get_QCore_pos(pixel);
2727

28-
for (const auto& hit : hitList) {
28+
for (const auto& hit : hitList_) {
2929
if (get_QCore_pos(hit) == pos) {
3030
int i = (4 * (hit.row() % 4) + (hit.col() % 4) + 8) % 16;
3131
adcs[i] = hit.adc();
@@ -103,7 +103,7 @@ std::vector<Phase2ITQCore> link_QCores(std::vector<Phase2ITQCore> qcores) {
103103
std::vector<Phase2ITQCore> Phase2ITChip::get_organized_QCores() {
104104
std::vector<Phase2ITQCore> qcores = {};
105105

106-
for (const auto& hit : hitList) {
106+
for (const auto& hit : hitList_) {
107107
qcores.push_back(get_QCore_from_hit(hit));
108108
}
109109

@@ -114,7 +114,7 @@ std::vector<Phase2ITQCore> Phase2ITChip::get_organized_QCores() {
114114
std::vector<bool> Phase2ITChip::get_chip_code() {
115115
std::vector<bool> code = {};
116116

117-
if (hitList.size() > 0) {
117+
if (hitList_.size() > 0) {
118118
std::vector<Phase2ITQCore> qcores = get_organized_QCores();
119119
bool is_new_col = true;
120120

‎DataFormats/Phase2TrackerDigi/src/Phase2ITQCore.cc‎

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,20 @@ Phase2ITQCore::Phase2ITQCore(int rocid,
99
int qcrow_in,
1010
bool isneighbour_in,
1111
bool islast_in,
12-
std::vector<int> adcs_in,
13-
std::vector<int> hits_in) {
12+
const std::vector<int>& adcs_in,
13+
const std::vector<int>& hits_in) {
1414
rocid_ = rocid;
15-
ccol = ccol_in;
16-
qcrow = qcrow_in;
15+
ccol_ = ccol_in;
16+
qcrow_ = qcrow_in;
1717
isneighbour_ = isneighbour_in;
1818
islast_ = islast_in;
19-
adcs = adcs_in;
20-
hits = hits_in;
19+
adcs_ = adcs_in;
20+
hits_ = hits_in;
2121
}
2222

2323
//Takes a hitmap in sensor coordinates in 4x4 and converts it to readout chip coordinates with 2x8
2424
std::vector<bool> Phase2ITQCore::toRocCoordinates(std::vector<bool>& hitmap) {
25-
std::vector<bool> ROC_hitmap(16, 0);
25+
std::vector<bool> ROC_hitmap(16, false);
2626

2727
for (size_t i = 0; i < hitmap.size(); i++) {
2828
int row = std::floor(i / 4);
@@ -49,7 +49,7 @@ std::vector<bool> Phase2ITQCore::toRocCoordinates(std::vector<bool>& hitmap) {
4949
std::vector<bool> Phase2ITQCore::getHitmap() {
5050
std::vector<bool> hitmap = {};
5151

52-
for (auto hit : hits) {
52+
for (auto hit : hits_) {
5353
hitmap.push_back(hit > 0);
5454
}
5555

@@ -59,7 +59,7 @@ std::vector<bool> Phase2ITQCore::getHitmap() {
5959
std::vector<int> Phase2ITQCore::getADCs() {
6060
std::vector<int> adcmap = {};
6161

62-
for (auto adc : adcs) {
62+
for (auto adc : adcs_) {
6363
adcmap.push_back(adc);
6464
}
6565

@@ -68,16 +68,11 @@ std::vector<int> Phase2ITQCore::getADCs() {
6868

6969
//Converts an integer into a binary, and formats it with the given length
7070
std::vector<bool> Phase2ITQCore::intToBinary(int num, int length) {
71-
int n = num;
72-
std::vector<bool> bi_num(length, 0);
71+
std::vector<bool> bi_num(length, false);
7372

74-
for (int i = length; i > 0; i--) {
75-
if (n >= pow(2, i - 1)) {
76-
bi_num[length - i] = 1;
77-
n -= pow(2, i - 1);
78-
} else {
79-
bi_num[length - i] = 0;
80-
}
73+
for (int i = 0; i < length; ++i) {
74+
// Extract the (length - 1 - i)th bit from num
75+
bi_num[i] = (num >> (length - 1 - i)) & 1;
8176
}
8277

8378
return bi_num;
@@ -111,8 +106,8 @@ std::vector<bool> Phase2ITQCore::getHitmapCode(std::vector<bool> hitmap) {
111106
bool hit_right = containsHit(right_hitmap);
112107

113108
if (hit_left && hit_right) {
114-
code.push_back(1);
115-
code.push_back(1);
109+
code.push_back(true);
110+
code.push_back(true);
116111

117112
std::vector<bool> left_code = getHitmapCode(left_hitmap);
118113
std::vector<bool> right_code = getHitmapCode(right_hitmap);
@@ -122,14 +117,14 @@ std::vector<bool> Phase2ITQCore::getHitmapCode(std::vector<bool> hitmap) {
122117

123118
} else if (hit_right) {
124119
//Huffman encoding compresses 01 into 0
125-
code.push_back(0);
120+
code.push_back(false);
126121

127122
std::vector<bool> right_code = getHitmapCode(right_hitmap);
128123
code.insert(code.end(), right_code.begin(), right_code.end());
129124

130125
} else if (hit_left) {
131-
code.push_back(1);
132-
code.push_back(0);
126+
code.push_back(true);
127+
code.push_back(false);
133128

134129
std::vector<bool> left_code = getHitmapCode(left_hitmap);
135130
code.insert(code.end(), left_code.begin(), left_code.end());
@@ -143,22 +138,22 @@ std::vector<bool> Phase2ITQCore::encodeQCore(bool is_new_col) {
143138
std::vector<bool> code = {};
144139

145140
if (is_new_col) {
146-
std::vector<bool> col_code = intToBinary(ccol, 6);
141+
std::vector<bool> col_code = intToBinary(ccol_, 6);
147142
code.insert(code.end(), col_code.begin(), col_code.end());
148143
}
149144

150145
code.push_back(islast_);
151146
code.push_back(isneighbour_);
152147

153148
if (!isneighbour_) {
154-
std::vector<bool> row_code = intToBinary(qcrow, 8);
149+
std::vector<bool> row_code = intToBinary(qcrow_, 8);
155150
code.insert(code.end(), row_code.begin(), row_code.end());
156151
}
157152

158153
std::vector<bool> hitmap_code = getHitmapCode(getHitmap());
159154
code.insert(code.end(), hitmap_code.begin(), hitmap_code.end());
160155

161-
for (auto adc : adcs) {
156+
for (auto adc : adcs_) {
162157
std::vector<bool> adc_code = intToBinary(adc, 4);
163158
code.insert(code.end(), adc_code.begin(), adc_code.end());
164159
}

‎EventFilter/Phase2PixelRawToDigi/plugins/Phase2ITQCoreProducer.cc‎

Lines changed: 20 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,7 @@ class Phase2ITQCoreProducer : public edm::stream::EDProducer<> {
3838
~Phase2ITQCoreProducer() override = default;
3939

4040
private:
41-
virtual void beginJob(const edm::EventSetup&);
4241
virtual void produce(edm::Event&, const edm::EventSetup&);
43-
virtual void endJob();
4442

4543
const edm::InputTag src_;
4644
const edm::EDGetTokenT<edm::DetSetVector<PixelDigi>> pixelDigi_token_;
@@ -51,8 +49,8 @@ class Phase2ITQCoreProducer : public edm::stream::EDProducer<> {
5149

5250
Phase2ITQCoreProducer::Phase2ITQCoreProducer(const edm::ParameterSet& iConfig)
5351
: src_(iConfig.getParameter<edm::InputTag>("src")),
54-
pixelDigi_token_(consumes<edm::DetSetVector<PixelDigi>>(iConfig.getParameter<edm::InputTag>("siPixelDigi"))),
55-
tTopoToken_(esConsumes<TrackerTopology, TrackerTopologyRcd>()) {
52+
pixelDigi_token_(consumes(iConfig.getParameter<edm::InputTag>("siPixelDigi"))),
53+
tTopoToken_(esConsumes()) {
5654
produces<edm::DetSetVector<Phase2ITQCore>>();
5755
produces<edm::DetSetVector<Phase2ITChipBitStream>>();
5856
}
@@ -62,13 +60,13 @@ namespace {
6260
// Dimension for 4 chips module = 1354 X 434 = (672 + 5 + 672 + 5) X (216 + 1 + 216 + 1)
6361
// Spacing 1 in column and 5 in row is introduced for each chip in between
6462
// if neighboring chip exists
65-
const int kQCoresInChipRow = (672);
66-
const int kQCoresInChipColumn = (216);
67-
const int kQCoresInChipRowGap = (5);
68-
const int kQCoresInChipColumnGap = (10);
63+
constexpr int kQCoresInChipRow = (672);
64+
constexpr int kQCoresInChipColumn = (216);
65+
constexpr int kQCoresInChipRowGap = (5);
66+
constexpr int kQCoresInChipColumnGap = (10);
6967
} // namespace
7068

71-
Phase2ITDigiHit updateHitCoordinatesForLargePixels(Phase2ITDigiHit& hit) {
69+
void updateHitCoordinatesForLargePixels(Phase2ITDigiHit& hit) {
7270
/*
7371
In-place modification of Hit coordinates to take into account large pixels
7472
Hits corresponding to large pixels are remapped so they lie on the boundary of the chip
@@ -109,21 +107,18 @@ Phase2ITDigiHit updateHitCoordinatesForLargePixels(Phase2ITDigiHit& hit) {
109107

110108
hit.set_row(updated_row);
111109
hit.set_col(updated_col);
112-
113-
return hit;
114110
}
115111

116-
std::vector<Phase2ITDigiHit> adjustEdges(std::vector<Phase2ITDigiHit> hitList) {
112+
void adjustEdges(std::vector<Phase2ITDigiHit> hitList) {
117113
/*
118114
In-place modification of Hit coordinates to take into account large pixels
119115
*/
120116
std::for_each(hitList.begin(), hitList.end(), &updateHitCoordinatesForLargePixels);
121-
return hitList;
122117
}
123118

124-
std::vector<Phase2ITChip> splitByChip(std::vector<Phase2ITDigiHit> hitList) {
119+
std::vector<Phase2ITChip> splitByChip(const std::vector<Phase2ITDigiHit>& hitList) {
125120
// Split the hit list by read out chip
126-
std::vector<std::vector<Phase2ITDigiHit>> hits_per_chip(4);
121+
std::array<std::vector<Phase2ITDigiHit>, 4> hits_per_chip;
127122
for (auto hit : hitList) {
128123
int chip_index = (hit.col() < kQCoresInChipColumn) ? 0 : 1;
129124
if (hit.row() >= kQCoresInChipRow) {
@@ -134,6 +129,7 @@ std::vector<Phase2ITChip> splitByChip(std::vector<Phase2ITDigiHit> hitList) {
134129

135130
// Generate Phase2ITChip objects from the hit lists
136131
std::vector<Phase2ITChip> chips;
132+
chips.reserve(4);
137133
for (int chip_index = 0; chip_index < 4; chip_index++) {
138134
chips.push_back(Phase2ITChip(chip_index, hits_per_chip[chip_index]));
139135
}
@@ -142,16 +138,8 @@ std::vector<Phase2ITChip> splitByChip(std::vector<Phase2ITDigiHit> hitList) {
142138
}
143139

144140
std::vector<Phase2ITChip> processHits(std::vector<Phase2ITDigiHit> hitList) {
145-
std::vector<Phase2ITDigiHit> newHitList;
146-
newHitList = adjustEdges(hitList);
147-
148-
std::vector<Phase2ITChip> chips = splitByChip(newHitList);
149-
std::vector<bool> code;
150-
151-
for (size_t i = 0; i < chips.size(); i++) {
152-
Phase2ITChip chip = chips[i];
153-
code = chip.get_chip_code();
154-
}
141+
adjustEdges(hitList);
142+
std::vector<Phase2ITChip> chips = splitByChip(hitList);
155143

156144
return chips;
157145
}
@@ -167,13 +155,10 @@ void Phase2ITQCoreProducer::produce(edm::Event& iEvent, const edm::EventSetup& i
167155

168156
auto const& tTopo = iSetup.getData(tTopoToken_);
169157

170-
edm::Handle<edm::DetSetVector<PixelDigi>> pixelDigiHandle;
171-
iEvent.getByToken(pixelDigi_token_, pixelDigiHandle);
172-
173-
edm::DetSetVector<PixelDigi>::const_iterator iterDet;
174-
for (iterDet = pixelDigiHandle->begin(); iterDet != pixelDigiHandle->end(); iterDet++) {
175-
DetId tkId = iterDet->id;
176-
edm::DetSet<PixelDigi> theDigis = (*pixelDigiHandle)[tkId];
158+
auto pixelDigiHandle = iEvent.getHandle(pixelDigi_token_);
159+
const edm::DetSetVector<PixelDigi>& pixelDigi = *pixelDigiHandle;
160+
for (const auto& theDigis : pixelDigi) {
161+
DetId tkId = theDigis.id;
177162
std::vector<Phase2ITDigiHit> hitlist;
178163
std::vector<int> id;
179164

@@ -191,11 +176,11 @@ void Phase2ITQCoreProducer::produce(edm::Event& iEvent, const edm::EventSetup& i
191176
id = {tkId.subdetId(), module_num, disk_num, blade_num, panel_num, side_num};
192177
}
193178

194-
for (auto iterDigi = theDigis.begin(); iterDigi != theDigis.end(); ++iterDigi) {
195-
hitlist.emplace_back(Phase2ITDigiHit(iterDigi->row(), iterDigi->column(), iterDigi->adc()));
179+
for (const auto& digi : theDigis) {
180+
hitlist.emplace_back(digi.row(), digi.column(), digi.adc());
196181
}
197182

198-
std::vector<Phase2ITChip> chips = processHits(hitlist);
183+
std::vector<Phase2ITChip> chips = processHits(std::move(hitlist));
199184

200185
DetSet<Phase2ITQCore> DetSetQCores(tkId);
201186
DetSet<Phase2ITChipBitStream> DetSetBitStream(tkId);
@@ -219,8 +204,4 @@ void Phase2ITQCoreProducer::produce(edm::Event& iEvent, const edm::EventSetup& i
219204
iEvent.put(std::move(aBitStreamVector));
220205
}
221206

222-
void Phase2ITQCoreProducer::beginJob(edm::EventSetup const&) {}
223-
224-
void Phase2ITQCoreProducer::endJob() {}
225-
226207
DEFINE_FWK_MODULE(Phase2ITQCoreProducer);

0 commit comments

Comments
 (0)