From 079e91ac59e27c876c8f45adafadd6c26f445ced Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 15 Oct 2024 17:33:16 +0000 Subject: [PATCH 1/8] [pre-commit.ci] pre-commit autoupdate (#1626) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/pre-commit/pre-commit-hooks: v4.6.0 → v5.0.0](https://github.com/pre-commit/pre-commit-hooks/compare/v4.6.0...v5.0.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Dmitry Kalinkin --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cf215b1c65..033cb95c37 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -2,7 +2,7 @@ ci: skip: [clang-format] repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.6.0 + rev: v5.0.0 hooks: - id: end-of-file-fixer - id: trailing-whitespace From a0e235a4db25481fa87604d50bf6f1ae03aa3e81 Mon Sep 17 00:00:00 2001 From: Dmitry Kalinkin Date: Tue, 15 Oct 2024 14:23:53 -0400 Subject: [PATCH 2/8] EEEMCal: remove 2% smearing (#1633) This comes from Athena times where all calorimeters were smeared by 2%. We don't expect this term, we want to have something more motivated like #1064, instead. Submitting separately to confirm the effect. --- src/detectors/EEMC/EEMC.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/detectors/EEMC/EEMC.cc b/src/detectors/EEMC/EEMC.cc index 9604b03090..8908c7ac72 100644 --- a/src/detectors/EEMC/EEMC.cc +++ b/src/detectors/EEMC/EEMC.cc @@ -40,7 +40,7 @@ extern "C" { {"EcalEndcapNRawHits"}, #endif { - .eRes = {0.0 * sqrt(dd4hep::GeV), 0.02, 0.0 * dd4hep::GeV}, + .eRes = {0.0 * sqrt(dd4hep::GeV), 0.0, 0.0 * dd4hep::GeV}, .tRes = 0.0 * dd4hep::ns, .threshold = 0.0 * dd4hep::MeV, // Use ADC cut instead .capADC = EcalEndcapN_capADC, From 0d35c7d112600ec4dcfefb1958dcfaa0ebe79faa Mon Sep 17 00:00:00 2001 From: Alex Jentsch Date: Wed, 16 Oct 2024 13:05:55 -0400 Subject: [PATCH 3/8] Fix RP reco bug in p_y (#1634) ### Briefly, what does this PR introduce? This PR introduces a bug fix for the p_y reconstruction in the Roman pots, and also switches the reco to global coordinates. There are some issues with the basic RP concept and how local coordinates are calculated using the cellIDs. It means an additional set of shifts would need to be calculated to do a proper transformation to the coordinate system local to the proton orbit. Using global coordinates simplifies this to the same procedure as before, but with different offsets (the matrices themselves don't actually change). ### What kind of change does this PR introduce? - [ xx] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [x ] Tests for the changes have been added - [x] Documentation has been added / updated - [x] Changes have been communicated to collaborators See attached slides for some discussion and comparison plots. ### Does this PR introduce breaking changes? What changes might users need to make to their code? No, it fixes the default momentum reconstruction to be correct (within the limits of how the static matrix approach works). Users will only need to change the axis limits on their 2D plots of the hits on the Roman pots (see the Roman pots geometry definition to pick a more-reasonable range). ### Does this PR change default behavior? Yes, it improves the pT reconstruction for the Roman pots protons. [ePIC_roman_pots_reco_matrix_bug_fix_10_15_2024.pdf](https://github.com/user-attachments/files/17382021/ePIC_roman_pots_reco_matrix_bug_fix_10_15_2024.pdf) --------- Co-authored-by: Alexander Jentsch Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .../fardetectors/MatrixTransferStatic.cc | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/algorithms/fardetectors/MatrixTransferStatic.cc b/src/algorithms/fardetectors/MatrixTransferStatic.cc index 48331e6c67..01b933806a 100644 --- a/src/algorithms/fardetectors/MatrixTransferStatic.cc +++ b/src/algorithms/fardetectors/MatrixTransferStatic.cc @@ -81,10 +81,10 @@ void eicrecon::MatrixTransferStatic::process( aY[1][0] = 0.0204108951; //c aY[1][1] = -0.139318692; //d - local_x_offset = -0.339334; - local_y_offset = -0.000299454; - local_x_slope_offset = -0.219603248; - local_y_slope_offset = -0.000176128; + local_x_offset = -1209.29;//-0.339334; these are the local coordinate values + local_y_offset = 0.00132511;//-0.000299454; + local_x_slope_offset = -45.4772;//-0.219603248; + local_y_slope_offset = 0.000745498;//-0.000176128; } else if(abs(100.0 - nomMomentum)/100.0 < nomMomentumError){ @@ -99,10 +99,10 @@ void eicrecon::MatrixTransferStatic::process( aY[1][0] = 0.0226283320; //c aY[1][1] = -0.082666019; //d - local_x_offset = -0.329072; - local_y_offset = -0.00028343; - local_x_slope_offset = -0.218525084; - local_y_slope_offset = -0.00015321; + local_x_offset = -1209.27;//-0.329072; + local_y_offset = 0.00355218;//-0.00028343; + local_x_slope_offset = -45.4737;//-0.218525084; + local_y_slope_offset = 0.00204394;//-0.00015321; } else if(abs(41.0 - nomMomentum)/41.0 < nomMomentumError){ @@ -117,10 +117,10 @@ void eicrecon::MatrixTransferStatic::process( aY[1][0] = 0.0179664765; //c aY[1][1] = 0.004160679; //d - local_x_offset = -0.283273; - local_y_offset = -0.00552451; - local_x_slope_offset = -0.21174031; - local_y_slope_offset = -0.003212011; + local_x_offset = -1209.22;//-0.283273; + local_y_offset = 0.00868737;//-0.00552451; + local_x_slope_offset = -45.4641;//-0.21174031; + local_y_slope_offset = 0.00498786;//-0.003212011; } else if(abs(135.0 - nomMomentum)/135.0 < nomMomentumError){ //135 GeV deuterons @@ -202,16 +202,16 @@ void eicrecon::MatrixTransferStatic::process( if(!goodHit2 && gpos.z() > m_cfg.hit2minZ && gpos.z() < m_cfg.hit2maxZ){ - goodHit[1].x = pos0.x(); - goodHit[1].y = pos0.y(); - goodHit[1].z = gpos.z(); + goodHit[1].x = gpos.x(); //pos0.x() - pos0 is local coordinates, gpos is global + goodHit[1].y = gpos.y(); //pos0.y() - temporarily changing to global to solve the local coordinate issue + goodHit[1].z = gpos.z(); // - which is unique to the Roman pots situation goodHit2 = true; } if(!goodHit1 && gpos.z() > m_cfg.hit1minZ && gpos.z() < m_cfg.hit1maxZ){ - goodHit[0].x = pos0.x(); - goodHit[0].y = pos0.y(); + goodHit[0].x = gpos.x(); //pos0.x() + goodHit[0].y = gpos.y(); //pos0.y() goodHit[0].z = gpos.z(); goodHit1 = true; @@ -252,6 +252,8 @@ void eicrecon::MatrixTransferStatic::process( } } + Yip[1] = Yrp[0]/aY[0][1]; + // convert polar angles to radians double rsx = Xip[1] * dd4hep::mrad; double rsy = Yip[1] * dd4hep::mrad; From 468c0a74e1fd37450c16f92279fbaee7952d21ba Mon Sep 17 00:00:00 2001 From: Michael Pitt Date: Wed, 16 Oct 2024 22:18:33 +0300 Subject: [PATCH 4/8] update dynamic range of B0 ECAL (#1623) ### Briefly, what does this PR introduce? This RP solves the truncation in energy response of the B0 calorimeter as reported in slide 11 [here](https://indico.bnl.gov/event/24923/#12-b0-emcal). The energy threshold based on the simulated light yields of calorimeter crystals interfaces with the readout (slide 12 from the presentation above) ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [X] Other: update thresholds in reconstruction ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? no ### Does this PR change default behavior? restore correct energy response for B0 ECAL. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- src/detectors/B0ECAL/B0ECAL.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/detectors/B0ECAL/B0ECAL.cc b/src/detectors/B0ECAL/B0ECAL.cc index 9d912a8221..10e8be761c 100644 --- a/src/detectors/B0ECAL/B0ECAL.cc +++ b/src/detectors/B0ECAL/B0ECAL.cc @@ -33,11 +33,12 @@ extern "C" { {"B0ECalRawHits"}, #endif { - .eRes = {0.0 * sqrt(dd4hep::GeV), 0.02, 0.0 * dd4hep::GeV}, + // The stocastic term is set using light yield in PbOW4 of N_photons = 145.75 / GeV / mm, for 6x6 mm2 sensors with PDE=0.18 (a=1/sqrt(145.75*36*0.18)) + .eRes = {0.0326 * sqrt(dd4hep::GeV), 0.00, 0.0 * dd4hep::GeV}, .tRes = 0.0 * dd4hep::ns, .threshold= 5.0 * dd4hep::MeV, .capADC = 16384, - .dyRangeADC = 20 * dd4hep::GeV, + .dyRangeADC = 170 * dd4hep::GeV, .pedMeanADC = 100, .pedSigmaADC = 1, .resolutionTDC = 1e-11, @@ -50,12 +51,12 @@ extern "C" { "B0ECalRecHits", {"B0ECalRawHits"}, {"B0ECalRecHits"}, { .capADC = 16384, - .dyRangeADC = 20. * dd4hep::GeV, + .dyRangeADC = 170. * dd4hep::GeV, .pedMeanADC = 100, .pedSigmaADC = 1, .resolutionTDC = 1e-11, .thresholdFactor = 0.0, - .thresholdValue = 0.0, + .thresholdValue = 1.0, // using threshold of 10 photons = 10 MeV = 1 ADC .sampFrac = "0.998", .readout = "B0ECalHits", .sectorField = "sector", From 7251aaf229ebc6dbed9fa2a350f765c26f68f2bb Mon Sep 17 00:00:00 2001 From: Barak Schmookler Date: Wed, 23 Oct 2024 07:00:49 -0500 Subject: [PATCH 5/8] Fix: Correctly fill track parameters covariance error matrix (#1639) ### Briefly, what does this PR introduce? In the loop that writes the covariance matrix for the track parameters to the edm4eic output, the indices were not being incremented. This lead to this output: ``` root [1] events->Scan("CentralCKFTrackParameters.covariance.covariance") *********************************** * Row * Instance * CentralCK * *********************************** * 0 * 0 * 0.0003341 * * 0 * 1 * 0 * * 0 * 2 * 0 * * 0 * 3 * 0 * * 0 * 4 * 0 * * 0 * 5 * 0 * * 0 * 6 * 0 * * 0 * 7 * 0 * * 0 * 8 * 0 * * 0 * 9 * 0 * * 0 * 10 * 0 * * 0 * 11 * 0 * * 0 * 12 * 0 * * 0 * 13 * 0 * * 0 * 14 * 0 * * 0 * 15 * 0 * * 0 * 16 * 0 * * 0 * 17 * 0 * * 0 * 18 * 0 * * 0 * 19 * 0 * * 0 * 20 * 0 * * 1 * 0 * 0.0005269 * * 1 * 1 * 0 * * 1 * 2 * 0 * * 1 * 3 * 0 * ``` With the fix applied here, the correct covariance matrix is written out: ``` root [1] events->Scan("CentralCKFTrackParameters.covariance.covariance") *********************************** * Row * Instance * CentralCK * *********************************** * 0 * 0 * 0.0013565 * * 0 * 1 * -1.27e-05 * * 0 * 2 * 0.0514564 * * 0 * 3 * -3.36e-05 * * 0 * 4 * -7.13e-07 * * 0 * 5 * 8.446e-07 * * 0 * 6 * 2.282e-08 * * 0 * 7 * 3.088e-05 * * 0 * 8 * -1.17e-09 * * 0 * 9 * 1.880e-08 * * 0 * 10 * 7.277e-06 * * 0 * 11 * -6.51e-07 * * 0 * 12 * -1.60e-07 * * 0 * 13 * -5.63e-11 * * 0 * 14 * 4.297e-07 * * 0 * 15 * -1.14e-07 * * 0 * 16 * 0.0001654 * * 0 * 17 * -9.91e-10 * * 0 * 18 * 1.011e-07 * * 0 * 19 * -4.97e-09 * * 0 * 20 * 0.0003341 * * 1 * 0 * 0.2452857 * * 1 * 1 * -0.024439 * * 1 * 2 * 16.420986 * * 1 * 3 * -0.004535 * Type to continue or q to quit ==> * 1 * 4 * 0.0026367 * * 1 * 5 * 8.454e-05 * * 1 * 6 * -3.22e-05 * * 1 * 7 * 0.0043999 * * 1 * 8 * 1.179e-06 * * 1 * 9 * 1.181e-06 * * 1 * 10 * 0.0003995 * * 1 * 11 * 0.0001339 * * 1 * 12 * -1.09e-05 * * 1 * 13 * 6.757e-09 * * 1 * 14 * 3.378e-05 * * 1 * 15 * 7.834e-05 * * 1 * 16 * -0.054508 * * 1 * 17 * -8.71e-06 * * 1 * 18 * -1.46e-05 * * 1 * 19 * -2.89e-07 * * 1 * 20 * 0.0005269 * ``` This can be needed when propagating the track in an analysis script (e.g. https://github.com/eic/snippets/tree/main/Tracking/ImpactPointEstimator). ### What kind of change does this PR introduce? - [x ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. A user will only see a change if they use covariance matrix written out to the ROOT file in their analysis. ### Does this PR change default behavior? No --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- src/algorithms/tracking/ActsToTracks.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/algorithms/tracking/ActsToTracks.cc b/src/algorithms/tracking/ActsToTracks.cc index 79480d1884..aa8c069449 100644 --- a/src/algorithms/tracking/ActsToTracks.cc +++ b/src/algorithms/tracking/ActsToTracks.cc @@ -120,7 +120,9 @@ void ActsToTracks::process(const Input& input, const Output& output) const { for (size_t j = 0; const auto& [b, y] : edm4eic_indexed_units) { // FIXME why not pars.getCovariance()(i,j) = covariance(a,b) / x / y; cov(i,j) = covariance(a,b) / x / y; + ++j; } + ++i; } pars.setCovariance(cov); From 5e39885644bdf867aa2060ccc46da2500678c1da Mon Sep 17 00:00:00 2001 From: Simon Gardner Date: Wed, 23 Oct 2024 20:54:42 +0100 Subject: [PATCH 6/8] Swap the order of the CKF algorithm arguments (#1640) Minor code clean up. The order of the CKF algorithm arguments has been swapped around to match the order provided to the factory generator making it slightly easier to follow the code. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [x] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No ### Does this PR change default behavior? No --- src/algorithms/tracking/CKFTracking.cc | 4 ++-- src/algorithms/tracking/CKFTracking.h | 4 ++-- src/global/tracking/CKFTracking_factory.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/algorithms/tracking/CKFTracking.cc b/src/algorithms/tracking/CKFTracking.cc index 93d33feff7..371574de5a 100644 --- a/src/algorithms/tracking/CKFTracking.cc +++ b/src/algorithms/tracking/CKFTracking.cc @@ -109,8 +109,8 @@ namespace eicrecon { std::vector, std::vector > - CKFTracking::process(const edm4eic::Measurement2DCollection& meas2Ds, - const edm4eic::TrackParametersCollection &init_trk_params) { + CKFTracking::process(const edm4eic::TrackParametersCollection& init_trk_params, + const edm4eic::Measurement2DCollection& meas2Ds) { // create sourcelink and measurement containers diff --git a/src/algorithms/tracking/CKFTracking.h b/src/algorithms/tracking/CKFTracking.h index 6373488b79..896944c542 100644 --- a/src/algorithms/tracking/CKFTracking.h +++ b/src/algorithms/tracking/CKFTracking.h @@ -74,8 +74,8 @@ namespace eicrecon { std::vector, std::vector > - process(const edm4eic::Measurement2DCollection& meas2Ds, - const edm4eic::TrackParametersCollection &init_trk_params); + process(const edm4eic::TrackParametersCollection& init_trk_params, + const edm4eic::Measurement2DCollection& meas2Ds); private: std::shared_ptr m_log; diff --git a/src/global/tracking/CKFTracking_factory.h b/src/global/tracking/CKFTracking_factory.h index 91f2e7c2fd..ba17ca3ca6 100644 --- a/src/global/tracking/CKFTracking_factory.h +++ b/src/global/tracking/CKFTracking_factory.h @@ -54,8 +54,8 @@ class CKFTracking_factory : m_acts_trajectories_output(), m_acts_tracks_output() ) = m_algo->process( - *m_measurements_input(), - *m_parameters_input() + *m_parameters_input(), + *m_measurements_input() ); } }; From 593f51bfa296511a690c9520fd3b17998f05bc18 Mon Sep 17 00:00:00 2001 From: "minho.kim@anl.gov" Date: Fri, 25 Oct 2024 20:01:16 -0500 Subject: [PATCH 7/8] BIC sampling fraction update (#1642) ### Briefly, what does this PR introduce? Sampling fraction of the BIC ScFi layer has been updated using 5 GeV photons generated at eta = 0. ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [x] New feature (issue EICrecon #1638) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [x] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? ### Does this PR change default behavior? --- src/detectors/BEMC/BEMC.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/detectors/BEMC/BEMC.cc b/src/detectors/BEMC/BEMC.cc index 36b2d9953f..0dcbd9a513 100644 --- a/src/detectors/BEMC/BEMC.cc +++ b/src/detectors/BEMC/BEMC.cc @@ -68,7 +68,7 @@ extern "C" { .resolutionTDC = EcalBarrelScFi_resolutionTDC, .thresholdFactor = 0.0, // use only thresholdValue .thresholdValue = 5.0, // 16384 ADC counts/1500 MeV * 0.5 MeV (desired threshold) = 5.46 - .sampFrac = "0.10200085", + .sampFrac = "0.09320426", .readout = "EcalBarrelScFiHits", .layerField = "layer", .sectorField = "sector", From 32d380003f5e7c658883daafda260222c14020af Mon Sep 17 00:00:00 2001 From: Dmitry Kalinkin Date: Sat, 26 Oct 2024 19:56:33 -0400 Subject: [PATCH 8/8] B0ECAL.cc: fix spelling (#1641) --- src/detectors/B0ECAL/B0ECAL.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/detectors/B0ECAL/B0ECAL.cc b/src/detectors/B0ECAL/B0ECAL.cc index 10e8be761c..2b653f841f 100644 --- a/src/detectors/B0ECAL/B0ECAL.cc +++ b/src/detectors/B0ECAL/B0ECAL.cc @@ -33,7 +33,7 @@ extern "C" { {"B0ECalRawHits"}, #endif { - // The stocastic term is set using light yield in PbOW4 of N_photons = 145.75 / GeV / mm, for 6x6 mm2 sensors with PDE=0.18 (a=1/sqrt(145.75*36*0.18)) + // The stochastic term is set using light yield in PbOW4 of N_photons = 145.75 / GeV / mm, for 6x6 mm2 sensors with PDE=0.18 (a=1/sqrt(145.75*36*0.18)) .eRes = {0.0326 * sqrt(dd4hep::GeV), 0.00, 0.0 * dd4hep::GeV}, .tRes = 0.0 * dd4hep::ns, .threshold= 5.0 * dd4hep::MeV,