From 95379ec79075f2d90b7a2579022ef48612b7b70b Mon Sep 17 00:00:00 2001 From: Jaime Leon Date: Thu, 15 Sep 2022 17:56:12 +0200 Subject: [PATCH 1/4] Fix primos filter and added back good confirmation --- .../src/MPQualityEnhancerFilter.cc | 10 +- .../DTTriggerPhase2/src/MuonPathAssociator.cc | 106 +++++++++++++----- 2 files changed, 83 insertions(+), 33 deletions(-) diff --git a/L1Trigger/DTTriggerPhase2/src/MPQualityEnhancerFilter.cc b/L1Trigger/DTTriggerPhase2/src/MPQualityEnhancerFilter.cc index 0a76fa7cffa6f..2802ca13a0cbb 100644 --- a/L1Trigger/DTTriggerPhase2/src/MPQualityEnhancerFilter.cc +++ b/L1Trigger/DTTriggerPhase2/src/MPQualityEnhancerFilter.cc @@ -120,9 +120,13 @@ void MPQualityEnhancerFilter::filterCousins(std::vector &inMPaths bestI = i; } } - if (areCousins(inMPaths[i], inMPaths[i + 1]) != 0) { - primo_index++; - } else { //areCousing==0 + bool add_paths = (i == (int)(inMPaths.size() - 1)); + if (!add_paths) { + add_paths = areCousins(inMPaths[i], inMPaths[i + 1]) == 0; + } + if (!add_paths) { + primo_index++; + } else { //areCousing==0 if (oneof4) { outMPaths.push_back(inMPaths[bestI]); bestI = -1; diff --git a/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc b/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc index 7c4e3560aeb9a..dddc6e3f229c1 100644 --- a/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc +++ b/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc @@ -375,7 +375,7 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, for (const auto &dtLayerId_It : *dtdigis) { const DTLayerId dtLId = dtLayerId_It.first; // creating a new DTSuperLayerId object to compare with the required SL id - const DTSuperLayerId dtSLId(dtLId.wheel(), dtLId.station(), dtLId.sector(), 3); + const DTSuperLayerId dtSLId(dtLId.wheel(), dtLId.station(), dtLId.sector(), dtLId.superLayer()); if (dtSLId.rawId() != sl3Id.rawId()) continue; double l_shift = 0; @@ -390,6 +390,8 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, double x_inSL3 = SL1metaPrimitive->x - SL1metaPrimitive->tanPhi * (VERT_PHI1_PHI3 + l_shift); for (auto digiIt = (dtLayerId_It.second).first; digiIt != (dtLayerId_It.second).second; ++digiIt) { DTWireId wireId(dtLId, (*digiIt).wire()); + if ((*digiIt).time() < SL1metaPrimitive->t0) + continue; double x_wire = shiftinfo_[wireId.rawId()] + ((*digiIt).time() - SL1metaPrimitive->t0) * DRIFT_SPEED / 10.; double x_wire_left = @@ -398,27 +400,47 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, if (std::abs(x_inSL3 - x_wire) > std::abs(x_inSL3 - x_wire_left)) { x_wire = x_wire_left; //choose the closest laterality lat = 0; - } + } if (std::abs(x_inSL3 - x_wire) < minx) { - min2x = minx; - minx = std::abs(x_inSL3 - x_wire); - next_wire = best_wire; - next_tdc = best_tdc; - next_layer = best_layer; - next_lat = best_lat; - - best_wire = (*digiIt).wire(); - best_tdc = (*digiIt).time(); - best_layer = dtLId.layer(); - best_lat = lat; - matched_digis++; + // different layer than the stored in best, hit added, matched_digis++;. This approach in somewhat + // buggy, as we could have stored as best LayerX -> LayerY -> LayerX, and this should + // count only as 2 hits. However, as we confirm with at least 2 hits, having 2 or more + // makes no difference + if (dtLId.layer() != best_layer) { + minx = std::abs(x_inSL3 - x_wire); + next_wire = best_wire; + next_tdc = best_tdc; + next_layer = best_layer; + next_lat = best_lat; + + best_wire = (*digiIt).wire(); + best_tdc = (*digiIt).time(); + best_layer = dtLId.layer(); + best_lat = lat; + matched_digis++; + } else if (dtLId.layer() == best_layer) { // same layer than stored, just substituting the hit, no matched_digis++; + best_wire = (*digiIt).wire(); + best_tdc = (*digiIt).time(); + best_layer = dtLId.layer(); + best_lat = lat; + } } else if ((std::abs(x_inSL3 - x_wire) >= minx) && (std::abs(x_inSL3 - x_wire) < min2x)) { + // same layer than the stored in best, no hit added + if (dtLId.layer() == best_layer) + continue; + // different layer than the stored in next, hit added. This approach in somewhat + // buggy, as we could have stored as next LayerX -> LayerY -> LayerX, and this should + // count only as 2 hits. However, as we confirm with at least 2 hits, having 2 or more + // makes no difference + else if (dtLId.layer() != next_layer) + matched_digis++; + // whether the layer is the same for this hit and the stored in next, we substitute + // the one stored and modify the min distance min2x = std::abs(x_inSL3 - x_wire); next_wire = (*digiIt).wire(); next_tdc = (*digiIt).time(); next_layer = dtLId.layer(); next_lat = lat; - matched_digis++; } } } @@ -587,7 +609,7 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, for (const auto &dtLayerId_It : *dtdigis) { const DTLayerId dtLId = dtLayerId_It.first; // creating a new DTSuperLayerId object to compare with the required SL id - const DTSuperLayerId dtSLId(dtLId.wheel(), dtLId.station(), dtLId.sector(), 1); + const DTSuperLayerId dtSLId(dtLId.wheel(), dtLId.station(), dtLId.sector(), dtLId.superLayer()); if (dtSLId.rawId() != sl1Id.rawId()) continue; double l_shift = 0; @@ -602,6 +624,8 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, double x_inSL1 = SL3metaPrimitive->x + SL3metaPrimitive->tanPhi * (VERT_PHI1_PHI3 - l_shift); for (auto digiIt = (dtLayerId_It.second).first; digiIt != (dtLayerId_It.second).second; ++digiIt) { DTWireId wireId(dtLId, (*digiIt).wire()); + if ((*digiIt).time() < SL3metaPrimitive->t0) + continue; double x_wire = shiftinfo_[wireId.rawId()] + ((*digiIt).time() - SL3metaPrimitive->t0) * DRIFT_SPEED / 10.; double x_wire_left = @@ -612,24 +636,45 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, lat = 0; } if (std::abs(x_inSL1 - x_wire) < minx) { - minx = std::abs(x_inSL1 - x_wire); - next_wire = best_wire; - next_tdc = best_tdc; - next_layer = best_layer; - next_lat = best_lat; - - best_wire = (*digiIt).wire(); - best_tdc = (*digiIt).time(); - best_layer = dtLId.layer(); - best_lat = lat; - matched_digis++; + // different layer than the stored in best, hit added, matched_digis++;. This approach in somewhat + // buggy, as we could have stored as best LayerX -> LayerY -> LayerX, and this should + // count only as 2 hits. However, as we confirm with at least 2 hits, having 2 or more + // makes no difference + if (dtLId.layer() != best_layer) { + minx = std::abs(x_inSL1 - x_wire); + next_wire = best_wire; + next_tdc = best_tdc; + next_layer = best_layer; + next_lat = best_lat; + + best_wire = (*digiIt).wire(); + best_tdc = (*digiIt).time(); + best_layer = dtLId.layer(); + best_lat = lat; + matched_digis++; + } else if (dtLId.layer() == best_layer) { // same layer than stored, just substituting the hit, no matched_digis++; + best_wire = (*digiIt).wire(); + best_tdc = (*digiIt).time(); + best_layer = dtLId.layer(); + best_lat = lat; + } } else if ((std::abs(x_inSL1 - x_wire) >= minx) && (std::abs(x_inSL1 - x_wire) < min2x)) { - minx = std::abs(x_inSL1 - x_wire); + // same layer than the stored in best, no hit added + if (dtLId.layer() == best_layer) + continue; + // different layer than the stored in next, hit added. This approach in somewhat + // buggy, as we could have stored as next LayerX -> LayerY -> LayerX, and this should + // count only as 2 hits. However, as we confirm with at least 2 hits, having 2 or more + // makes no difference + else if (dtLId.layer() != next_layer) + matched_digis++; + // whether the layer is the same for this hit and the stored in next, we substitute + // the one stored and modify the min distance + min2x = std::abs(x_inSL1 - x_wire); next_wire = (*digiIt).wire(); next_tdc = (*digiIt).time(); next_layer = dtLId.layer(); next_lat = lat; - matched_digis++; } } } @@ -939,7 +984,8 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, for (auto metaprimitiveIt = inMPaths.begin(); metaprimitiveIt != inMPaths.end(); ++metaprimitiveIt) if (metaprimitiveIt->rawId == sl2Id.rawId()) { SL2metaPrimitives.push_back(*metaprimitiveIt); - printmPC(*metaprimitiveIt); + if (debug_) + printmPC(*metaprimitiveIt); outMPaths.push_back(*metaprimitiveIt); } } From 98b9e6e28937acf93e84493e7c32548d0c8bc086 Mon Sep 17 00:00:00 2001 From: Jaime Leon Date: Fri, 16 Sep 2022 09:44:16 +0200 Subject: [PATCH 2/4] Remove spaces --- L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc b/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc index dddc6e3f229c1..549038b4792e0 100644 --- a/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc +++ b/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc @@ -400,7 +400,7 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, if (std::abs(x_inSL3 - x_wire) > std::abs(x_inSL3 - x_wire_left)) { x_wire = x_wire_left; //choose the closest laterality lat = 0; - } + } if (std::abs(x_inSL3 - x_wire) < minx) { // different layer than the stored in best, hit added, matched_digis++;. This approach in somewhat // buggy, as we could have stored as best LayerX -> LayerY -> LayerX, and this should From a5f5d8197e23a9c4f742adc1cf0ee806b3357188 Mon Sep 17 00:00:00 2001 From: Jaime Leon Date: Fri, 16 Sep 2022 10:16:40 +0200 Subject: [PATCH 3/4] Code-checks and code-format --- .../DTTriggerPhase2/src/MPQualityEnhancerFilter.cc | 12 ++++++------ .../DTTriggerPhase2/src/MuonPathAssociator.cc | 14 ++++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/L1Trigger/DTTriggerPhase2/src/MPQualityEnhancerFilter.cc b/L1Trigger/DTTriggerPhase2/src/MPQualityEnhancerFilter.cc index 2802ca13a0cbb..94b72556b9dc3 100644 --- a/L1Trigger/DTTriggerPhase2/src/MPQualityEnhancerFilter.cc +++ b/L1Trigger/DTTriggerPhase2/src/MPQualityEnhancerFilter.cc @@ -121,12 +121,12 @@ void MPQualityEnhancerFilter::filterCousins(std::vector &inMPaths } } bool add_paths = (i == (int)(inMPaths.size() - 1)); - if (!add_paths) { - add_paths = areCousins(inMPaths[i], inMPaths[i + 1]) == 0; - } - if (!add_paths) { - primo_index++; - } else { //areCousing==0 + if (!add_paths) { + add_paths = areCousins(inMPaths[i], inMPaths[i + 1]) == 0; + } + if (!add_paths) { + primo_index++; + } else { //areCousing==0 if (oneof4) { outMPaths.push_back(inMPaths[bestI]); bestI = -1; diff --git a/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc b/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc index 549038b4792e0..0f406b32c47bf 100644 --- a/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc +++ b/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc @@ -418,7 +418,8 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, best_layer = dtLId.layer(); best_lat = lat; matched_digis++; - } else if (dtLId.layer() == best_layer) { // same layer than stored, just substituting the hit, no matched_digis++; + } else if (dtLId.layer() == + best_layer) { // same layer than stored, just substituting the hit, no matched_digis++; best_wire = (*digiIt).wire(); best_tdc = (*digiIt).time(); best_layer = dtLId.layer(); @@ -426,13 +427,13 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, } } else if ((std::abs(x_inSL3 - x_wire) >= minx) && (std::abs(x_inSL3 - x_wire) < min2x)) { // same layer than the stored in best, no hit added - if (dtLId.layer() == best_layer) + if (dtLId.layer() == best_layer) continue; // different layer than the stored in next, hit added. This approach in somewhat // buggy, as we could have stored as next LayerX -> LayerY -> LayerX, and this should // count only as 2 hits. However, as we confirm with at least 2 hits, having 2 or more // makes no difference - else if (dtLId.layer() != next_layer) + else if (dtLId.layer() != next_layer) matched_digis++; // whether the layer is the same for this hit and the stored in next, we substitute // the one stored and modify the min distance @@ -652,7 +653,8 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, best_layer = dtLId.layer(); best_lat = lat; matched_digis++; - } else if (dtLId.layer() == best_layer) { // same layer than stored, just substituting the hit, no matched_digis++; + } else if (dtLId.layer() == + best_layer) { // same layer than stored, just substituting the hit, no matched_digis++; best_wire = (*digiIt).wire(); best_tdc = (*digiIt).time(); best_layer = dtLId.layer(); @@ -660,13 +662,13 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, } } else if ((std::abs(x_inSL1 - x_wire) >= minx) && (std::abs(x_inSL1 - x_wire) < min2x)) { // same layer than the stored in best, no hit added - if (dtLId.layer() == best_layer) + if (dtLId.layer() == best_layer) continue; // different layer than the stored in next, hit added. This approach in somewhat // buggy, as we could have stored as next LayerX -> LayerY -> LayerX, and this should // count only as 2 hits. However, as we confirm with at least 2 hits, having 2 or more // makes no difference - else if (dtLId.layer() != next_layer) + else if (dtLId.layer() != next_layer) matched_digis++; // whether the layer is the same for this hit and the stored in next, we substitute // the one stored and modify the min distance From 572ce9665f2baaa77e2fce1ba50fe93e99a38429 Mon Sep 17 00:00:00 2001 From: Jaime Leon Date: Mon, 19 Sep 2022 14:28:00 +0200 Subject: [PATCH 4/4] Modifications from PR --- .../DTTriggerPhase2/src/MuonPathAssociator.cc | 39 ++++++------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc b/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc index 0f406b32c47bf..bcb8a6f62c4dc 100644 --- a/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc +++ b/L1Trigger/DTTriggerPhase2/src/MuonPathAssociator.cc @@ -133,7 +133,7 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, int sl1 = 0; int sl3 = 0; for (auto SL1metaPrimitive = SL1metaPrimitives.begin(); SL1metaPrimitive != SL1metaPrimitives.end(); - ++SL1metaPrimitive, sl1++, sl3 = -1) { + ++SL1metaPrimitive, sl1++, sl3 = 0) { if (clean_chi2_correlation_) at_least_one_correlation = false; for (auto SL3metaPrimitive = SL3metaPrimitives.begin(); SL3metaPrimitive != SL3metaPrimitives.end(); @@ -412,19 +412,13 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, next_tdc = best_tdc; next_layer = best_layer; next_lat = best_lat; - - best_wire = (*digiIt).wire(); - best_tdc = (*digiIt).time(); - best_layer = dtLId.layer(); - best_lat = lat; matched_digis++; - } else if (dtLId.layer() == - best_layer) { // same layer than stored, just substituting the hit, no matched_digis++; - best_wire = (*digiIt).wire(); - best_tdc = (*digiIt).time(); - best_layer = dtLId.layer(); - best_lat = lat; } + best_wire = (*digiIt).wire(); + best_tdc = (*digiIt).time(); + best_layer = dtLId.layer(); + best_lat = lat; + } else if ((std::abs(x_inSL3 - x_wire) >= minx) && (std::abs(x_inSL3 - x_wire) < min2x)) { // same layer than the stored in best, no hit added if (dtLId.layer() == best_layer) @@ -433,8 +427,7 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, // buggy, as we could have stored as next LayerX -> LayerY -> LayerX, and this should // count only as 2 hits. However, as we confirm with at least 2 hits, having 2 or more // makes no difference - else if (dtLId.layer() != next_layer) - matched_digis++; + matched_digis++; // whether the layer is the same for this hit and the stored in next, we substitute // the one stored and modify the min distance min2x = std::abs(x_inSL3 - x_wire); @@ -647,19 +640,12 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, next_tdc = best_tdc; next_layer = best_layer; next_lat = best_lat; - - best_wire = (*digiIt).wire(); - best_tdc = (*digiIt).time(); - best_layer = dtLId.layer(); - best_lat = lat; matched_digis++; - } else if (dtLId.layer() == - best_layer) { // same layer than stored, just substituting the hit, no matched_digis++; - best_wire = (*digiIt).wire(); - best_tdc = (*digiIt).time(); - best_layer = dtLId.layer(); - best_lat = lat; } + best_wire = (*digiIt).wire(); + best_tdc = (*digiIt).time(); + best_layer = dtLId.layer(); + best_lat = lat; } else if ((std::abs(x_inSL1 - x_wire) >= minx) && (std::abs(x_inSL1 - x_wire) < min2x)) { // same layer than the stored in best, no hit added if (dtLId.layer() == best_layer) @@ -668,8 +654,7 @@ void MuonPathAssociator::correlateMPaths(edm::Handle dtdigis, // buggy, as we could have stored as next LayerX -> LayerY -> LayerX, and this should // count only as 2 hits. However, as we confirm with at least 2 hits, having 2 or more // makes no difference - else if (dtLId.layer() != next_layer) - matched_digis++; + matched_digis++; // whether the layer is the same for this hit and the stored in next, we substitute // the one stored and modify the min distance min2x = std::abs(x_inSL1 - x_wire);