From f5c79fe967a57724ba1d39790d258524af61f7dc Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Tue, 10 Sep 2024 13:10:09 -0700 Subject: [PATCH 01/14] Exclude 3rd party from static analysis checks --- CMakeLists.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 93e2d9936..83d02f9be 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,6 +30,12 @@ list(APPEND CMAKE_MODULE_PATH ${LDMX_SW_SOURCE_DIR}/cmake/) # is set to NOTFOUND. include(BuildMacros RESULT_VARIABLE build_macros_found) +# Adding ROOT, ACTS, HLT arb prec as "SYSTEM" +# which will silence compiler warnings from these 3rd party softwares +include_directories(SYSTEM /usr/local/include/root) +include_directories(SYSTEM Tracking/acts/Core/include) +include_directories(SYSTEM Trigger/HLS_arbitrary_Precision_Types/include) + # If an install location hasn't been set via CMAKE_INSTALL_PREFIX, set it to # a reasonable default ($PWD/install). if (CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT) From 4f5780481f6dd1424f92d42379dd6b575c2ce454 Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Tue, 10 Sep 2024 13:10:10 -0700 Subject: [PATCH 02/14] Static analysis improvement for Biasing and Conditions --- .../include/Biasing/PhotoNuclearTopologyFilters.h | 2 +- Biasing/include/Biasing/TargetProcessFilter.h | 3 ++- .../src/Biasing/PhotoNuclearTopologyFilters.cxx | 4 ++-- Conditions/src/Conditions/GeneralCSVLoader.cxx | 7 ++++--- Conditions/src/Conditions/SimpleTableStreamers.cxx | 13 +++++++------ Conditions/src/Conditions/URLStreamer.cxx | 7 ++++--- Conditions/test/TestConditions.cxx | 14 ++++++++------ 7 files changed, 28 insertions(+), 22 deletions(-) diff --git a/Biasing/include/Biasing/PhotoNuclearTopologyFilters.h b/Biasing/include/Biasing/PhotoNuclearTopologyFilters.h index dccd69b3d..ba2f4670c 100644 --- a/Biasing/include/Biasing/PhotoNuclearTopologyFilters.h +++ b/Biasing/include/Biasing/PhotoNuclearTopologyFilters.h @@ -91,7 +91,7 @@ class PhotoNuclearTopologyFilter : public simcore::UserAction { * */ constexpr bool skipCountingParticle(const int pdgcode) const { - return !(pdgcode < 10000 || count_light_ions_ && isLightIon(pdgcode)); + return !(pdgcode < 10000 || (count_light_ions_ && isLightIon(pdgcode))); } constexpr bool isNeutron(const int pdgID) const { return pdgID == 2112; } diff --git a/Biasing/include/Biasing/TargetProcessFilter.h b/Biasing/include/Biasing/TargetProcessFilter.h index 77f89b393..1fca4f56d 100644 --- a/Biasing/include/Biasing/TargetProcessFilter.h +++ b/Biasing/include/Biasing/TargetProcessFilter.h @@ -66,7 +66,8 @@ class TargetProcessFilter : public simcore::UserAction { G4Track *currentTrack_{nullptr}; /** Flag indicating if the reaction of intereset occurred. */ - bool reactionOccurred_{false}; + // It is usused, should it be? FIXME + // bool reactionOccurred_{false}; /// The process to bias std::string process_{""}; diff --git a/Biasing/src/Biasing/PhotoNuclearTopologyFilters.cxx b/Biasing/src/Biasing/PhotoNuclearTopologyFilters.cxx index 22ef7c4d5..f5c26b545 100644 --- a/Biasing/src/Biasing/PhotoNuclearTopologyFilters.cxx +++ b/Biasing/src/Biasing/PhotoNuclearTopologyFilters.cxx @@ -44,9 +44,9 @@ bool SingleNeutronFilter::rejectEvent( PhotoNuclearTopologyFilter::PhotoNuclearTopologyFilter( const std::string& name, framework::config::Parameters& parameters) : UserAction{name, parameters}, + count_light_ions_{parameters.getParameter("count_light_ions")}, hard_particle_threshold_{ - parameters.getParameter("hard_particle_threshold")}, - count_light_ions_{parameters.getParameter("count_light_ions")} {} + parameters.getParameter("hard_particle_threshold")} {} void PhotoNuclearTopologyFilter::stepping(const G4Step* step) { // Get the track associated with this step. diff --git a/Conditions/src/Conditions/GeneralCSVLoader.cxx b/Conditions/src/Conditions/GeneralCSVLoader.cxx index c26d4511c..62c79fb41 100644 --- a/Conditions/src/Conditions/GeneralCSVLoader.cxx +++ b/Conditions/src/Conditions/GeneralCSVLoader.cxx @@ -52,9 +52,9 @@ bool GeneralCSVLoader::nextRow() { colNames_.swap(line_split); continue; } - if (line_split.size() == colNames_.size()) + if (line_split.size() == colNames_.size()) { rowData_.swap(line_split); - else { + } else { EXCEPTION_RAISE("CSVLineMismatch", "Reading CSV found line with " + std::to_string(line_split.size()) + " in CSV with " + @@ -73,12 +73,13 @@ StringCSVLoader::StringCSVLoader(const std::string& source, std::string StringCSVLoader::getNextLine() { std::string retval; - if (rowBegin_ != rowEnd_) + if (rowBegin_ != rowEnd_) { if (rowEnd_ == std::string::npos) { retval = source_.substr(rowBegin_, rowEnd_); } else { retval = source_.substr(rowBegin_, rowEnd_ - rowBegin_); } + } // now we look for the follow on. // find the first non-end-of-line character rowBegin_ = source_.find_first_not_of(linesep_, rowEnd_); diff --git a/Conditions/src/Conditions/SimpleTableStreamers.cxx b/Conditions/src/Conditions/SimpleTableStreamers.cxx index 083b1c74d..37d875548 100644 --- a/Conditions/src/Conditions/SimpleTableStreamers.cxx +++ b/Conditions/src/Conditions/SimpleTableStreamers.cxx @@ -19,8 +19,9 @@ static void storeIdFields(unsigned int id, std::ostream& s) { template void storeT(const T& t, std::ostream& s, bool expandIds) { - char buffer[100]; - // write the header line + // buffer is unused, was it meant to be? FIXME + // char buffer[100]; + // write the header line s << "\"DetID\""; if (expandIds && t.getRowCount() > 0) { storeIdFields(t.getRowId(0), s); @@ -160,15 +161,15 @@ void loadT(T& table, std::istream& is) { ") on line " + std::to_string(iline)); } - unsigned int id(0); + unsigned int tempId(0); std::vector values(table_to_csv.size(), 0); - if (iDetID >= 0) id = strtoul(split[iDetID].c_str(), 0, 0); + if (iDetID >= 0) tempId = strtoul(split[iDetID].c_str(), 0, 0); V dummy(0); for (auto icopy : table_to_csv) { values[icopy.first] = convert(split[icopy.second], dummy); } - if (id != 0) { - table.add(id, values); + if (tempId != 0) { + table.add(tempId, values); } } } diff --git a/Conditions/src/Conditions/URLStreamer.cxx b/Conditions/src/Conditions/URLStreamer.cxx index 78a6f5d50..8b8fb7a30 100644 --- a/Conditions/src/Conditions/URLStreamer.cxx +++ b/Conditions/src/Conditions/URLStreamer.cxx @@ -22,7 +22,7 @@ void urlstatistics(unsigned int& http_requests, unsigned int& http_failures) { } std::unique_ptr urlstream(const std::string& url) { - if (url.find("file://") == 0 || url.length() > 0 && url[0] == '/') { + if ((url.find("file://") == 0 || url.length() > 0) && (url[0] == '/')) { std::string fname = url; if (fname.find("file://") == 0) fname = url.substr(url.find("file://") + strlen("file://")); @@ -46,8 +46,9 @@ std::unique_ptr urlstream(const std::string& url) { execl("/usr/bin/wget", "wget", "-q", "--no-check-certificate", "-O", fname, "-o", "/tmp/wget.log", url.c_str(), (char*)0); } else { - int wstatus; - int wrv = waitpid(apid, &wstatus, 0); + int wstatus{9999}; + // wrv is not used, was it meant to be? FIXME + // int wrv = waitpid(apid, &wstatus, 0); // std::cout << "EXITED: " << WIFEXITED(wstatus) << " STATUS: " << // WEXITSTATUS(wstatus) << std::endl; if (WIFEXITED(wstatus) != 1 || WEXITSTATUS(wstatus) != 0) { diff --git a/Conditions/test/TestConditions.cxx b/Conditions/test/TestConditions.cxx index 87820b33b..5ecb8b4b4 100644 --- a/Conditions/test/TestConditions.cxx +++ b/Conditions/test/TestConditions.cxx @@ -309,9 +309,10 @@ TEST_CASE("Conditions", "[Conditions]") { cxt.setRun(128); - const DoubleTableCondition& httpTable128 = - hp->getConditions().getCondition( - "testbeam22_pedestals"); + // This is unused, should it be? FIXME + // const DoubleTableCondition& httpTable128 = + // hp->getConditions().getCondition( + // "testbeam22_pedestals"); hp->getConditions().getCondition( "testbeam22_pedestals"); @@ -331,9 +332,10 @@ TEST_CASE("Conditions", "[Conditions]") { cxt.setRun(140); - const DoubleTableCondition& httpTable140 = - hp->getConditions().getCondition( - "testbeam22_pedestals"); + // this is unused, should it be? FIXME + // const DoubleTableCondition& httpTable140 = + // hp->getConditions().getCondition( + // "testbeam22_pedestals"); conditions::urlstatistics(http_requests[1], http_failures[1]); REQUIRE(((http_requests[1] - http_requests[0]) == 2 && From 421ab9316fc4c95dd14367e6765d783ce708e3a5 Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Tue, 10 Sep 2024 13:10:10 -0700 Subject: [PATCH 03/14] Static analysis improvement for DQM and DetDescr --- DQM/src/DQM/DarkBremInteraction.cxx | 1 - DQM/src/DQM/HcalInefficiencyDQM.cxx | 4 +-- DQM/src/DQM/NtuplizeHgcrocDigiCollection.cxx | 1 - DetDescr/src/DetDescr/EcalGeometry.cxx | 5 ++-- DetDescr/src/DetDescr/HcalGeometry.cxx | 27 +++++++++++++------- 5 files changed, 23 insertions(+), 15 deletions(-) diff --git a/DQM/src/DQM/DarkBremInteraction.cxx b/DQM/src/DQM/DarkBremInteraction.cxx index 5951aeba3..1668156c1 100644 --- a/DQM/src/DQM/DarkBremInteraction.cxx +++ b/DQM/src/DQM/DarkBremInteraction.cxx @@ -111,7 +111,6 @@ void DarkBremInteraction::produce(framework::Event& event) { double incident_energy = energy(incident_p, recoil->getMass()); double recoil_energy = energy(recoil_p, recoil->getMass()); - double visible_energy = (beam->getEnergy() - incident_energy) + recoil_energy; std::vector ap_vertex{aprime->getVertex()}; std::string ap_vertex_volume{aprime->getVertexVolume()}; diff --git a/DQM/src/DQM/HcalInefficiencyDQM.cxx b/DQM/src/DQM/HcalInefficiencyDQM.cxx index 97923a3e0..61af8e20f 100644 --- a/DQM/src/DQM/HcalInefficiencyDQM.cxx +++ b/DQM/src/DQM/HcalInefficiencyDQM.cxx @@ -15,10 +15,10 @@ void HcalInefficiencyAnalyzer::analyze(const framework::Event &event) { const std::vector sectionNames{"back", "top", "bottom", "right", "left"}; - for (const auto hit : hcalRecHits) { + for (const auto &hit : hcalRecHits) { const ldmx::HcalID id{static_cast(hit.getID())}; const auto section{id.section()}; - const auto z{hit.getZPos()}; + // const auto z{hit.getZPos()}; const auto layer{id.layer()}; if (hitPassesVeto(hit, section)) { if (layer < firstLayersHit[section]) { diff --git a/DQM/src/DQM/NtuplizeHgcrocDigiCollection.cxx b/DQM/src/DQM/NtuplizeHgcrocDigiCollection.cxx index ec7d8f5ca..933860b4e 100644 --- a/DQM/src/DQM/NtuplizeHgcrocDigiCollection.cxx +++ b/DQM/src/DQM/NtuplizeHgcrocDigiCollection.cxx @@ -99,7 +99,6 @@ void NtuplizeHgcrocDigiCollection::analyze(const framework::Event& event) { raw_id_ = static_cast(d.id()); if (using_eid_) { ldmx::HcalElectronicsID eid(d.id()); - ldmx::HcalDigiID detid = detmap.get(eid); fpga_ = eid.fiber(); link_ = eid.elink(); good_link_ = (good_bxheader.at(link_) and good_trailer.at(link_)); diff --git a/DetDescr/src/DetDescr/EcalGeometry.cxx b/DetDescr/src/DetDescr/EcalGeometry.cxx index 848e093c2..d8cc71963 100644 --- a/DetDescr/src/DetDescr/EcalGeometry.cxx +++ b/DetDescr/src/DetDescr/EcalGeometry.cxx @@ -19,8 +19,9 @@ static double distance(const std::pair& p1, (p1.second - p2.second) * (p1.second - p2.second)); } -static double distance(const std::tuple& p1, - const std::tuple& p2) { +[[maybe_unused]] static double distance( + const std::tuple& p1, + const std::tuple& p2) { return sqrt((std::get<0>(p1) - std::get<0>(p2)) * (std::get<0>(p1) - std::get<0>(p2)) + (std::get<1>(p1) - std::get<1>(p2)) * diff --git a/DetDescr/src/DetDescr/HcalGeometry.cxx b/DetDescr/src/DetDescr/HcalGeometry.cxx index ab4e6615a..88e9d8b92 100644 --- a/DetDescr/src/DetDescr/HcalGeometry.cxx +++ b/DetDescr/src/DetDescr/HcalGeometry.cxx @@ -57,9 +57,12 @@ std::vector HcalGeometry::rotateGlobalToLocalBarPosition( case ScintillatorOrientation::vertical: return {globalPosition[2], globalPosition[0], globalPosition[1]}; default: // Should not be possible with current geometries - break; + EXCEPTION_RAISE("InvalidRotation", + "Attempted to rotate into an invalid " + "orientation for a scintillator bar!"); } case ldmx::HcalID::HcalSection::TOP: + [[fallthrough]]; case ldmx::HcalID::HcalSection::BOTTOM: switch (orientation) { case ScintillatorOrientation::horizontal: @@ -67,9 +70,12 @@ std::vector HcalGeometry::rotateGlobalToLocalBarPosition( case ScintillatorOrientation::depth: return {globalPosition[1], globalPosition[0], globalPosition[2]}; default: // Should not be possible with current geometries - break; + EXCEPTION_RAISE("InvalidRotation", + "Attempted to rotate into an invalid " + "orientation for a scintillator bar!"); } case ldmx::HcalID::HcalSection::LEFT: + [[fallthrough]]; case ldmx::HcalID::HcalSection::RIGHT: switch (orientation) { case ScintillatorOrientation::vertical: @@ -77,15 +83,18 @@ std::vector HcalGeometry::rotateGlobalToLocalBarPosition( case ScintillatorOrientation::depth: return globalPosition; default: // Should not be possible with current geometries - break; + EXCEPTION_RAISE("InvalidRotation", + "Attempted to rotate into an invalid " + "orientation for a scintillator bar!"); } + default: + // Can only reach this part if we somehow didn't match any of the options + // above. This could happen if someone introduces a new geometry but + // doesn't patch this part. + EXCEPTION_RAISE("InvalidRotation", + "Attempted to rotate into an invalid " + "orientation for a scintillator bar!"); } - // Can only reach this part if we somehow didn't match any of the options - // above. This could happen if someone introduces a new geometry but doesn't - // patch this part. - EXCEPTION_RAISE("InvalidRotation", - "Attempted to rotate into an invalid " - "orientation for a scintillator bar!"); } HcalGeometry::ScintillatorOrientation HcalGeometry::getScintillatorOrientation( From 55cb97508ef6fe367747bb6364654ed972448e38 Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Tue, 10 Sep 2024 13:10:10 -0700 Subject: [PATCH 04/14] Static analysis improvement for Ecal --- Ecal/include/Ecal/EcalDigiProducer.h | 2 +- Ecal/include/Ecal/EcalVetoProcessor.h | 7 +- Ecal/include/Ecal/Event/EcalVetoResult.h | 25 +- Ecal/include/Ecal/MyClusterWeight.h | 7 +- Ecal/src/Ecal/EcalRawDecoder.cxx | 48 +-- Ecal/src/Ecal/EcalRawEncoder.cxx | 14 +- Ecal/src/Ecal/EcalTriggerGeometry.cxx | 9 +- Ecal/src/Ecal/EcalVetoProcessor.cxx | 12 +- Ecal/src/Ecal/Event/EcalDigiCollection.cxx | 6 +- Ecal/src/Ecal/Event/EcalVetoResult.cxx | 356 ++++++++++----------- Ecal/test/EcalDigiPipelineTest.cxx | 8 +- 11 files changed, 249 insertions(+), 245 deletions(-) diff --git a/Ecal/include/Ecal/EcalDigiProducer.h b/Ecal/include/Ecal/EcalDigiProducer.h index 6bf5ef542..5fc07421f 100644 --- a/Ecal/include/Ecal/EcalDigiProducer.h +++ b/Ecal/include/Ecal/EcalDigiProducer.h @@ -109,7 +109,7 @@ class EcalDigiProducer : public framework::Producer { std::unique_ptr hgcroc_; /// Total number of channels in the ECal - int nTotalChannels_; + [[maybe_unused]] int nTotalChannels_; /// Conversion from time in ns to ticks of the internal clock double ns_; diff --git a/Ecal/include/Ecal/EcalVetoProcessor.h b/Ecal/include/Ecal/EcalVetoProcessor.h index 53b068828..07aae67ad 100644 --- a/Ecal/include/Ecal/EcalVetoProcessor.h +++ b/Ecal/include/Ecal/EcalVetoProcessor.h @@ -70,8 +70,8 @@ class EcalVetoProcessor : public framework::Producer { /* Function to take loaded hit maps and find isolated hits in them */ void fillIsolatedHitMap(const std::vector& ecalRecHits, ldmx::EcalID globalCentroid, - std::map& cellMap_, - std::map& cellMapIso_, + std::map& cellMap, + std::map& cellMapIso, bool doTight = false); std::vector getTrajectory(std::vector momentum, @@ -113,10 +113,8 @@ class EcalVetoProcessor : public framework::Producer { std::vector> roc_range_values_; int nEcalLayers_{0}; - int backEcalStartingLayer_{0}; int nReadoutHits_{0}; int deepestLayerHit_{0}; - int doBdt_{0}; double summedDet_{0}; double summedTightIso_{0}; @@ -152,7 +150,6 @@ class EcalVetoProcessor : public framework::Producer { double beamEnergyMeV_{0}; bool verbose_{false}; - bool doesPassVeto_{false}; std::string bdtFileName_; std::string rocFileName_; diff --git a/Ecal/include/Ecal/Event/EcalVetoResult.h b/Ecal/include/Ecal/Event/EcalVetoResult.h index 985157920..301abdb92 100644 --- a/Ecal/include/Ecal/Event/EcalVetoResult.h +++ b/Ecal/include/Ecal/Event/EcalVetoResult.h @@ -3,6 +3,7 @@ * @brief Class used to encapsulate the results obtained from * EcalVetoProcessor. * @author Omar Moreno, SLAC National Accelerator Laboratory + * @author Danyi Zhang, Tamas Almos Vami (UCSB) */ #ifndef EVENT_ECALVETORESULT_H_ @@ -27,7 +28,7 @@ class EcalVetoResult { EcalVetoResult(); /** Destructor */ - ~EcalVetoResult(); + virtual ~EcalVetoResult(); /** * Set the sim particle and 'is findable' flag. @@ -219,23 +220,23 @@ class EcalVetoResult { }; /** Return the x position of the recoil at the Ecal face. */ - const double getRecoilX() const { return recoilX_; }; + double getRecoilX() const { return recoilX_; }; /** Return the y position of the recoil at the Ecal face. */ - const double getRecoilY() const { return recoilY_; }; + double getRecoilY() const { return recoilY_; }; /// Number of straight tracks found - const int getNStraightTracks() const { return nStraightTracks_; } + int getNStraightTracks() const { return nStraightTracks_; } /// Number of linear-regression tracks found - const int getNLinRegTracks() const { return nLinregTracks_; } - - const int getFirstNearPhLayer() const { return firstNearPhLayer_; } - const int getNNearPhHits() const { return nNearPhHits_; } - const int getPhotonTerritoryHits() const { return photonTerritoryHits_; } - const float getEPAng() const { return epAng_; } - const float getEPSep() const { return epSep_; } - const float getEPDot() const { return epDot_; } + int getNLinRegTracks() const { return nLinregTracks_; } + + int getFirstNearPhLayer() const { return firstNearPhLayer_; } + int getNNearPhHits() const { return nNearPhHits_; } + int getPhotonTerritoryHits() const { return photonTerritoryHits_; } + float getEPAng() const { return epAng_; } + float getEPSep() const { return epSep_; } + float getEPDot() const { return epDot_; } private: /** Flag indicating whether the event is vetoed by the Ecal. */ diff --git a/Ecal/include/Ecal/MyClusterWeight.h b/Ecal/include/Ecal/MyClusterWeight.h index 90f9a55b3..7680b25b5 100644 --- a/Ecal/include/Ecal/MyClusterWeight.h +++ b/Ecal/include/Ecal/MyClusterWeight.h @@ -30,12 +30,13 @@ class MyClusterWeight { double bZ = b.centroid().Pz(); double dijz; - double eFrac; + // eFrac is set but not used, should it be? FIXME + // double eFrac; if (aE >= bE) { - eFrac = bE / aE; + // eFrac = bE / aE; dijz = bZ - aZ; } else { - eFrac = aE / bE; + // eFrac = aE / bE; dijz = aZ - bZ; } diff --git a/Ecal/src/Ecal/EcalRawDecoder.cxx b/Ecal/src/Ecal/EcalRawDecoder.cxx index 5459d89bf..14f50b9ef 100644 --- a/Ecal/src/Ecal/EcalRawDecoder.cxx +++ b/Ecal/src/Ecal/EcalRawDecoder.cxx @@ -56,12 +56,13 @@ void EcalRawDecoder::produce(framework::Event& event) { */ uint32_t whole_event_header; reader >> whole_event_header; + /* these are unused, should they have been? FIXME uint32_t version = (whole_event_header >> 28) & packing::utility::mask<4>; uint32_t fpga = (whole_event_header >> 20) & packing::utility::mask<8>; + uint32_t eventlen = whole_event_header & packing::utility::mask<16>; +*/ uint32_t nsamples = (whole_event_header >> 16) & packing::utility::mask<4>; - uint32_t eventlen = whole_event_header & packing::utility::mask<16>; - // sample counters std::vector length_per_sample(nsamples, 0); for (uint32_t i_sample{0}; i_sample < nsamples; i_sample++) { @@ -108,16 +109,16 @@ void EcalRawDecoder::produce(framework::Event& event) { uint32_t fpga = (head1 >> 20) & packing::utility::mask<8>; uint32_t nlinks = (head1 >> 14) & packing::utility::mask<6>; - uint32_t len = head1 & packing::utility::mask<12>; + // uint32_t len = head1 & packing::utility::mask<12>; // std::cout << ", fpga: " << fpga << ", nlinks: " << nlinks << ", len: " << // len << std::endl; fpga_crc << head2; // std::cout << hex(head2) << " : "; - uint32_t bx_id = (head2 >> 20) & packing::utility::mask<12>; - uint32_t rreq = (head2 >> 10) & packing::utility::mask<10>; - uint32_t orbit = head2 & packing::utility::mask<10>; + // uint32_t bx_id = (head2 >> 20) & packing::utility::mask<12>; + // uint32_t rreq = (head2 >> 10) & packing::utility::mask<10>; + // uint32_t orbit = head2 & packing::utility::mask<10>; // std::cout << "bx_id: " << bx_id << ", rreq: " << rreq << ", orbit: " << // orbit << std::endl; @@ -130,8 +131,10 @@ void EcalRawDecoder::produce(framework::Event& event) { // std::cout << hex(w) << " : Four Link Pack " << std::endl; } uint32_t shift_in_word = 8 * (i_link % 4); - bool rid_ok = (w >> (shift_in_word + 7)) & packing::utility::mask<1> == 1; - bool cdc_ok = (w >> (shift_in_word + 6)) & packing::utility::mask<1> == 1; + // These are unused, should they be? FIXME + // bool rid_ok = ((w >> (shift_in_word + 7)) & packing::utility::mask<1>) + // == 1; bool cdc_ok = ((w >> (shift_in_word + 6)) & + // packing::utility::mask<1>) == 1; length_per_link[i_link] = (w >> shift_in_word) & packing::utility::mask<6>; // std::cout << " Link " << i_link << " readout " << @@ -155,7 +158,7 @@ void EcalRawDecoder::produce(framework::Event& event) { fpga_crc << w; link_crc << w; uint32_t roc_id = (w >> 16) & packing::utility::mask<16>; - bool crc_ok = (w >> 15) & packing::utility::mask<1> == 1; + // bool crc_ok = (w >> 15) & packing::utility::mask<1> == 1; // std::cout << hex(w) << " : roc_id " << roc_id << ", crc_ok (v2 // always false) " << std::boolalpha << crc_ok << std::endl; @@ -195,11 +198,14 @@ void EcalRawDecoder::produce(framework::Event& event) { * 10101010 | BXID (12) | WADD (9) | 1010 */ // std::cout << " : ROC Header"; + /* + * These are unused, should they be? FIXME link_crc << w; uint32_t bx_id = (w >> 16) & packing::utility::mask<12>; uint32_t short_event = (w >> 10) & packing::utility::mask<6>; uint32_t short_orbit = (w >> 7) & packing::utility::mask<3>; uint32_t hamming_errs = (w >> 4) & packing::utility::mask<3>; + */ } else if (channel_id == common_mode_channel) { /** Common Mode Channels * 10 | 0000000000 | Common Mode ADC 0 (10) | Common Mode ADC 1 (10) @@ -207,17 +213,17 @@ void EcalRawDecoder::produce(framework::Event& event) { link_crc << w; // std::cout << " : Common Mode"; } else if (channel_id == 39) { - // CRC checksum from ROC - uint32_t crc = w; - // std::cout << " : CRC checksum : " << hex(link_crc.get()) << - // " =? " << hex(crc); /* - if (link_crc.get() != crc) { - EXCEPTION_RAISE("BadCRC", - "Our calculated link checksum doesn't match the " - "one from raw data."); - } - */ + // CRC checksum from ROC + uint32_t crc = w; + // std::cout << " : CRC checksum : " << hex(link_crc.get()) << + // " =? " << hex(crc); + if (link_crc.get() != crc) { + EXCEPTION_RAISE("BadCRC", + "Our calculated link checksum doesn't match the " + "one from raw data."); + } + */ } else { /// DAQ Channels @@ -257,8 +263,8 @@ void EcalRawDecoder::produce(framework::Event& event) { } // loop over links // another CRC checksum from FPGA - reader >> w; - uint32_t crc = w; + // reader >> w; + // uint32_t crc = w; // std::cout << "FPGA Checksum : " << hex(fpga_crc.get()) << " =? " // << hex(crc) << std::endl; /* TODO diff --git a/Ecal/src/Ecal/EcalRawEncoder.cxx b/Ecal/src/Ecal/EcalRawEncoder.cxx index 0e4f6c8d2..e280aa0a0 100644 --- a/Ecal/src/Ecal/EcalRawEncoder.cxx +++ b/Ecal/src/Ecal/EcalRawEncoder.cxx @@ -29,7 +29,7 @@ void EcalRawEncoder::produce(framework::Event& event) { /** * Static parameters depending on ROC version */ - static const unsigned int common_mode_channel = roc_version_ == 2 ? 19 : 1; + // static const unsigned int common_mode_channel = roc_version_ == 2 ? 19 : 1; auto digis{ event.getObject(input_name_, input_pass_)}; @@ -72,7 +72,8 @@ void EcalRawEncoder::produce(framework::Event& event) { * the header information the calculate the CRC checksums. */ std::vector buffer; - static uint32_t word; // word to use for constructing buffer + // word to use for constructing buffer + static uint32_t word; uint32_t i_bx{0}; for (auto const& bunch : sorted_samples) { /**TODO calculate bunch ID, read request, and orbit from sample ID, event @@ -135,10 +136,11 @@ void EcalRawEncoder::produce(framework::Event& event) { * ... other listing of links ... */ word = 0; - word |= (1 << 12 + 1 + 6 + 8); // version - word |= (fpga_id & packing::utility::mask<8>) << 12 + 1 + 6; // FPGA - word |= (links.size() & packing::utility::mask<6>) << 12 + 1; // NLINKS - word |= (total_length & packing::utility::mask<12>); // LEN TODO + + word |= (1 << (12 + 1 + 6 + 8)); // version + word |= (fpga_id & packing::utility::mask<8>) << (12 + 1 + 6); // FPGA + word |= (links.size() & packing::utility::mask<6>) << (12 + 1); // NLINKS + word |= (total_length & packing::utility::mask<12>); // LEN TODO buffer.push_back(word); fpga_crc << word; diff --git a/Ecal/src/Ecal/EcalTriggerGeometry.cxx b/Ecal/src/Ecal/EcalTriggerGeometry.cxx index 396a6dbb8..675d0e12f 100644 --- a/Ecal/src/Ecal/EcalTriggerGeometry.cxx +++ b/Ecal/src/Ecal/EcalTriggerGeometry.cxx @@ -9,9 +9,10 @@ namespace ecal { -static const int LAYERS_MASK = 0xFF; +// These are unused, should they be? FIXME +// static const int LAYERS_MASK = 0xFF; +// static const int LAYERS_ODDEVEN = 0x02; static const int LAYERS_IDENTICAL = 0x01; -static const int LAYERS_ODDEVEN = 0x02; static const int MODULES_MASK = 0xFF00; static const int INPLANE_IDENTICAL = 0x0100; @@ -47,8 +48,8 @@ EcalTriggerGeometry::EcalTriggerGeometry(int symmetry, std::vector pids; for (int dv = -1; dv <= 1; dv++) { for (int du = -1; du <= 1; du++) { - ldmx::EcalID pid(0, 0, u + du + dv, - v + dv); // changes directions here + // changes directions here + ldmx::EcalID pid(0, 0, u + du + dv, v + dv); precision2trigger_[pid] = tid; pids.push_back(pid); } diff --git a/Ecal/src/Ecal/EcalVetoProcessor.cxx b/Ecal/src/Ecal/EcalVetoProcessor.cxx index b0239bebf..061a26160 100644 --- a/Ecal/src/Ecal/EcalVetoProcessor.cxx +++ b/Ecal/src/Ecal/EcalVetoProcessor.cxx @@ -1177,17 +1177,17 @@ ldmx::EcalID EcalVetoProcessor::GetShowerCentroidIDAndRMS( */ void EcalVetoProcessor::fillHitMap( const std::vector &ecalRecHits, - std::map &cellMap_) { + std::map &cellMap) { for (const ldmx::EcalHit &hit : ecalRecHits) { ldmx::EcalID id(hit.getID()); - cellMap_.emplace(id, hit.getEnergy()); + cellMap.emplace(id, hit.getEnergy()); } } void EcalVetoProcessor::fillIsolatedHitMap( const std::vector &ecalRecHits, ldmx::EcalID globalCentroid, - std::map &cellMap_, - std::map &cellMapIso_, bool doTight) { + std::map &cellMap, + std::map &cellMapIso, bool doTight) { for (const ldmx::EcalHit &hit : ecalRecHits) { auto isolatedHit = std::make_pair(true, ldmx::EcalID()); ldmx::EcalID id(hit.getID()); @@ -1212,7 +1212,7 @@ void EcalVetoProcessor::fillIsolatedHitMap( cellNbrIds[k] = ldmx::EcalID(id.layer(), cellNbrIds[k].module(), cellNbrIds[k].cell()); // look in cell hit map to see if it is there - if (cellMap_.find(cellNbrIds[k]) != cellMap_.end()) { + if (cellMap.find(cellNbrIds[k]) != cellMap.end()) { isolatedHit = std::make_pair(false, cellNbrIds[k]); break; } @@ -1221,7 +1221,7 @@ void EcalVetoProcessor::fillIsolatedHitMap( continue; } // Insert isolated hit - cellMapIso_.emplace(id, hit.getEnergy()); + cellMapIso.emplace(id, hit.getEnergy()); } } diff --git a/Ecal/src/Ecal/Event/EcalDigiCollection.cxx b/Ecal/src/Ecal/Event/EcalDigiCollection.cxx index 036c02d08..75ada50e0 100644 --- a/Ecal/src/Ecal/Event/EcalDigiCollection.cxx +++ b/Ecal/src/Ecal/Event/EcalDigiCollection.cxx @@ -37,8 +37,10 @@ ClassImp(ldmx::EcalDigiCollection) // this is where the word --> measurements translation occurs - bool firstFlag = ONE_BIT_MASK & (word >> FIRSTFLAG_POS); - bool seconFlag = ONE_BIT_MASK & (word >> SECONFLAG_POS); + // Unused, should it be? FIXME + // bool firstFlag = ONE_BIT_MASK & (word >> FIRSTFLAG_POS); + // Unused, should it be? FIXME + // bool seconFlag = ONE_BIT_MASK & (word >> SECONFLAG_POS); int firstMeas = TEN_BIT_MASK & (word >> FIRSTMEAS_POS); int seconMeas = TEN_BIT_MASK & (word >> SECONMEAS_POS); int lastMeas = TEN_BIT_MASK & (word); diff --git a/Ecal/src/Ecal/Event/EcalVetoResult.cxx b/Ecal/src/Ecal/Event/EcalVetoResult.cxx index df5d652be..29e2a1da2 100644 --- a/Ecal/src/Ecal/Event/EcalVetoResult.cxx +++ b/Ecal/src/Ecal/Event/EcalVetoResult.cxx @@ -1,185 +1,185 @@ #include "Ecal/Event/EcalVetoResult.h" -ClassImp(ldmx::EcalVetoResult) - - namespace ldmx { - EcalVetoResult::EcalVetoResult() {} - - EcalVetoResult::~EcalVetoResult() { Clear(); } - - void EcalVetoResult::Clear() { - passesVeto_ = false; - - nReadoutHits_ = 0; - summedDet_ = 0; - summedTightIso_ = 0; - maxCellDep_ = 0; - showerRMS_ = 0; - xStd_ = 0; - yStd_ = 0; - avgLayerHit_ = 0; - stdLayerHit_ = 0; - deepestLayerHit_ = 0; - ecalBackEnergy_ = 0; - // MIP tracking - nStraightTracks_ = 0; - nLinregTracks_ = 0; - firstNearPhLayer_ = 0; - nNearPhHits_ = 0; - photonTerritoryHits_ = 0; - epAng_ = 0; - epSep_ = 0; - epDot_ = 0; - - electronContainmentEnergy_.clear(); - photonContainmentEnergy_.clear(); - outsideContainmentEnergy_.clear(); - outsideContainmentNHits_.clear(); - outsideContainmentXStd_.clear(); - outsideContainmentYStd_.clear(); - - energySeg_.clear(); - xMeanSeg_.clear(); - yMeanSeg_.clear(); - xStdSeg_.clear(); - yStdSeg_.clear(); - layerMeanSeg_.clear(); - layerStdSeg_.clear(); - - eContEnergy_.clear(); - eContXMean_.clear(); - eContYMean_.clear(); - gContEnergy_.clear(); - gContNHits_.clear(); - gContXMean_.clear(); - gContYMean_.clear(); - oContEnergy_.clear(); - oContNHits_.clear(); - oContXMean_.clear(); - oContYMean_.clear(); - oContXStd_.clear(); - oContYStd_.clear(); - oContLayerMean_.clear(); - oContLayerStd_.clear(); - - discValue_ = 0; - - recoilPx_ = -9999; - recoilPy_ = -9999; - recoilPz_ = -9999; - recoilX_ = -9999; - recoilY_ = -9999; - - ecalLayerEdepReadout_.clear(); +ClassImp(ldmx::EcalVetoResult); + +namespace ldmx { +EcalVetoResult::EcalVetoResult() {} + +EcalVetoResult::~EcalVetoResult() { Clear(); } + +void EcalVetoResult::Clear() { + passesVeto_ = false; + + nReadoutHits_ = 0; + summedDet_ = 0; + summedTightIso_ = 0; + maxCellDep_ = 0; + showerRMS_ = 0; + xStd_ = 0; + yStd_ = 0; + avgLayerHit_ = 0; + stdLayerHit_ = 0; + deepestLayerHit_ = 0; + ecalBackEnergy_ = 0; + // MIP tracking + nStraightTracks_ = 0; + nLinregTracks_ = 0; + firstNearPhLayer_ = 0; + nNearPhHits_ = 0; + photonTerritoryHits_ = 0; + epAng_ = 0; + epSep_ = 0; + epDot_ = 0; + + electronContainmentEnergy_.clear(); + photonContainmentEnergy_.clear(); + outsideContainmentEnergy_.clear(); + outsideContainmentNHits_.clear(); + outsideContainmentXStd_.clear(); + outsideContainmentYStd_.clear(); + + energySeg_.clear(); + xMeanSeg_.clear(); + yMeanSeg_.clear(); + xStdSeg_.clear(); + yStdSeg_.clear(); + layerMeanSeg_.clear(); + layerStdSeg_.clear(); + + eContEnergy_.clear(); + eContXMean_.clear(); + eContYMean_.clear(); + gContEnergy_.clear(); + gContNHits_.clear(); + gContXMean_.clear(); + gContYMean_.clear(); + oContEnergy_.clear(); + oContNHits_.clear(); + oContXMean_.clear(); + oContYMean_.clear(); + oContXStd_.clear(); + oContYStd_.clear(); + oContLayerMean_.clear(); + oContLayerStd_.clear(); + + discValue_ = 0; + + recoilPx_ = -9999; + recoilPy_ = -9999; + recoilPz_ = -9999; + recoilX_ = -9999; + recoilY_ = -9999; + + ecalLayerEdepReadout_.clear(); +} + +void EcalVetoResult::setVariables( + int nReadoutHits, int deepestLayerHit, float summedDet, + float summedTightIso, float maxCellDep, float showerRMS, float xStd, + float yStd, float avgLayerHit, float stdLayerHit, float ecalBackEnergy, + int nStraightTracks, int nLinregTracks, int firstNearPhLayer, + int nNearPhHits, int photonTerritoryHits, float epAng, float epSep, + float epDot, + + std::vector electronContainmentEnergy, + std::vector photonContainmentEnergy, + std::vector outsideContainmentEnergy, + std::vector outsideContainmentNHits, + std::vector outsideContainmentXStd, + std::vector outsideContainmentYStd, + + std::vector energySeg, std::vector xMeanSeg, + std::vector yMeanSeg, std::vector xStdSeg, + std::vector yStdSeg, std::vector layerMeanSeg, + std::vector layerStdSeg, + + std::vector> eContEnergy, + std::vector> eContXMean, + std::vector> eContYMean, + std::vector> gContEnergy, + std::vector> gContNHits, + std::vector> gContXMean, + std::vector> gContYMean, + std::vector> oContEnergy, + std::vector> oContNHits, + std::vector> oContXMean, + std::vector> oContYMean, + std::vector> oContXStd, + std::vector> oContYStd, + std::vector> oContLayerMean, + std::vector> oContLayerStd, + + std::vector EcalLayerEdepReadout, std::vector recoilP, + std::vector recoilPos) { + nReadoutHits_ = nReadoutHits; + summedDet_ = summedDet; + summedTightIso_ = summedTightIso; + maxCellDep_ = maxCellDep; + showerRMS_ = showerRMS; + xStd_ = xStd; + yStd_ = yStd; + avgLayerHit_ = avgLayerHit; + stdLayerHit_ = stdLayerHit; + deepestLayerHit_ = deepestLayerHit; + ecalBackEnergy_ = ecalBackEnergy; + // MIP tracking + nStraightTracks_ = nStraightTracks; + nLinregTracks_ = nLinregTracks; + firstNearPhLayer_ = firstNearPhLayer; + nNearPhHits_ = nNearPhHits; + photonTerritoryHits_ = photonTerritoryHits; + epAng_ = epAng; + epSep_ = epSep; + epDot_ = epDot; + + electronContainmentEnergy_ = electronContainmentEnergy; + photonContainmentEnergy_ = photonContainmentEnergy; + outsideContainmentEnergy_ = outsideContainmentEnergy; + outsideContainmentNHits_ = outsideContainmentNHits; + outsideContainmentXStd_ = outsideContainmentXStd; + outsideContainmentYStd_ = outsideContainmentYStd; + + energySeg_ = energySeg; + xMeanSeg_ = xMeanSeg; + yMeanSeg_ = yMeanSeg; + xStdSeg_ = xStdSeg; + yStdSeg_ = yStdSeg; + layerMeanSeg_ = layerMeanSeg; + layerStdSeg_ = layerStdSeg; + + eContEnergy_ = eContEnergy; + eContXMean_ = eContXMean; + eContYMean_ = eContYMean; + gContEnergy_ = gContEnergy; + gContNHits_ = gContNHits; + gContXMean_ = gContXMean; + gContYMean_ = gContYMean; + oContEnergy_ = oContEnergy; + oContNHits_ = oContNHits; + oContXMean_ = oContXMean; + oContYMean_ = oContYMean; + oContXStd_ = oContXStd; + oContYStd_ = oContYStd; + oContLayerMean_ = oContLayerMean; + oContLayerStd_ = oContLayerStd; + + // discvalue not set here + + if (!recoilP.empty()) { + recoilPx_ = recoilP[0]; + recoilPy_ = recoilP[1]; + recoilPz_ = recoilP[2]; + recoilX_ = recoilPos[0]; + recoilY_ = recoilPos[1]; } - void EcalVetoResult::setVariables( - int nReadoutHits, int deepestLayerHit, float summedDet, - float summedTightIso, float maxCellDep, float showerRMS, float xStd, - float yStd, float avgLayerHit, float stdLayerHit, float ecalBackEnergy, - int nStraightTracks, int nLinregTracks, int firstNearPhLayer, - int nNearPhHits, int photonTerritoryHits, float epAng, float epSep, - float epDot, - - std::vector electronContainmentEnergy, - std::vector photonContainmentEnergy, - std::vector outsideContainmentEnergy, - std::vector outsideContainmentNHits, - std::vector outsideContainmentXStd, - std::vector outsideContainmentYStd, - - std::vector energySeg, std::vector xMeanSeg, - std::vector yMeanSeg, std::vector xStdSeg, - std::vector yStdSeg, std::vector layerMeanSeg, - std::vector layerStdSeg, - - std::vector> eContEnergy, - std::vector> eContXMean, - std::vector> eContYMean, - std::vector> gContEnergy, - std::vector> gContNHits, - std::vector> gContXMean, - std::vector> gContYMean, - std::vector> oContEnergy, - std::vector> oContNHits, - std::vector> oContXMean, - std::vector> oContYMean, - std::vector> oContXStd, - std::vector> oContYStd, - std::vector> oContLayerMean, - std::vector> oContLayerStd, - - std::vector EcalLayerEdepReadout, std::vector recoilP, - std::vector recoilPos) { - nReadoutHits_ = nReadoutHits; - summedDet_ = summedDet; - summedTightIso_ = summedTightIso; - maxCellDep_ = maxCellDep; - showerRMS_ = showerRMS; - xStd_ = xStd; - yStd_ = yStd; - avgLayerHit_ = avgLayerHit; - stdLayerHit_ = stdLayerHit; - deepestLayerHit_ = deepestLayerHit; - ecalBackEnergy_ = ecalBackEnergy; - // MIP tracking - nStraightTracks_ = nStraightTracks; - nLinregTracks_ = nLinregTracks; - firstNearPhLayer_ = firstNearPhLayer; - nNearPhHits_ = nNearPhHits; - photonTerritoryHits_ = photonTerritoryHits; - epAng_ = epAng; - epSep_ = epSep; - epDot_ = epDot; - - electronContainmentEnergy_ = electronContainmentEnergy; - photonContainmentEnergy_ = photonContainmentEnergy; - outsideContainmentEnergy_ = outsideContainmentEnergy; - outsideContainmentNHits_ = outsideContainmentNHits; - outsideContainmentXStd_ = outsideContainmentXStd; - outsideContainmentYStd_ = outsideContainmentYStd; - - energySeg_ = energySeg; - xMeanSeg_ = xMeanSeg; - yMeanSeg_ = yMeanSeg; - xStdSeg_ = xStdSeg; - yStdSeg_ = yStdSeg; - layerMeanSeg_ = layerMeanSeg; - layerStdSeg_ = layerStdSeg; - - eContEnergy_ = eContEnergy; - eContXMean_ = eContXMean; - eContYMean_ = eContYMean; - gContEnergy_ = gContEnergy; - gContNHits_ = gContNHits; - gContXMean_ = gContXMean; - gContYMean_ = gContYMean; - oContEnergy_ = oContEnergy; - oContNHits_ = oContNHits; - oContXMean_ = oContXMean; - oContYMean_ = oContYMean; - oContXStd_ = oContXStd; - oContYStd_ = oContYStd; - oContLayerMean_ = oContLayerMean; - oContLayerStd_ = oContLayerStd; - - // discvalue not set here - - if (!recoilP.empty()) { - recoilPx_ = recoilP[0]; - recoilPy_ = recoilP[1]; - recoilPz_ = recoilP[2]; - recoilX_ = recoilPos[0]; - recoilY_ = recoilPos[1]; - } - - ecalLayerEdepReadout_ = EcalLayerEdepReadout; - } + ecalLayerEdepReadout_ = EcalLayerEdepReadout; +} - void EcalVetoResult::Print() const { - std::cout << "[ EcalVetoResult ]:\n" - << "\t Passes veto : " << passesVeto_ << "\n" - << std::endl; - } +void EcalVetoResult::Print() const { + std::cout << "[ EcalVetoResult ]:\n" + << "\t Passes veto : " << passesVeto_ << "\n" + << std::endl; +} } // namespace ldmx diff --git a/Ecal/test/EcalDigiPipelineTest.cxx b/Ecal/test/EcalDigiPipelineTest.cxx index ab6e7f3c5..d77f9e49d 100644 --- a/Ecal/test/EcalDigiPipelineTest.cxx +++ b/Ecal/test/EcalDigiPipelineTest.cxx @@ -87,12 +87,6 @@ static const double MAX_ENERGY_ERROR_TP = 2 * MIP_SI_ENERGY; */ static const int NUM_TEST_SIM_HITS = 2000; -/** - * Should the sim/rec/tp energies be ntuplized - * for your viewing? - */ -static const bool NTUPLIZE_ENERGIES = true; - /** * Our custom energy checker which makes sure that * the input energy is "close enough" to the truth @@ -314,7 +308,7 @@ DECLARE_ANALYZER_NS(ecal::test, EcalCheckEnergyReconstruction) TEST_CASE("Ecal Digi Pipeline test", "[Ecal][functionality]") { const std::string config_file{"ecal_digi_pipeline_test_config.py"}; - char **args; + char **args{nullptr}; framework::ProcessHandle p; framework::ConfigurePython cfg(config_file, args, 0); From 6b1423f3fad3bba56a7eb34fb136abce68a14656 Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Tue, 10 Sep 2024 13:10:10 -0700 Subject: [PATCH 05/14] Static analysis improvement for Framework --- Framework/include/Framework/Performance/Timer.h | 13 +++++++++---- .../src/Framework/ConditionsObjectProvider.cxx | 6 +++--- Framework/src/Framework/ConfigurePython.cxx | 2 +- Framework/src/Framework/EventFile.cxx | 4 ++-- Framework/src/Framework/EventProcessor.cxx | 8 ++++---- Framework/test/ConfigurePythonTest.cxx | 3 ++- 6 files changed, 21 insertions(+), 15 deletions(-) diff --git a/Framework/include/Framework/Performance/Timer.h b/Framework/include/Framework/Performance/Timer.h index 8292c7ca1..44b920f5d 100644 --- a/Framework/include/Framework/Performance/Timer.h +++ b/Framework/include/Framework/Performance/Timer.h @@ -43,22 +43,25 @@ class Timer { * The comment beginning with `//!` is what marks this member * as "transient" for ROOT I/O. */ - std::chrono::time_point - begin_; //! not serialized, just for measurement purposes + //! not serialized, just for measurement purposes + std::chrono::time_point begin_; + /** * The time_point when the timer is stopped. * * The comment beginning with `//!` is what marks this member * as "transient" for ROOT I/O. */ - std::chrono::time_point - end_; //! not serialized, just for measurement purposes + //! not serialized, just for measurement purposes + std::chrono::time_point end_; + /** * Time stamp for when timer was started in nanoseconds since UNIX epoch * * Set to -1 if timer was not started */ long int start_time_{-1}; + /** * Length of time recorded by the timer in seconds * @@ -73,6 +76,8 @@ class Timer { public: /// create a timer but don't start it yet Timer() = default; + /// create defualt destructor + virtual ~Timer() = default; /// reset a timer to un-started state without re-allocating void reset(); /// start the timer diff --git a/Framework/src/Framework/ConditionsObjectProvider.cxx b/Framework/src/Framework/ConditionsObjectProvider.cxx index b6608723a..78f6a2d27 100644 --- a/Framework/src/Framework/ConditionsObjectProvider.cxx +++ b/Framework/src/Framework/ConditionsObjectProvider.cxx @@ -9,10 +9,10 @@ namespace framework { ConditionsObjectProvider::ConditionsObjectProvider( const std::string& objname, const std::string& tagname, const framework::config::Parameters& params, Process& process) - : process_{process}, + : theLog_{logging::makeLogger(objname)}, + process_{process}, objectName_{objname}, - tagname_{tagname}, - theLog_{logging::makeLogger(objname)} {} + tagname_{tagname} {} std::pair ConditionsObjectProvider::requestParentCondition( diff --git a/Framework/src/Framework/ConfigurePython.cxx b/Framework/src/Framework/ConfigurePython.cxx index 21a03a4ad..427f5280e 100644 --- a/Framework/src/Framework/ConfigurePython.cxx +++ b/Framework/src/Framework/ConfigurePython.cxx @@ -267,7 +267,7 @@ static std::map getMembers(PyObject* object) { } // python object type } // loop through python dictionary - return std::move(params); + return params; } ConfigurePython::ConfigurePython(const std::string& pythonScript, char* args[], diff --git a/Framework/src/Framework/EventFile.cxx b/Framework/src/Framework/EventFile.cxx index b5e35c87d..9e7f3b289 100644 --- a/Framework/src/Framework/EventFile.cxx +++ b/Framework/src/Framework/EventFile.cxx @@ -14,10 +14,10 @@ EventFile::EventFile(const framework::config::Parameters ¶ms, const std::string &filename, EventFile *parent, bool isOutputFile, bool isSingleOutput, bool isLoopable) : fileName_(filename), - parent_(parent), isOutputFile_(isOutputFile), isSingleOutput_(isSingleOutput), - isLoopable_(isLoopable) { + isLoopable_(isLoopable), + parent_(parent) { if (isOutputFile_) { // we are writting out so open the file and make sure it is writable file_ = new TFile(fileName_.c_str(), "RECREATE"); diff --git a/Framework/src/Framework/EventProcessor.cxx b/Framework/src/Framework/EventProcessor.cxx index a92b8fa9e..082990dfd 100644 --- a/Framework/src/Framework/EventProcessor.cxx +++ b/Framework/src/Framework/EventProcessor.cxx @@ -9,10 +9,10 @@ namespace framework { EventProcessor::EventProcessor(const std::string &name, Process &process) - : process_{process}, - name_{name}, - histograms_{name}, - theLog_{logging::makeLogger(name)} {} + : histograms_{name}, + theLog_{logging::makeLogger(name)}, + process_{process}, + name_{name} {} Conditions &EventProcessor::getConditions() const { return process_.getConditions(); diff --git a/Framework/test/ConfigurePythonTest.cxx b/Framework/test/ConfigurePythonTest.cxx index 255831e23..a2a421951 100644 --- a/Framework/test/ConfigurePythonTest.cxx +++ b/Framework/test/ConfigurePythonTest.cxx @@ -109,7 +109,8 @@ class TestConfig : public framework::Producer { * Sometimes it is helpful to leave the generated files, so * maybe we can make the removal optional? */ -static bool removeFile(const char *filepath) { return remove(filepath) == 0; } +// static bool removeFile(const char *filepath) { return remove(filepath) == 0; +// } } // namespace test } // namespace framework From 1e23a270ff793ba9b06a03257010b26e3b9c75f3 Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Tue, 10 Sep 2024 13:10:10 -0700 Subject: [PATCH 06/14] Static analysis improvement for Hcal --- Hcal/include/Hcal/HcalClusterProducer.h | 1 - Hcal/include/Hcal/MyClusterWeight.h | 9 +- Hcal/src/Hcal/HcalDigiProducer.cxx | 6 + Hcal/test/HcalDigiPipelineTest.cxx | 49 ++++---- Hcal/test/HcalGeometryTest.cxx | 158 +++++++++++++----------- 5 files changed, 121 insertions(+), 102 deletions(-) diff --git a/Hcal/include/Hcal/HcalClusterProducer.h b/Hcal/include/Hcal/HcalClusterProducer.h index 07488b77e..f30ad1443 100644 --- a/Hcal/include/Hcal/HcalClusterProducer.h +++ b/Hcal/include/Hcal/HcalClusterProducer.h @@ -47,7 +47,6 @@ class HcalClusterProducer : public framework::Producer { void produce(framework::Event& event) override; private: - bool verbose_{false}; // double EminSeed_{0.}; double EnoiseCut_{0.}; double deltaTime_{0}; diff --git a/Hcal/include/Hcal/MyClusterWeight.h b/Hcal/include/Hcal/MyClusterWeight.h index 68734bb5c..a0c1933fa 100644 --- a/Hcal/include/Hcal/MyClusterWeight.h +++ b/Hcal/include/Hcal/MyClusterWeight.h @@ -29,12 +29,13 @@ class MyClusterWeight { double bZ = b.centroid().Pz(); double dijz; - double eFrac; + // This is unused, should it be? FIXME + // double eFrac; if (aE >= bE) { - eFrac = bE / aE; // ratio of energies - dijz = bZ - aZ; // differences in Z + // eFrac = bE / aE; // ratio of energies + dijz = bZ - aZ; // differences in Z } else { - eFrac = aE / bE; + // eFrac = aE / bE; dijz = aZ - bZ; } diff --git a/Hcal/src/Hcal/HcalDigiProducer.cxx b/Hcal/src/Hcal/HcalDigiProducer.cxx index 98e47241d..65421cb73 100644 --- a/Hcal/src/Hcal/HcalDigiProducer.cxx +++ b/Hcal/src/Hcal/HcalDigiProducer.cxx @@ -185,6 +185,12 @@ void HcalDigiProducer::produce(framework::Event& event) { (section == ldmx::HcalID::HcalSection::RIGHT)) { distance_along_bar = position[1]; distance_ecal = ecal_dy; + } else { + distance_along_bar = -9999.; + EXCEPTION_RAISE( + "BadCode", + "We should never end up here " + "All cases of HCAL considered, end_close is meaningless"); } end_close = (distance_along_bar > half_total_width) ? 0 : 1; distance_close = (end_close == 0) diff --git a/Hcal/test/HcalDigiPipelineTest.cxx b/Hcal/test/HcalDigiPipelineTest.cxx index 44b569b10..cde6fd85a 100644 --- a/Hcal/test/HcalDigiPipelineTest.cxx +++ b/Hcal/test/HcalDigiPipelineTest.cxx @@ -27,8 +27,8 @@ static const double PE_ENERGY = 4.66 / 68; // 0.069 MeV * Conversion between voltage and deposited energy * [MeV/mV] * 1 PE ~ 5 mV + * static const double MeV_per_mV = PE_ENERGY / 5; // 0.013 MeV/mV */ -static const double MeV_per_mV = PE_ENERGY / 5; // 0.013 MeV/mV /** * Maximum error that a single hit energy/PE @@ -43,8 +43,8 @@ static const double MeV_per_mV = PE_ENERGY / 5; // 0.013 MeV/mV // static const double MAX_ENERGY_ERROR_DAQ = 4 * PE_ENERGY; // static const double MAX_ENERGY_PERCENT_ERROR_DAQ = 0.2; static const double MAX_PE_ERROR_DAQ = 40; -static const double MAX_PE_PERCENT_ERROR_DAQ = - 0.4; // large percentage error for now +// large percentage error for now +static const double MAX_PE_PERCENT_ERROR_DAQ = 0.4; /** * Maximum error that a single hit position along the bar @@ -54,9 +54,9 @@ static const double MAX_PE_PERCENT_ERROR_DAQ = * Comparing simulated position vs * reconstructed position along the bar for even/odd layers in the back Hcal. */ -static const double MAX_POSITION_ERROR_DAQ = - 50. / 2; // mm // scintillator length/2 -static const double MAX_POSITION_PERCENT_ERROR_DAQ = 0.3; +// mm // scintillator length/2 +// static const double MAX_POSITION_ERROR_DAQ = 50. / 2; +// static const double MAX_POSITION_PERCENT_ERROR_DAQ = 0.3; /** * Number of sim hits to create. @@ -288,23 +288,24 @@ class HcalCheckReconstruction : public framework::Analyzer { // << std::endl; // std::cout << "npes " << hit.getPE() << " approx PE " << int(truth_energy // / PE_ENERGY) << std::endl; - - if (id.section() == 0) { - double truth_pos, rec_pos; - if ((id.layer() % 2) == 1) { - truth_pos = simHits.at(0).getPosition()[0]; - rec_pos = hit.getXPos(); - } else { - truth_pos = simHits.at(0).getPosition()[1]; - rec_pos = hit.getYPos(); - } - // std::cout << "rec pos " << rec_pos << " truth " << truth_pos << - // std::endl; - // comment position check for now - // CHECK_THAT(rec_pos, isCloseEnough(truth_pos, MAX_POSITION_ERROR_DAQ, - // MAX_POSITION_PERCENT_ERROR_DAQ)); - } - + /* + if (id.section() == 0) { + double truth_pos, rec_pos; + if ((id.layer() % 2) == 1) { + truth_pos = simHits.at(0).getPosition()[0]; + rec_pos = hit.getXPos(); + } else { + truth_pos = simHits.at(0).getPosition()[1]; + rec_pos = hit.getYPos(); + } + // std::cout << "rec pos " << rec_pos << " truth " << truth_pos << + // std::endl; + // comment position check for now + // CHECK_THAT(rec_pos, isCloseEnough(truth_pos, + MAX_POSITION_ERROR_DAQ, + // MAX_POSITION_PERCENT_ERROR_DAQ)); + } + */ return; } }; // HcalCheckReconstruction @@ -332,7 +333,7 @@ DECLARE_ANALYZER_NS(hcal::test, HcalCheckReconstruction) TEST_CASE("Hcal Digi Pipeline test", "[Hcal][functionality]") { const std::string config_file{"hcal_digi_pipeline_test_config.py"}; - char **args; + char **args{nullptr}; framework::ProcessHandle p; framework::ConfigurePython cfg(config_file, args, 0); diff --git a/Hcal/test/HcalGeometryTest.cxx b/Hcal/test/HcalGeometryTest.cxx index 828561f9c..bf535705f 100644 --- a/Hcal/test/HcalGeometryTest.cxx +++ b/Hcal/test/HcalGeometryTest.cxx @@ -18,10 +18,10 @@ namespace test { * can be reconstructed/mapped. * 20% for now. */ -static const double MAX_ENERGY_PERCENT_ERROR_ZPOSITION = 20; -static const double MAX_ENERGY_PERCENT_ERROR_XPOSITION = 50; -static const double MAX_ENERGY_PERCENT_ERROR_YPOSITION = 50; -static const double MAX_ENERGY_PERCENT_ERROR_ZPOSITION_SIDE = 100; +// static const double MAX_ENERGY_PERCENT_ERROR_ZPOSITION = 20; +// static const double MAX_ENERGY_PERCENT_ERROR_XPOSITION = 50; +// static const double MAX_ENERGY_PERCENT_ERROR_YPOSITION = 50; +// static const double MAX_ENERGY_PERCENT_ERROR_ZPOSITION_SIDE = 100; /** * @class HcalCheckPositionMap @@ -43,74 +43,86 @@ class HcalCheckPositionMap : public framework::Analyzer { event.getCollection("HcalSimHits"); CHECK(simHits.size() > 0); - - for (int ihit = 0; ihit < simHits.size(); ihit++) { - auto hit = simHits.at(ihit); - double x = hit.getPosition()[0]; - double y = hit.getPosition()[1]; - double z = hit.getPosition()[2]; - - unsigned int hitID = hit.getID(); - ldmx::HcalID detID(hitID); - int section = detID.section(); - int layer = detID.layer(); - int strip = detID.strip(); - // std::cout << "ihit " << ihit << " section " << detID.section() << - // " layer " << detID.layer() << " strip " << detID.strip() << std::endl; - - // get the Hcal geometry - const auto &hcalGeometry = getCondition( - ldmx::HcalGeometry::CONDITIONS_OBJECT_NAME); - - // get position - auto positionMap = hcalGeometry.getStripCenterPosition(detID); - - if (section == ldmx::HcalID::HcalSection::BACK) { - auto target_z = - Approx(z).epsilon(MAX_ENERGY_PERCENT_ERROR_ZPOSITION / 100); - // CHECK(positionMap.Z() == target_z); - // std::cout << " BACK " << "x sim hit " << x << " (Sec,strip,layer) " - // << section << " " << strip << " " << layer << " x " << - // positionMap.X() << std::endl; std::cout << " BACK " << "y sim hit " - // << y << " (Sec,strip,layer) " << section << " " << strip << " " << - // layer << " y " << positionMap.Y() << std::endl; std::cout << " BACK " - // << "z sim hit " << z << " (Sec,strip,layer) " << section << " " << - // strip << " " << layer << " z " << positionMap.Z() << std::endl; - - if (layer % 2 == 1) { - auto target_y = - Approx(y).epsilon(MAX_ENERGY_PERCENT_ERROR_YPOSITION / 100); - // CHECK(positionMap.Y() == target_y); - } else { - auto target_x = - Approx(x).epsilon(MAX_ENERGY_PERCENT_ERROR_YPOSITION / 100); - // CHECK(positionMap.X() == target_x); - } - } else { - auto target_z = - Approx(z).epsilon(MAX_ENERGY_PERCENT_ERROR_ZPOSITION_SIDE / 100); - // CHECK(positionMap.Z() == target_z); - // std::cout << " SIDE " << "x sim hit " << x << " (Sec,strip,layer) " - // << section << " " << strip << " " << layer << " x " << - // positionMap.X() << std::endl; std::cout << " SIDE " << "y sim hit " - // << y << " (Sec,strip,layer) " << section << " " << strip << " " << - // layer << " y " << positionMap.Y() << std::endl; std::cout << " SIDE " - // << "z sim hit " << z << " (Sec,strip,layer) " << section << " " << - // strip << " " << layer << " z " << positionMap.Z() << std::endl; - - if ((section == ldmx::HcalID::HcalSection::TOP) || - (section == ldmx::HcalID::HcalSection::BOTTOM)) { - auto target_y = - Approx(y).epsilon(MAX_ENERGY_PERCENT_ERROR_YPOSITION / 100); - // CHECK(positionMap.Y() == target_y); - } else if ((section == ldmx::HcalID::HcalSection::LEFT) || - (section == ldmx::HcalID::HcalSection::RIGHT)) { - auto target_x = - Approx(x).epsilon(MAX_ENERGY_PERCENT_ERROR_YPOSITION / 100); - // CHECK(positionMap.X() == target_x); - } - } - } + /* + for (int ihit = 0; ihit < simHits.size(); ihit++) { + auto hit = simHits.at(ihit); + double x = hit.getPosition()[0]; + double y = hit.getPosition()[1]; + double z = hit.getPosition()[2]; + + unsigned int hitID = hit.getID(); + ldmx::HcalID detID(hitID); + int section = detID.section(); + int layer = detID.layer(); + int strip = detID.strip(); + // std::cout << "ihit " << ihit << " section " << detID.section() << + // " layer " << detID.layer() << " strip " << detID.strip() << + std::endl; + + // get the Hcal geometry + const auto &hcalGeometry = getCondition( + ldmx::HcalGeometry::CONDITIONS_OBJECT_NAME); + + // get position + auto positionMap = hcalGeometry.getStripCenterPosition(detID); + + if (section == ldmx::HcalID::HcalSection::BACK) { + auto target_z = + Approx(z).epsilon(MAX_ENERGY_PERCENT_ERROR_ZPOSITION / 100); + // CHECK(positionMap.Z() == target_z); + // std::cout << " BACK " << "x sim hit " << x << " (Sec,strip,layer) + " + // << section << " " << strip << " " << layer << " x " << + // positionMap.X() << std::endl; std::cout << " BACK " << "y sim hit + " + // << y << " (Sec,strip,layer) " << section << " " << strip << " " + << + // layer << " y " << positionMap.Y() << std::endl; std::cout << " + BACK " + // << "z sim hit " << z << " (Sec,strip,layer) " << section << " " + << + // strip << " " << layer << " z " << positionMap.Z() << std::endl; + + if (layer % 2 == 1) { + auto target_y = + Approx(y).epsilon(MAX_ENERGY_PERCENT_ERROR_YPOSITION / 100); + // CHECK(positionMap.Y() == target_y); + } else { + auto target_x = + Approx(x).epsilon(MAX_ENERGY_PERCENT_ERROR_YPOSITION / 100); + // CHECK(positionMap.X() == target_x); + } + } else { + auto target_z = + Approx(z).epsilon(MAX_ENERGY_PERCENT_ERROR_ZPOSITION_SIDE / + 100); + // CHECK(positionMap.Z() == target_z); + // std::cout << " SIDE " << "x sim hit " << x << " (Sec,strip,layer) + " + // << section << " " << strip << " " << layer << " x " << + // positionMap.X() << std::endl; std::cout << " SIDE " << "y sim hit + " + // << y << " (Sec,strip,layer) " << section << " " << strip << " " + << + // layer << " y " << positionMap.Y() << std::endl; std::cout << " + SIDE " + // << "z sim hit " << z << " (Sec,strip,layer) " << section << " " + << + // strip << " " << layer << " z " << positionMap.Z() << std::endl; + + if ((section == ldmx::HcalID::HcalSection::TOP) || + (section == ldmx::HcalID::HcalSection::BOTTOM)) { + auto target_y = + Approx(y).epsilon(MAX_ENERGY_PERCENT_ERROR_YPOSITION / 100); + // CHECK(positionMap.Y() == target_y); + } else if ((section == ldmx::HcalID::HcalSection::LEFT) || + (section == ldmx::HcalID::HcalSection::RIGHT)) { + auto target_x = + Approx(x).epsilon(MAX_ENERGY_PERCENT_ERROR_YPOSITION / 100); + // CHECK(positionMap.X() == target_x); + } + } + } */ return; } }; // HcalCheckPositionMap @@ -126,7 +138,7 @@ DECLARE_ANALYZER_NS(hcal::test, HcalCheckPositionMap) TEST_CASE("Hcal Geometry test", "[Hcal][functionality]") { const std::string config_file{"hcal_geometry_test_config.py"}; - char **args; + char **args{nullptr}; framework::ProcessHandle p; framework::ConfigurePython cfg(config_file, args, 0); From 788400d1bc02d42c6a5099d891b018e841afba6e Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Tue, 10 Sep 2024 13:10:11 -0700 Subject: [PATCH 07/14] Static analysis improvement for Packing, Tools and Recon --- Packing/src/Packing/RawDataFile/File.cxx | 12 +++++---- Recon/include/Recon/PFEcalClusterProducer.h | 10 -------- Recon/include/Recon/PFHcalClusterProducer.h | 10 -------- Recon/include/Recon/PFTrackProducer.h | 10 -------- Recon/include/Recon/PFTruthProducer.h | 12 --------- Recon/include/Recon/ParticleFlow.h | 9 ------- Recon/src/Recon/PFEcalClusterProducer.cxx | 28 +-------------------- Recon/src/Recon/PFHcalClusterProducer.cxx | 26 +------------------ Recon/src/Recon/PFTrackProducer.cxx | 25 ------------------ Recon/src/Recon/PFTruthProducer.cxx | 25 ------------------ Recon/src/Recon/ParticleFlow.cxx | 18 ------------- Tools/include/Tools/HgcrocEmulator.h | 6 ++--- Tools/src/Tools/HgcrocEmulator.cxx | 6 +++-- 13 files changed, 16 insertions(+), 181 deletions(-) diff --git a/Packing/src/Packing/RawDataFile/File.cxx b/Packing/src/Packing/RawDataFile/File.cxx index f0ef35570..b79a5b886 100644 --- a/Packing/src/Packing/RawDataFile/File.cxx +++ b/Packing/src/Packing/RawDataFile/File.cxx @@ -43,8 +43,8 @@ File::File(const framework::config::Parameters &ps) { run_ = ((word >> 4) & utility::mask<28>); reader_.seek(-2, std::ios::end); - auto eof{ - reader_.tell()}; // save EOF in number of 32-bit-width words + // save EOF in number of 32-bit-width words + auto eof{reader_.tell()}; uint32_t crc_read_in; reader_ >> entries_ >> crc_read_in; i_entry_ = 0; @@ -130,9 +130,11 @@ void File::writeRunHeader(ldmx::RunHeader &header) { if (is_output_) { // use passed run number run_ = header.getRunNumber(); - uint32_t header = (0 & utility::mask<4>)+((run_ & utility::mask<28>) << 4); - writer_ << header; - crc_ << header; + // Why cant we just take the header from above? + uint32_t tempHeader = + (0 & utility::mask<4>)+((run_ & utility::mask<28>) << 4); + writer_ << tempHeader; + crc_ << tempHeader; } else { // put our read-in run number here header.setIntParameter("raw_run", run_); diff --git a/Recon/include/Recon/PFEcalClusterProducer.h b/Recon/include/Recon/PFEcalClusterProducer.h index 5cfb47958..d89598a9e 100644 --- a/Recon/include/Recon/PFEcalClusterProducer.h +++ b/Recon/include/Recon/PFEcalClusterProducer.h @@ -35,17 +35,7 @@ class PFEcalClusterProducer : public framework::Producer { virtual void produce(framework::Event& event); - virtual void onFileOpen(); - - virtual void onFileClose(); - - virtual void onProcessStart(); - - virtual void onProcessEnd(); - private: - // specific verbosity of this producer - int verbose_{0}; bool singleCluster_{true}; bool logEnergyWeight_{true}; diff --git a/Recon/include/Recon/PFHcalClusterProducer.h b/Recon/include/Recon/PFHcalClusterProducer.h index a52285f31..5777bfadb 100644 --- a/Recon/include/Recon/PFHcalClusterProducer.h +++ b/Recon/include/Recon/PFHcalClusterProducer.h @@ -27,17 +27,7 @@ class PFHcalClusterProducer : public framework::Producer { virtual void produce(framework::Event& event); - virtual void onFileOpen(); - - virtual void onFileClose(); - - virtual void onProcessStart(); - - virtual void onProcessEnd(); - private: - // specific verbosity of this producer - int verbose_{0}; bool singleCluster_{true}; bool logEnergyWeight_{true}; diff --git a/Recon/include/Recon/PFTrackProducer.h b/Recon/include/Recon/PFTrackProducer.h index 5e1e38849..f2d3ce545 100644 --- a/Recon/include/Recon/PFTrackProducer.h +++ b/Recon/include/Recon/PFTrackProducer.h @@ -27,17 +27,7 @@ class PFTrackProducer : public framework::Producer { virtual void produce(framework::Event& event); - virtual void onFileOpen(); - - virtual void onFileClose(); - - virtual void onProcessStart(); - - virtual void onProcessEnd(); - private: - // specific verbosity of this producer - int verbose_{0}; bool truthTracking_{true}; // name of collection for track inputs to be passed diff --git a/Recon/include/Recon/PFTruthProducer.h b/Recon/include/Recon/PFTruthProducer.h index b3122c98a..8c0936dc9 100644 --- a/Recon/include/Recon/PFTruthProducer.h +++ b/Recon/include/Recon/PFTruthProducer.h @@ -27,19 +27,7 @@ class PFTruthProducer : public framework::Producer { virtual void produce(framework::Event& event); - virtual void onFileOpen(); - - virtual void onFileClose(); - - virtual void onProcessStart(); - - virtual void onProcessEnd(); - private: - // specific verbosity of this producer - int verbose_{0}; - bool truthTracking_{true}; - // name of collection for target, ecal, hcal truth to be output std::string primaryCollName_; std::string targetCollName_; diff --git a/Recon/include/Recon/ParticleFlow.h b/Recon/include/Recon/ParticleFlow.h index 6784757d9..7afab6df2 100644 --- a/Recon/include/Recon/ParticleFlow.h +++ b/Recon/include/Recon/ParticleFlow.h @@ -34,12 +34,6 @@ class ParticleFlow : public framework::Producer { virtual void produce(framework::Event& event); - virtual void onFileOpen(); - - virtual void onFileClose(); - - virtual void onProcessStart(); - virtual void onProcessEnd(); void fillCandTrack(ldmx::PFCandidate& cand, const ldmx::SimTrackerHit& tk); @@ -47,9 +41,6 @@ class ParticleFlow : public framework::Producer { void fillCandHadCalo(ldmx::PFCandidate& cand, const ldmx::CaloCluster& had); private: - // specific verbosity of this producer - int verbose_{0}; - TGraph* eCorr_{0}; TGraph* hCorr_{0}; diff --git a/Recon/src/Recon/PFEcalClusterProducer.cxx b/Recon/src/Recon/PFEcalClusterProducer.cxx index 1bb2f6c08..87cfa3300 100644 --- a/Recon/src/Recon/PFEcalClusterProducer.cxx +++ b/Recon/src/Recon/PFEcalClusterProducer.cxx @@ -22,8 +22,6 @@ void PFEcalClusterProducer::configure(framework::config::Parameters& ps) { void PFEcalClusterProducer::produce(framework::Event& event) { if (!event.exists(hitCollName_)) return; const auto ecalRecHits = event.getCollection(hitCollName_); - const auto& geo = getCondition( - ldmx::EcalGeometry::CONDITIONS_OBJECT_NAME); float eTotal = 0; for (const auto& h : ecalRecHits) eTotal += h.getEnergy(); @@ -37,7 +35,7 @@ void PFEcalClusterProducer::produce(framework::Event& event) { std::vector > all_hit_ptrs = cb.runDBSCAN(ptrs, false); - for (const auto hit_ptrs : all_hit_ptrs) { + for (const auto& hit_ptrs : all_hit_ptrs) { ldmx::CaloCluster cl; cb.fillClusterInfoFromHits(&cl, hit_ptrs, logEnergyWeight_); pfClusters.push_back(cl); @@ -63,30 +61,6 @@ void PFEcalClusterProducer::produce(framework::Event& event) { event.add("EcalTotalEnergy" + suffix_, eTotal); } -void PFEcalClusterProducer::onFileOpen() { - ldmx_log(debug) << "Opening file!"; - - return; -} - -void PFEcalClusterProducer::onFileClose() { - ldmx_log(debug) << "Closing file!"; - - return; -} - -void PFEcalClusterProducer::onProcessStart() { - ldmx_log(debug) << "Process starts!"; - - return; -} - -void PFEcalClusterProducer::onProcessEnd() { - ldmx_log(debug) << "Process ends!"; - - return; -} - } // namespace recon DECLARE_PRODUCER_NS(recon, PFEcalClusterProducer); diff --git a/Recon/src/Recon/PFHcalClusterProducer.cxx b/Recon/src/Recon/PFHcalClusterProducer.cxx index ce2060240..1039a03c0 100644 --- a/Recon/src/Recon/PFHcalClusterProducer.cxx +++ b/Recon/src/Recon/PFHcalClusterProducer.cxx @@ -39,7 +39,7 @@ void PFHcalClusterProducer::produce(framework::Event& event) { std::vector > all_hit_ptrs = cb.runDBSCAN(ptrs, false); - for (const auto hit_ptrs : all_hit_ptrs) { + for (const auto& hit_ptrs : all_hit_ptrs) { ldmx::CaloCluster cl; cb.fillClusterInfoFromHits(&cl, hit_ptrs, logEnergyWeight_); pfClusters.push_back(cl); @@ -66,30 +66,6 @@ void PFHcalClusterProducer::produce(framework::Event& event) { event.add("HcalTotalEnergy" + suffix_, eTotal); } -void PFHcalClusterProducer::onFileOpen() { - ldmx_log(debug) << "Opening file!"; - - return; -} - -void PFHcalClusterProducer::onFileClose() { - ldmx_log(debug) << "Closing file!"; - - return; -} - -void PFHcalClusterProducer::onProcessStart() { - ldmx_log(debug) << "Process starts!"; - - return; -} - -void PFHcalClusterProducer::onProcessEnd() { - ldmx_log(debug) << "Process ends!"; - - return; -} - } // namespace recon DECLARE_PRODUCER_NS(recon, PFHcalClusterProducer); diff --git a/Recon/src/Recon/PFTrackProducer.cxx b/Recon/src/Recon/PFTrackProducer.cxx index db6065263..b3e3bbe04 100644 --- a/Recon/src/Recon/PFTrackProducer.cxx +++ b/Recon/src/Recon/PFTrackProducer.cxx @@ -40,31 +40,6 @@ void PFTrackProducer::produce(framework::Event& event) { }); event.add(outputTrackCollName_, pfTracks); } - -void PFTrackProducer::onFileOpen() { - ldmx_log(debug) << "Opening file!"; - - return; -} - -void PFTrackProducer::onFileClose() { - ldmx_log(debug) << "Closing file!"; - - return; -} - -void PFTrackProducer::onProcessStart() { - ldmx_log(debug) << "Process starts!"; - - return; -} - -void PFTrackProducer::onProcessEnd() { - ldmx_log(debug) << "Process ends!"; - - return; -} - } // namespace recon DECLARE_PRODUCER_NS(recon, PFTrackProducer); diff --git a/Recon/src/Recon/PFTruthProducer.cxx b/Recon/src/Recon/PFTruthProducer.cxx index 423a08bd0..8fbdad9ec 100644 --- a/Recon/src/Recon/PFTruthProducer.cxx +++ b/Recon/src/Recon/PFTruthProducer.cxx @@ -69,31 +69,6 @@ void PFTruthProducer::produce(framework::Event &event) { event.add(ecalCollName_, atEcal); event.add(hcalCollName_, atHcal); } - -void PFTruthProducer::onFileOpen() { - ldmx_log(debug) << "Opening file!"; - - return; -} - -void PFTruthProducer::onFileClose() { - ldmx_log(debug) << "Closing file!"; - - return; -} - -void PFTruthProducer::onProcessStart() { - ldmx_log(debug) << "Process starts!"; - - return; -} - -void PFTruthProducer::onProcessEnd() { - ldmx_log(debug) << "Process ends!"; - - return; -} - } // namespace recon DECLARE_PRODUCER_NS(recon, PFTruthProducer); diff --git a/Recon/src/Recon/ParticleFlow.cxx b/Recon/src/Recon/ParticleFlow.cxx index a109aee8d..e98fe60ef 100644 --- a/Recon/src/Recon/ParticleFlow.cxx +++ b/Recon/src/Recon/ParticleFlow.cxx @@ -395,24 +395,6 @@ void ParticleFlow::produce(framework::Event& event) { event.add(outputCollName_, pfCands); } -void ParticleFlow::onFileOpen() { - ldmx_log(debug) << "Opening file!"; - - return; -} - -void ParticleFlow::onFileClose() { - ldmx_log(debug) << "Closing file!"; - - return; -} - -void ParticleFlow::onProcessStart() { - ldmx_log(debug) << "Process starts!"; - - return; -} - void ParticleFlow::onProcessEnd() { ldmx_log(debug) << "Process ends!"; delete eCorr_; diff --git a/Tools/include/Tools/HgcrocEmulator.h b/Tools/include/Tools/HgcrocEmulator.h index 5ca448af9..140c9e96f 100644 --- a/Tools/include/Tools/HgcrocEmulator.h +++ b/Tools/include/Tools/HgcrocEmulator.h @@ -340,15 +340,15 @@ class HgcrocEmulator { */ std::vector> hits_; + /// reference to pulse shape function shared by all pulses + TF1& pulseFunc_; + /// gain for current chip we are emulating double gain_; /// pedestal for current chip we are emulating double pedestal_; - /// reference to pulse shape function shared by all pulses - TF1& pulseFunc_; - }; // CompositePulse private: diff --git a/Tools/src/Tools/HgcrocEmulator.cxx b/Tools/src/Tools/HgcrocEmulator.cxx index 1fa820e44..d21d4beda 100644 --- a/Tools/src/Tools/HgcrocEmulator.cxx +++ b/Tools/src/Tools/HgcrocEmulator.cxx @@ -84,7 +84,8 @@ bool HgcrocEmulator::digitize( /// the time here is nominal (zero gives peak if hit.second is zero) // step 3: go through each BX sample one by one - bool doReadout = false; + // doReadout is unused, should it be? FIXME + // bool doReadout = false; bool wasTOA = false; for (int iADC = 0; iADC < nADCs_; iADC++) { double startBX = (iADC - iSOI_) * clockCycle_ - measTime; @@ -146,7 +147,8 @@ bool HgcrocEmulator::digitize( double tot = charge_deposited / drainRate; // calculate the index that tot will complete on - int num_whole_clocks = int(tot / clockCycle_); + // Unused, should it be? + // int num_whole_clocks = int(tot / clockCycle_); // calculate the TDC counts for this tot measurement // internally, the chip uses 12 bits (2^12 = 4096) From bc2adc882e671b995f371c01692b325538e83cdd Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Tue, 10 Sep 2024 13:10:11 -0700 Subject: [PATCH 08/14] Static analysis improvement for Tracking --- Tracking/include/Tracking/Event/Measurement.h | 2 +- Tracking/include/Tracking/Event/TruthTrack.h | 2 +- Tracking/include/Tracking/Reco/CKFProcessor.h | 3 +- .../Tracking/Reco/CustomStatePropagator.h | 2 +- Tracking/include/Tracking/Reco/GSFProcessor.h | 41 ++++++------ .../Tracking/Reco/SeedFinderProcessor.h | 6 +- .../Tracking/Reco/TrackExtrapolatorTool.h | 12 ++-- Tracking/include/Tracking/Reco/Vertexer.h | 2 +- .../include/Tracking/Sim/LdmxSpacePoint.h | 3 +- .../Tracking/Sim/SeedToTrackParamMaker.h | 3 +- .../Tracking/Sim/SeedToTrackParamMaker.ipp | 19 +++--- Tracking/include/Tracking/Sim/TrackingUtils.h | 2 - .../include/Tracking/dqm/TrackingRecoDQM.h | 11 ++-- Tracking/src/Tracking/Reco/CKFProcessor.cxx | 15 ++--- .../Tracking/Reco/CustomStatePropagator.cxx | 4 +- .../Tracking/Reco/DigitizationProcessor.cxx | 5 +- Tracking/src/Tracking/Reco/GSFProcessor.cxx | 4 +- .../src/Tracking/Reco/SeedFinderProcessor.cxx | 64 ++++++++++--------- .../src/Tracking/Reco/TruthSeedProcessor.cxx | 4 +- .../src/Tracking/Reco/VertexProcessor.cxx | 8 ++- Tracking/src/Tracking/Reco/Vertexer.cxx | 20 +++--- Tracking/src/Tracking/dqm/TrackingRecoDQM.cxx | 40 ++++++------ .../src/Tracking/geo/EcalTrackingGeometry.cxx | 3 +- 23 files changed, 140 insertions(+), 135 deletions(-) diff --git a/Tracking/include/Tracking/Event/Measurement.h b/Tracking/include/Tracking/Event/Measurement.h index 2c1f2181b..a3fb527e5 100644 --- a/Tracking/include/Tracking/Event/Measurement.h +++ b/Tracking/include/Tracking/Event/Measurement.h @@ -27,7 +27,7 @@ class Measurement { const float& cov_vv = 1.0); /// Default destructor - ~Measurement() = default; + virtual ~Measurement() = default; /** * Set the global position i.e. position of the measurement in the detector diff --git a/Tracking/include/Tracking/Event/TruthTrack.h b/Tracking/include/Tracking/Event/TruthTrack.h index 10737c8d8..d9f16969e 100644 --- a/Tracking/include/Tracking/Event/TruthTrack.h +++ b/Tracking/include/Tracking/Event/TruthTrack.h @@ -13,7 +13,7 @@ class TruthTrack { TruthTrack() = default; - ~TruthTrack() = default; + virtual ~TruthTrack() = default; /** * * Use the vertex position of the SimParticle to extract diff --git a/Tracking/include/Tracking/Reco/CKFProcessor.h b/Tracking/include/Tracking/Reco/CKFProcessor.h index f19b8b53c..0a691488c 100644 --- a/Tracking/include/Tracking/Reco/CKFProcessor.h +++ b/Tracking/include/Tracking/Reco/CKFProcessor.h @@ -201,7 +201,8 @@ class CKFProcessor final : public TrackingGeometryUser { std::string out_trk_collection_{"Tracks"}; // Mass for the propagator hypothesis in MeV - double mass_{0.511}; + // This is unused, should it be? FIXME + // double mass_{0.511}; // The seed track collection std::string seed_coll_name_{"seedTracks"}; diff --git a/Tracking/include/Tracking/Reco/CustomStatePropagator.h b/Tracking/include/Tracking/Reco/CustomStatePropagator.h index ed7ae7c54..fa11d3bb9 100644 --- a/Tracking/include/Tracking/Reco/CustomStatePropagator.h +++ b/Tracking/include/Tracking/Reco/CustomStatePropagator.h @@ -76,7 +76,7 @@ class CustomStatePropagator : public framework::Producer { std::shared_ptr histo_loc01; double state_nr{0.}; - int charge{0}; + int charge_{0}; double gen_x{0.}; double gen_y{0.}; double gen_z{0.}; diff --git a/Tracking/include/Tracking/Reco/GSFProcessor.h b/Tracking/include/Tracking/Reco/GSFProcessor.h index f6d00cad0..dcf9f1da1 100644 --- a/Tracking/include/Tracking/Reco/GSFProcessor.h +++ b/Tracking/include/Tracking/Reco/GSFProcessor.h @@ -109,7 +109,7 @@ class GSFProcessor final : public TrackingGeometryUser { GSFProcessor(const std::string &name, framework::Process &process); /// Destructor - ~GSFProcessor(); + virtual ~GSFProcessor() = default; /** * @@ -157,21 +157,18 @@ class GSFProcessor final : public TrackingGeometryUser { // ActsExamples::IndexSourceLink>; // If we want to dump the tracking geometry - bool dumpobj_{false}; - - int pionstates_{10}; - - int nevents_{0}; - + // bool dumpobj_{false}; + // int pionstates_{10}; + // int nevents_{0}; // Processing time counter - double processing_time_{0.}; + // double processing_time_{0.}; // time profiling std::map profiling_map_; // refitting of tracks - bool kf_refit_{false}; - bool gsf_refit_{false}; + // bool kf_refit_{false}; + // bool gsf_refit_{false}; bool debug_{false}; @@ -181,39 +178,39 @@ class GSFProcessor final : public TrackingGeometryUser { std::shared_ptr> normal_; // Constant BField - double bfield_{0}; + // double bfield_{0}; // Use constant bfield - bool const_b_field_{true}; + // bool const_b_field_{true}; // Remove stereo measurements - bool remove_stereo_{false}; + // bool remove_stereo_{false}; // Use 2d measurements instead of 1D - bool use1Dmeasurements_{true}; + // bool use1Dmeasurements_{true}; // Minimum number of hits on tracks - int min_hits_{7}; + // int min_hits_{7}; // The extrapolation surface - bool use_extrapolate_location_{true}; + // bool use_extrapolate_location_{true}; std::vector extrapolate_location_{0., 0., 0.}; - bool use_seed_perigee_{false}; + // bool use_seed_perigee_{false}; // The measurement collection to use for track reconstruction std::string measurement_collection_{"TaggerMeasurements"}; - double outlier_pval_{3.84}; + // double outlier_pval_{3.84}; // The output track collection std::string out_trk_collection_{"GSFTracks"}; // Select the hits using TrackID and pdg_id__ - int track_id_{-1}; - int pdg_id_{11}; + // int track_id_{-1}; + // int pdg_id_{11}; // Mass for the propagator hypothesis in MeV - double mass_{0.511}; + // double mass_{0.511}; // The seed track collection std::string seed_coll_name_{"seedTracks"}; @@ -238,7 +235,7 @@ class GSFProcessor final : public TrackingGeometryUser { std::string field_map_{""}; bool usePerigee_{false}; - bool usePlaneSurface_{false}; + // bool usePlaneSurface_{false}; // The Propagators std::unique_ptr propagator_; diff --git a/Tracking/include/Tracking/Reco/SeedFinderProcessor.h b/Tracking/include/Tracking/Reco/SeedFinderProcessor.h index cfd894788..fdb6e168a 100644 --- a/Tracking/include/Tracking/Reco/SeedFinderProcessor.h +++ b/Tracking/include/Tracking/Reco/SeedFinderProcessor.h @@ -145,9 +145,6 @@ class SeedFinderProcessor : public TrackingGeometryUser { std::vector strategies_{}; double bfield_{1.5}; - TFile* outputFile_; - TTree* outputTree_; - std::vector xhit_; std::vector yhit_; std::vector zhit_; @@ -172,7 +169,8 @@ class SeedFinderProcessor : public TrackingGeometryUser { // The measurements groups std::map> groups_map; - std::array groups_array; + // groups_array is unused, should it be? FIXME + // std::array groups_array; // Truth Matching tool std::shared_ptr truthMatchingTool_ = diff --git a/Tracking/include/Tracking/Reco/TrackExtrapolatorTool.h b/Tracking/include/Tracking/Reco/TrackExtrapolatorTool.h index 5057cc0e8..c305cad29 100644 --- a/Tracking/include/Tracking/Reco/TrackExtrapolatorTool.h +++ b/Tracking/include/Tracking/Reco/TrackExtrapolatorTool.h @@ -99,9 +99,9 @@ class TrackExtrapolatorTool { std::optional extrapolate( track_t track, const std::shared_ptr& target_surface) { // get first and last track state on surface - size_t nstates = track.nTrackStates(); - auto& tsc = track.container().trackStateContainer(); + // auto& tsc = track.container().trackStateContainer(); // auto ts_first = tsc.getTrackState(0); + // size_t nstates = track.nTrackStates(); // auto ts_last = tsc.getTrackState(nstates-1); auto outermost = *(track.trackStates().begin()); auto begin = track.trackStates().begin(); @@ -130,8 +130,8 @@ class TrackExtrapolatorTool { const auto& surface = ts.referenceSurface(); const auto& smoothed = ts.smoothed(); - bool hasSmoothed = ts.hasSmoothed(); - const auto& filtered = ts.filtered(); + // bool hasSmoothed = ts.hasSmoothed(); + // const auto& filtered = ts.filtered(); const auto& cov = ts.smoothedCovariance(); // std::cout<<"Surface::"<< @@ -156,8 +156,8 @@ class TrackExtrapolatorTool { // Now.. I'm taking whatever it is. I'm not checking here if it is a // measurement. - size_t nstates = track.nTrackStates(); - auto& tsc = track.container().trackStateContainer(); + // size_t nstates = track.nTrackStates(); + // auto& tsc = track.container().trackStateContainer(); auto begin = track.trackStates().begin(); auto ts_last = *begin; const auto& surface = (ts_last).referenceSurface(); diff --git a/Tracking/include/Tracking/Reco/Vertexer.h b/Tracking/include/Tracking/Reco/Vertexer.h index 45fcbafa2..a41aea039 100644 --- a/Tracking/include/Tracking/Reco/Vertexer.h +++ b/Tracking/include/Tracking/Reco/Vertexer.h @@ -77,7 +77,7 @@ class Vertexer : public framework::Producer { std::string trk_c_name_1{"TaggerTracks"}; std::string trk_c_name_2{"RecoilTracks"}; std::shared_ptr propagator_; - double processing_time_{0.}; + // double processing_time_{0.}; // Monitoring histograms diff --git a/Tracking/include/Tracking/Sim/LdmxSpacePoint.h b/Tracking/include/Tracking/Sim/LdmxSpacePoint.h index 153448604..a4bea901c 100644 --- a/Tracking/include/Tracking/Sim/LdmxSpacePoint.h +++ b/Tracking/include/Tracking/Sim/LdmxSpacePoint.h @@ -133,7 +133,8 @@ class LdmxSpacePoint { float m_varianceR; float m_varianceZ; int m_id; - int m_surfaceId; + // m_surfaceId is unused, should it be? FIXME + // int m_surfaceId; int m_layer; }; diff --git a/Tracking/include/Tracking/Sim/SeedToTrackParamMaker.h b/Tracking/include/Tracking/Sim/SeedToTrackParamMaker.h index 86cd3fa1c..b2dd3ee12 100644 --- a/Tracking/include/Tracking/Sim/SeedToTrackParamMaker.h +++ b/Tracking/include/Tracking/Sim/SeedToTrackParamMaker.h @@ -182,7 +182,8 @@ class SeedToTrackParamMaker { // coordinate (-1.*A/(2*B), 1./(2*B))) Acts::Vector3 transDirection(1., A, std::hypot(1, A) * invTanTheta); // Transform it back to the original frame - Acts::Vector3 direction = rotation * transDirection.normalized(); + [[maybe_unused]] Acts::Vector3 direction = + rotation * transDirection.normalized(); // Initialize the bound parameters vector Acts::BoundVector params = Acts::BoundVector::Zero(); diff --git a/Tracking/include/Tracking/Sim/SeedToTrackParamMaker.ipp b/Tracking/include/Tracking/Sim/SeedToTrackParamMaker.ipp index 1b452dae2..794bae2fe 100644 --- a/Tracking/include/Tracking/Sim/SeedToTrackParamMaker.ipp +++ b/Tracking/include/Tracking/Sim/SeedToTrackParamMaker.ipp @@ -27,8 +27,6 @@ bool SeedToTrackParamMaker::KarimakiFit( double Syr2 = 0., yr2m = 0.; // double Sw = 0.; // sum of weights - int n = sp.size(); - for (size_t i_sp = 0; i_sp < sp.size(); i_sp++) { double x = sp[i_sp]->x() - refPoint[0]; double y = sp[i_sp]->y() - refPoint[1]; @@ -204,7 +202,8 @@ bool SeedToTrackParamMaker::FitSeedAtlas( double y0 = sp[0]->y(); double z0 = sp[0]->z(); - double r0 = sqrt(x0 * x0 + y0 * y0); + // r0 is unused, should it be? FIXME + // double r0 = sqrt(x0 * x0 + y0 * y0); double x1 = sp[1]->x() - x0; double y1 = sp[1]->y() - y0; @@ -248,19 +247,17 @@ bool SeedToTrackParamMaker::FitSeedAtlas( data[5] = -C / (300. * bFieldZ); /// B in this computation is twice the usual B - double b_c = 1 / B; - double a_c = -1 * b_c * A; - double R = 1. / C; - + // double b_c = 1 / B; + // double a_c = -1 * b_c * A; + // double R = 1. / C; // std::cout<<"(a_c, b_c) = "<TrackStateAtSurface( + bool successAtTarget = trk_extrap_->TrackStateAtSurface( track, target_surface, tsAtTarget, ldmx::TrackStateType::AtTarget); - ldmx_log(debug) << "target extrapolation success??? " << success; - if (success) { + if (successAtTarget) { ldmx_log(debug) << "Successfully obtained TS at target"; ldmx_log(debug) << "Parameters At Target: \n" << tsAtTarget.params[0] << " " << tsAtTarget.params[1] @@ -565,11 +564,11 @@ void CKFProcessor::produce(framework::Event& event) { if (taggerTracking_) { ldmx_log(debug) << "Beam Origin Extrapolation"; ldmx::Track::TrackState tsAtBeamOrigin; - bool success = trk_extrap_->TrackStateAtSurface( + bool successAtBeam = trk_extrap_->TrackStateAtSurface( track, beamOrigin_surface, tsAtBeamOrigin, ldmx::TrackStateType::AtBeamOrigin); - if (success) { + if (successAtBeam) { trk.addTrackState(tsAtBeamOrigin); ldmx_log(debug) << "Successfully obtained TS at beam origin"; } @@ -579,10 +578,10 @@ void CKFProcessor::produce(framework::Event& event) { if (!taggerTracking_) { ldmx_log(debug) << "Ecal Extrapolation"; ldmx::Track::TrackState tsAtEcal; - success = trk_extrap_->TrackStateAtSurface(track, ecal_surface, tsAtEcal, - ldmx::TrackStateType::AtECAL); + bool successAtEcal = trk_extrap_->TrackStateAtSurface( + track, ecal_surface, tsAtEcal, ldmx::TrackStateType::AtECAL); - if (success) { + if (successAtEcal) { trk.addTrackState(tsAtEcal); ldmx_log(debug) << "Successfully obtained TS at Ecal"; ldmx_log(debug) << "Parameters At Ecal: \n" diff --git a/Tracking/src/Tracking/Reco/CustomStatePropagator.cxx b/Tracking/src/Tracking/Reco/CustomStatePropagator.cxx index 9f7a556e7..f1bab53bf 100644 --- a/Tracking/src/Tracking/Reco/CustomStatePropagator.cxx +++ b/Tracking/src/Tracking/Reco/CustomStatePropagator.cxx @@ -37,7 +37,7 @@ void CustomStatePropagator::onProcessStart() { 100, -50, 50, 100, -50, 50); outTree_->Branch("state_nr", &state_nr); - outTree_->Branch("charge", &charge); + outTree_->Branch("charge", &charge_); outTree_->Branch("gen_x", &gen_x); outTree_->Branch("gen_y", &gen_y); outTree_->Branch("gen_z", &gen_z); @@ -208,7 +208,7 @@ void CustomStatePropagator::fillTree( int state, int q, const Acts::Vector3 gen_pos, const Acts::Vector3 gen_mom, const Acts::BoundTrackParameters& endParams) { state_nr = state; - charge = q; + charge_ = q; gen_x = gen_pos(0); gen_y = gen_pos(1); gen_z = gen_pos(2); diff --git a/Tracking/src/Tracking/Reco/DigitizationProcessor.cxx b/Tracking/src/Tracking/Reco/DigitizationProcessor.cxx index 6854ea699..6dc1eac36 100644 --- a/Tracking/src/Tracking/Reco/DigitizationProcessor.cxx +++ b/Tracking/src/Tracking/Reco/DigitizationProcessor.cxx @@ -266,10 +266,11 @@ std::vector DigitizationProcessor::digitizeHits( sigma_v_ * sigma_v_); // transform to global - auto global_pos{hit_surface->localToGlobal( + auto transf_global_pos{hit_surface->localToGlobal( geometry_context(), local_pos, dummy_momentum)}; measurement.setGlobalPosition(measurement.getGlobalPosition()[0], - global_pos(1), global_pos(2)); + transf_global_pos(1), + transf_global_pos(2)); } // do smearing measurement.setLocalPosition(local_pos(0), local_pos(1)); diff --git a/Tracking/src/Tracking/Reco/GSFProcessor.cxx b/Tracking/src/Tracking/Reco/GSFProcessor.cxx index d02cfc730..3197af489 100644 --- a/Tracking/src/Tracking/Reco/GSFProcessor.cxx +++ b/Tracking/src/Tracking/Reco/GSFProcessor.cxx @@ -10,12 +10,10 @@ namespace reco { GSFProcessor::GSFProcessor(const std::string& name, framework::Process& process) : TrackingGeometryUser(name, process) {} -GSFProcessor::~GSFProcessor() {} - void GSFProcessor::onNewRun(const ldmx::RunHeader& rh) { // Custom transformation of the interpolated bfield map bool debugTransform = false; - auto transformPos = [this, debugTransform](const Acts::Vector3& pos) { + auto transformPos = [debugTransform](const Acts::Vector3& pos) { Acts::Vector3 rot_pos; rot_pos(0) = pos(1); rot_pos(1) = pos(2); diff --git a/Tracking/src/Tracking/Reco/SeedFinderProcessor.cxx b/Tracking/src/Tracking/Reco/SeedFinderProcessor.cxx index 55561f6ce..60b23db0a 100644 --- a/Tracking/src/Tracking/Reco/SeedFinderProcessor.cxx +++ b/Tracking/src/Tracking/Reco/SeedFinderProcessor.cxx @@ -85,7 +85,8 @@ void SeedFinderProcessor::configure(framework::config::Parameters& parameters) { } void SeedFinderProcessor::produce(framework::Event& event) { - const auto& tg{geometry()}; + // tg is unused, should it be? FIXME + // const auto& tg{geometry()}; auto start = std::chrono::high_resolution_clock::now(); ldmx::Tracks seed_tracks; @@ -215,7 +216,9 @@ ldmx::Track SeedFinderProcessor::SeedTracker( // Fit a straight line in the non-bending plane and a parabola in the bending // plane - const int N = vmeas.size(); + // N is unused, should it be? FIXME + // also it should be not a one-letter variable + // const int N = vmeas.size(); // Each measurement is treated as a 3D point, where the v direction is in the // center of the strip with sigma equal to the length of the strip / sqrt(12). @@ -276,8 +279,6 @@ ldmx::Track SeedFinderProcessor::SeedTracker( W_i(1, 1) = 1. / (vError * vError); Acts::Vector2 Yprime_i = loc + offset - xoffset; - - Acts::Vector2 yi = W_i * Yprime_i; Y += (A_i.transpose()) * W_i * Yprime_i; Acts::ActsMatrix<2, 5> WA_i = (W_i * A_i); @@ -395,26 +396,28 @@ ldmx::Track SeedFinderProcessor::SeedTracker( void SeedFinderProcessor::LineParabolaToHelix( const Acts::ActsVector<5> parameters, Acts::ActsVector<5>& helix_parameters, Acts::Vector3 ref) { - double R = 0.5 / abs(parameters(2)); - double xc = R * parameters(1); - double p2 = 0.5 * parameters(1) * parameters(1); - double factor = parameters(2) < 0 ? -1 : 1; - double yc = - parameters(0) + - factor * - (R * (1 - p2)); //+ or minus solution. I chose beta0 - R ( ... ), - // because the curvature is negative for electrons. - // The sign need to be chosen by the sign of B(2) - double theta0 = atan2(yc, xc); - double k = parameters(2) < 0 ? (-1. / R) : 1. / R; - double d0 = R - xc / cos(theta0); - double phi0 = theta0 + 1.57079632679; - double tanL = parameters(4) * cos(theta0); - double theta = 1.57079632679 - atan(tanL); - double z0 = parameters(3) + (d0 * tanL * tan(theta0)); - double qOp = factor / (0.3 * bfield_ * R * 0.001); - - // std::cout< meas_for_seeds; - meas_for_seeds.reserve(5); + // These seemd like code dupplication and is not needed FIXME + // std::vector meas_for_seeds; + // meas_for_seeds.reserve(5); std::map>::iterator groups_iter = groups_map.begin(); - std::vector::iterator meas_iter; + // meas_iter is unused, should it be? FIXME + // std::vector::iterator meas_iter; // Vector of iterators @@ -575,7 +580,8 @@ void SeedFinderProcessor::FindSeedsFromMap(ldmx::Tracks& seeds, // A seed is rejected if it is found incompatible with all the target // extrapolations - bool tgt_compatible = false; + // This is set but unused, FIXME + // bool tgt_compatible = false; for (auto tgt_pseudomeas : pmeas) { // The d0/z0 are in a frame with the same orientation of the target // surface @@ -586,7 +592,7 @@ void SeedFinderProcessor::FindSeedsFromMap(ldmx::Tracks& seeds, if (abs(delta_loc0) < loc0cut_ && abs(delta_loc1) < loc1cut_) { // found at least 1 compatible target location - tgt_compatible = true; + // tgt_compatible = true; break; } } diff --git a/Tracking/src/Tracking/Reco/TruthSeedProcessor.cxx b/Tracking/src/Tracking/Reco/TruthSeedProcessor.cxx index abd758679..d0b788d41 100644 --- a/Tracking/src/Tracking/Reco/TruthSeedProcessor.cxx +++ b/Tracking/src/Tracking/Reco/TruthSeedProcessor.cxx @@ -107,7 +107,6 @@ void TruthSeedProcessor::createTruthTrack( // Eigen::Matrix; Acts::Vector3 pos{pos_vec[0], pos_vec[1], pos_vec[2]}; Acts::Vector3 mom{p_vec[0], p_vec[1], p_vec[2]}; - double time{0.}; // Rotate the position and momentum into the ACTS frame. pos = tracking::sim::utils::Ldmx2Acts(pos); @@ -694,7 +693,8 @@ void TruthSeedProcessor::produce(framework::Event& event) { // Only take the first entry of the vector: it should be the scoring plane // hit with the highest momentum. const ldmx::SimTrackerHit& hit = scoring_hits.at(element.second.at(0)); - const ldmx::SimParticle& phit = particleMap[hit.getTrackID()]; + [[maybe_unused]] const ldmx::SimParticle& phit = + particleMap[hit.getTrackID()]; ldmx::SimTrackerHit ecal_hit; bool foundEcalHit = false; diff --git a/Tracking/src/Tracking/Reco/VertexProcessor.cxx b/Tracking/src/Tracking/Reco/VertexProcessor.cxx index 8c1d6a081..265e4e59a 100644 --- a/Tracking/src/Tracking/Reco/VertexProcessor.cxx +++ b/Tracking/src/Tracking/Reco/VertexProcessor.cxx @@ -21,6 +21,8 @@ void VertexProcessor::onProcessStart() { h_m_truthFilter_ = new TH1F("m_filter", "m", 100, 0., 1.); h_m_truth_ = new TH1F("m_truth", "m_truth", 100, 0., 1.); + /* + * this is unused, should it be? FIXME auto localToGlobalBin_xyz = [](std::array bins, std::array sizes) { return (bins[0] * (sizes[1] * sizes[2]) + bins[1] * sizes[2] + @@ -28,6 +30,7 @@ void VertexProcessor::onProcessStart() { // return (bins[1] * (sizes[2] * sizes[0]) + bins[2] * sizes[0] + bins[0]); // //zxy }; + */ // Setup a interpolated bfield map sp_interpolated_bField_ = @@ -93,6 +96,7 @@ void VertexProcessor::produce(framework::Event &event) { // TODO:: The perigee surface should be common between all tracks. // So should only be created once in principle. + // There should be no perigeeSurface2 std::shared_ptr perigeeSurface = Acts::Surface::makeShared(Acts::Vector3( @@ -135,7 +139,7 @@ void VertexProcessor::produce(framework::Event &event) { if (seeds.size() == 2) { for (int iSeed = 0; iSeed < seeds.size(); iSeed++) { - std::shared_ptr perigeeSurface = + std::shared_ptr perigeeSurface2 = Acts::Surface::makeShared(Acts::Vector3( seeds.at(iSeed).getPerigeeX(), seeds.at(iSeed).getPerigeeY(), seeds.at(iSeed).getPerigeeZ())); @@ -149,7 +153,7 @@ void VertexProcessor::produce(framework::Event &event) { tracking::sim::utils::unpackCov(seeds.at(iSeed).getPerigeeCov()); auto boundSeedParams = Acts::BoundTrackParameters( - perigeeSurface, paramVec, std::move(covMat)); + perigeeSurface2, paramVec, std::move(covMat)); TLorentzVector pion4v; pion4v.SetXYZM(boundSeedParams.momentum()(0), diff --git a/Tracking/src/Tracking/Reco/Vertexer.cxx b/Tracking/src/Tracking/Reco/Vertexer.cxx index 3a3f6f4d9..df909d59e 100644 --- a/Tracking/src/Tracking/Reco/Vertexer.cxx +++ b/Tracking/src/Tracking/Reco/Vertexer.cxx @@ -47,6 +47,8 @@ void Vertexer::onProcessStart() { gctx_ = Acts::GeometryContext(); bctx_ = Acts::MagneticFieldContext(); + /* + * This is unused now, should it be? auto localToGlobalBin_xyz = [](std::array bins, std::array sizes) { return (bins[0] * (sizes[1] * sizes[2]) + bins[1] * sizes[2] + @@ -54,6 +56,7 @@ void Vertexer::onProcessStart() { // return (bins[1] * (sizes[2] * sizes[0]) + bins[2] * sizes[0] + bins[0]); // //zxy }; + */ sp_interpolated_bField_ = std::make_shared(loadDefaultBField( @@ -66,13 +69,11 @@ void Vertexer::onProcessStart() { std::cout << "Check if nullptr::" << sp_interpolated_bField_.get() << std::endl; - auto&& stepper_const = Acts::EigenStepper<>{bField_}; - auto&& stepper = Acts::EigenStepper<>{sp_interpolated_bField_}; - // Set up propagator with void navigator - + // auto&& stepper = Acts::EigenStepper<>{sp_interpolated_bField_}; // propagator_ = std::make_shared(stepper); + auto&& stepper_const = Acts::EigenStepper<>{bField_}; propagator_ = std::make_shared(stepper_const); } @@ -88,7 +89,7 @@ void Vertexer::configure(framework::config::Parameters& parameters) { void Vertexer::produce(framework::Event& event) { nevents_++; - auto start = std::chrono::high_resolution_clock::now(); + // auto start = std::chrono::high_resolution_clock::now(); // Track linearizer in the proximity of the vertex location using Linearizer = Acts::HelicalTrackLinearizer; @@ -227,11 +228,12 @@ void Vertexer::TaggerRecoilMonitoring( ldmx::Track t_trk = tagger_tracks.at(0); ldmx::Track r_trk = recoil_tracks.at(0); - double t_d0, r_d0; - double t_phi, r_phi; double t_p, r_p; - double t_theta, r_theta; - double t_z0, r_z0; + // these are unsed, should they be? FIXME + // double t_d0, r_d0; + // double tt_p_phi, r_phi; + // double t_theta, r_theta; + // double t_z0, r_z0; t_p = t_trk.q() / t_trk.getQoP(); r_p = r_trk.q() / r_trk.getQoP(); diff --git a/Tracking/src/Tracking/dqm/TrackingRecoDQM.cxx b/Tracking/src/Tracking/dqm/TrackingRecoDQM.cxx index d4d0c9a38..8560efdaa 100644 --- a/Tracking/src/Tracking/dqm/TrackingRecoDQM.cxx +++ b/Tracking/src/Tracking/dqm/TrackingRecoDQM.cxx @@ -62,9 +62,9 @@ void TrackingRecoDQM::analyze(const framework::Event& event) { ldmx_log(debug) << "Do truth comparison::" << doTruthComparison << std::endl; if (doTruthComparison) { - sortTracks(tracks, uniqueTracks, duplicateTracks, fakeTracks); + sortTracks(tracks, uniqueTracks_, duplicateTracks_, fakeTracks_); } else { - uniqueTracks = tracks; + uniqueTracks_ = tracks; } ldmx_log(debug) << "Filling histograms " << std::endl; @@ -73,12 +73,12 @@ void TrackingRecoDQM::analyze(const framework::Event& event) { histograms_.fill(title_ + "N_tracks", tracks.size()); ldmx_log(debug) << "Track Monitoring on Unique Tracks" << std::endl; - TrackMonitoring(uniqueTracks, title_, true, true); + TrackMonitoring(uniqueTracks_, title_, true, true); ldmx_log(debug) << "Track Monitoring on duplicates and fakes" << std::endl; // Fakes and duplicates - TrackMonitoring(duplicateTracks, title_ + "dup_", false, false); - TrackMonitoring(fakeTracks, title_ + "fake_", false, false); + TrackMonitoring(duplicateTracks_, title_ + "dup_", false, false); + TrackMonitoring(fakeTracks_, title_ + "fake_", false, false); // Track Extrapolation to Ecal Monitoring @@ -104,9 +104,9 @@ void TrackingRecoDQM::analyze(const framework::Event& event) { // Tagger Recoil Matching // Clear the vectors - uniqueTracks.clear(); - duplicateTracks.clear(); - fakeTracks.clear(); + uniqueTracks_.clear(); + duplicateTracks_.clear(); + fakeTracks_.clear(); } void TrackingRecoDQM::onProcessEnd() { @@ -478,31 +478,31 @@ void TrackingRecoDQM::TrackStateMonitoring(const ldmx::Tracks& tracks, ldmx_log(debug) << "Unpacking covariance matrix" << std::endl; Acts::BoundSymMatrix cov = tracking::sim::utils::unpackCov(TargetState.cov); - double sigmaloc0 = sqrt( + [[maybe_unused]] double sigmaloc0 = sqrt( cov(Acts::BoundIndices::eBoundLoc0, Acts::BoundIndices::eBoundLoc0)); - double sigmaloc1 = sqrt( + [[maybe_unused]] double sigmaloc1 = sqrt( cov(Acts::BoundIndices::eBoundLoc1, Acts::BoundIndices::eBoundLoc1)); - double sigmaphi = + [[maybe_unused]] double sigmaphi = sqrt(cov(Acts::BoundIndices::eBoundPhi, Acts::BoundIndices::eBoundPhi)); - double sigmatheta = sqrt( + [[maybe_unused]] double sigmatheta = sqrt( cov(Acts::BoundIndices::eBoundTheta, Acts::BoundIndices::eBoundTheta)); - double sigmaqop = sqrt(cov(Acts::BoundIndices::eBoundQOverP, - Acts::BoundIndices::eBoundQOverP)); + [[maybe_unused]] double sigmaqop = sqrt(cov( + Acts::BoundIndices::eBoundQOverP, Acts::BoundIndices::eBoundQOverP)); double trk_qop = track.getQoP(); double trk_p = 1. / abs(trk_qop); double track_state_loc0 = TargetState.params[0]; double track_state_loc1 = TargetState.params[1]; - double track_state_phi = TargetState.params[2]; - double track_state_theta = TargetState.params[3]; - double track_state_p = TargetState.params[4]; + [[maybe_unused]] double track_state_phi = TargetState.params[2]; + [[maybe_unused]] double track_state_theta = TargetState.params[3]; + [[maybe_unused]] double track_state_p = TargetState.params[4]; double truth_state_loc0 = truthTargetState.params[0]; double truth_state_loc1 = truthTargetState.params[1]; - double truth_state_phi = truthTargetState.params[2]; - double truth_state_theta = truthTargetState.params[3]; - double truth_state_p = truthTargetState.params[4]; + [[maybe_unused]] double truth_state_phi = truthTargetState.params[2]; + [[maybe_unused]] double truth_state_theta = truthTargetState.params[3]; + [[maybe_unused]] double truth_state_p = truthTargetState.params[4]; // Check that the track state is filled if (TargetState.params.size() < 5) continue; diff --git a/Tracking/src/Tracking/geo/EcalTrackingGeometry.cxx b/Tracking/src/Tracking/geo/EcalTrackingGeometry.cxx index e656e9064..96ca3cd11 100644 --- a/Tracking/src/Tracking/geo/EcalTrackingGeometry.cxx +++ b/Tracking/src/Tracking/geo/EcalTrackingGeometry.cxx @@ -47,7 +47,8 @@ EcalTrackingGeometry::EcalTrackingGeometry(std::string gdmlfile, std::cout << fWorldPhysVol->GetTranslation() << std::endl; } // Get the logical volume from the world - G4LogicalVolume* fWorldLogicalVol = fWorldPhysVol->GetLogicalVolume(); + // fWorldLogicalVol is unused, should it be? FIXME + // G4LogicalVolume* fWorldLogicalVol = fWorldPhysVol->GetLogicalVolume(); if (debug) std::cout << "Loop on world daughters" << std::endl; From 5f30a6bb82cf6f06bcbb0ab1e22d3a74eb8c9cc8 Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Tue, 10 Sep 2024 13:10:12 -0700 Subject: [PATCH 09/14] Static analysis improvement for TrigScint and Trigger --- TrigScint/include/TrigScint/QIEAnalyzer.h | 1 - .../include/TrigScint/QualityFlagAnalyzer.h | 1 - .../TrigScint/TestBeamClusterAnalyzer.h | 2 +- .../include/TrigScint/TestBeamHitAnalyzer.h | 1 - .../include/TrigScint/TestBeamHitProducer.h | 3 +- TrigScint/src/TrigScint/QIEAnalyzer.cxx | 2 - TrigScint/src/TrigScint/QIEDecoder.cxx | 10 ++-- .../src/TrigScint/QualityFlagAnalyzer.cxx | 1 - TrigScint/src/TrigScint/SimQIE.cxx | 5 +- .../src/TrigScint/TestBeamHitAnalyzer.cxx | 1 - .../src/TrigScint/TestBeamHitProducer.cxx | 4 +- .../src/TrigScint/TrigScintDigiProducer.cxx | 9 ++-- Trigger/Algo/include/Trigger/DumpFileWriter.h | 7 --- Trigger/Algo/include/Trigger/EcalTPSelector.h | 11 ---- .../include/Trigger/IdealClusterBuilder.h | 1 + Trigger/Algo/include/Trigger/NtupleWriter.h | 1 - .../include/Trigger/TrigEcalClusterProducer.h | 11 ---- .../Algo/include/Trigger/TrigEcalEnergySum.h | 11 ---- .../include/Trigger/TrigElectronProducer.h | 9 ---- .../Algo/include/Trigger/TrigHcalEnergySum.h | 11 ---- Trigger/Algo/src/Trigger/DumpFileWriter.cxx | 18 ------- Trigger/Algo/src/Trigger/EcalTPSelector.cxx | 37 ++------------ .../Algo/src/Trigger/Event/TrigEnergySum.cxx | 38 +++++++------- .../Algo/src/Trigger/IdealClusterBuilder.cxx | 23 ++++----- Trigger/Algo/src/Trigger/NtupleWriter.cxx | 4 +- .../src/Trigger/TrigEcalClusterProducer.cxx | 29 +---------- .../Algo/src/Trigger/TrigEcalEnergySum.cxx | 37 +------------- .../Algo/src/Trigger/TrigElectronProducer.cxx | 14 +---- .../Algo/src/Trigger/TrigHcalEnergySum.cxx | 51 +++++-------------- 29 files changed, 67 insertions(+), 286 deletions(-) diff --git a/TrigScint/include/TrigScint/QIEAnalyzer.h b/TrigScint/include/TrigScint/QIEAnalyzer.h index 9cd1cbe9d..f10960d13 100644 --- a/TrigScint/include/TrigScint/QIEAnalyzer.h +++ b/TrigScint/include/TrigScint/QIEAnalyzer.h @@ -47,7 +47,6 @@ class QIEAnalyzer : public framework::Analyzer { int startSample_{0}; // plotting stuff - int evNb; int nEv{200}; int nChannels{16}; // int nTrkMax{100}; diff --git a/TrigScint/include/TrigScint/QualityFlagAnalyzer.h b/TrigScint/include/TrigScint/QualityFlagAnalyzer.h index 60eaa4200..c4d0b1357 100644 --- a/TrigScint/include/TrigScint/QualityFlagAnalyzer.h +++ b/TrigScint/include/TrigScint/QualityFlagAnalyzer.h @@ -48,7 +48,6 @@ class QualityFlagAnalyzer : public framework::Analyzer { int startSample_{0}; // plotting stuff - int evNb; const int nEv{200}; static constexpr int nChannels{16}; const int nFlags{6}; diff --git a/TrigScint/include/TrigScint/TestBeamClusterAnalyzer.h b/TrigScint/include/TrigScint/TestBeamClusterAnalyzer.h index 77274ac23..ee8e807bd 100644 --- a/TrigScint/include/TrigScint/TestBeamClusterAnalyzer.h +++ b/TrigScint/include/TrigScint/TestBeamClusterAnalyzer.h @@ -49,7 +49,7 @@ class TestBeamClusterAnalyzer : public framework::Analyzer { TH2F* hN2N1; TH1F* hNClusters; TH1F* hNHits; - TH1F* hNhitsInClusters; + // TH1F* hNhitsInClusters; TH1F* hPEinHits[16]; TH1F* hPEinClusters[16]; TH1F* hDeltaCentroids; diff --git a/TrigScint/include/TrigScint/TestBeamHitAnalyzer.h b/TrigScint/include/TrigScint/TestBeamHitAnalyzer.h index 3d6895ec6..9c704d94f 100644 --- a/TrigScint/include/TrigScint/TestBeamHitAnalyzer.h +++ b/TrigScint/include/TrigScint/TestBeamHitAnalyzer.h @@ -44,7 +44,6 @@ class TestBeamHitAnalyzer : public framework::Analyzer { int startSample_{0}; // plotting stuff - int evNb; int nEv{200}; int nChannels{16}; // int nTrkMax{100}; diff --git a/TrigScint/include/TrigScint/TestBeamHitProducer.h b/TrigScint/include/TrigScint/TestBeamHitProducer.h index 623c00632..612ae759b 100644 --- a/TrigScint/include/TrigScint/TestBeamHitProducer.h +++ b/TrigScint/include/TrigScint/TestBeamHitProducer.h @@ -32,7 +32,7 @@ class TestBeamHitProducer : public framework::Producer { public: TestBeamHitProducer(const std::string& name, framework::Process& process); - ~TestBeamHitProducer(); + virtual ~TestBeamHitProducer() = default; /** * Callback for the processor to configure itself from the given set @@ -46,7 +46,6 @@ class TestBeamHitProducer : public framework::Producer { private: /// Set the local verbosity level. - bool verbose_{false}; /// Name of the input collection containing the event readout samples std::string inputCol_; diff --git a/TrigScint/src/TrigScint/QIEAnalyzer.cxx b/TrigScint/src/TrigScint/QIEAnalyzer.cxx index 3f72f6aca..aa56a434c 100644 --- a/TrigScint/src/TrigScint/QIEAnalyzer.cxx +++ b/TrigScint/src/TrigScint/QIEAnalyzer.cxx @@ -199,8 +199,6 @@ void QIEAnalyzer::onProcessStart() { new TH2F("hTDCfireChanvsEvent", ";channel with TDC < 63;event number", nChannels, -0.5, nChannels - 0.5, nEv, 0, nEv); - evNb = 0; - ldmx_log(debug) << "done setting up histograms"; return; diff --git a/TrigScint/src/TrigScint/QIEDecoder.cxx b/TrigScint/src/TrigScint/QIEDecoder.cxx index 0c5bb96be..7e3f601f0 100644 --- a/TrigScint/src/TrigScint/QIEDecoder.cxx +++ b/TrigScint/src/TrigScint/QIEDecoder.cxx @@ -169,10 +169,12 @@ void QIEDecoder::produce(framework::Event &event) { mask8::m)}; bool isCIDunsync{static_cast((flags >> QIEStream::CID_UNSYNC_POS) & mask8::m)}; - bool isCRC1malformed{static_cast((flags >> QIEStream::CRC1_ERR_POS) & - mask8::m)}; - bool isCRC0malformed{static_cast((flags >> QIEStream::CRC0_ERR_POS) & - mask8::m)}; + // These are unused, should they be? FIXME + // bool isCRC1malformed{static_cast((flags >> QIEStream::CRC1_ERR_POS) & + // mask8::m)}; + // bool isCRC0malformed{static_cast((flags >> QIEStream::CRC0_ERR_POS) & + // mask8::m)}; + // checksum // really, this is just empty for now. // TODO: implement a checksum set/get diff --git a/TrigScint/src/TrigScint/QualityFlagAnalyzer.cxx b/TrigScint/src/TrigScint/QualityFlagAnalyzer.cxx index 05291b7cf..8b98e4fb2 100644 --- a/TrigScint/src/TrigScint/QualityFlagAnalyzer.cxx +++ b/TrigScint/src/TrigScint/QualityFlagAnalyzer.cxx @@ -200,7 +200,6 @@ void QualityFlagAnalyzer::onProcessStart() { new TH2F("hTDCfireChanvsEvent", ";channel with TDC < 63;event number", nChannels, -0.5, nChannels - 0.5, nEv, 0, nEv); - evNb = 0; peFillNb = 0; ldmx_log(debug) << "done setting up histograms"; diff --git a/TrigScint/src/TrigScint/SimQIE.cxx b/TrigScint/src/TrigScint/SimQIE.cxx index e3b77901a..f7e80c6fe 100644 --- a/TrigScint/src/TrigScint/SimQIE.cxx +++ b/TrigScint/src/TrigScint/SimQIE.cxx @@ -33,7 +33,6 @@ int SimQIE::Q2ADC(float Charge) { if (qq <= edges_[0]) return 0; if (qq >= edges_[16]) return 255; - int ID = 8; int a = 0; int b = 16; @@ -62,7 +61,8 @@ float SimQIE::ADC2Q(int ADC) { for (int i = 1; i < 4; i++) { // to get the subrange if (v1 > nbins_[i]) ss++; } - int cc = 64 * rr + nbins_[ss]; + // cc is unused, should it be? FIXME + // int cc = 64 * rr + nbins_[ss]; float temp = edges_[4 * rr + ss] + (v1 - nbins_[ss]) * sense_[4 * rr + ss] + sense_[4 * rr + ss] / 2; return (temp / gain_); @@ -73,7 +73,6 @@ float SimQIE::QErr(float Q) { if (Q <= edges_[0]) return 0; if (Q >= edges_[16]) return 0; - int ID = 8; int a = 0; int b = 16; while (b - a != 1) { diff --git a/TrigScint/src/TrigScint/TestBeamHitAnalyzer.cxx b/TrigScint/src/TrigScint/TestBeamHitAnalyzer.cxx index 6006266c1..52c3309dd 100644 --- a/TrigScint/src/TrigScint/TestBeamHitAnalyzer.cxx +++ b/TrigScint/src/TrigScint/TestBeamHitAnalyzer.cxx @@ -138,7 +138,6 @@ void TestBeamHitAnalyzer::onProcessStart() { nEv + 0.5, nChannels, -0.5, nChannels - 0.5); fillNb = 0; - evNb = 0; return; } diff --git a/TrigScint/src/TrigScint/TestBeamHitProducer.cxx b/TrigScint/src/TrigScint/TestBeamHitProducer.cxx index e46b1d5c6..7ebe9ed25 100644 --- a/TrigScint/src/TrigScint/TestBeamHitProducer.cxx +++ b/TrigScint/src/TrigScint/TestBeamHitProducer.cxx @@ -12,7 +12,6 @@ namespace trigscint { TestBeamHitProducer::TestBeamHitProducer(const std::string& name, framework::Process& process) : Producer(name, process) {} -TestBeamHitProducer::~TestBeamHitProducer() {} void TestBeamHitProducer::configure(framework::config::Parameters& parameters) { inputCol_ = parameters.getParameter("inputCollection"); @@ -95,7 +94,8 @@ void TestBeamHitProducer::produce(framework::Event& event) { event.getCollection(inputCol_, inputPassName_)}; int evNb = event.getEventNumber(); - int nChan = channels.size(); + // it is unused, should it be? FIXME + // int nChan = channels.size(); std::vector hits; for (auto chan : channels) { trigscint::TestBeamHit hit; diff --git a/TrigScint/src/TrigScint/TrigScintDigiProducer.cxx b/TrigScint/src/TrigScint/TrigScintDigiProducer.cxx index 757f62d95..e8600a983 100644 --- a/TrigScint/src/TrigScint/TrigScintDigiProducer.cxx +++ b/TrigScint/src/TrigScint/TrigScintDigiProducer.cxx @@ -78,8 +78,6 @@ void TrigScintDigiProducer::produce(framework::Event &event) { std::cout << id << std::endl; } - unsigned int detIDRaw = id.raw(); - // check if hits is from beam electron and, if so, add to beamFrac for (int i = 0; i < simHit.getNumberOfContribs(); i++) { auto contrib = simHit.getContrib(i); @@ -129,7 +127,6 @@ void TrigScintDigiProducer::produce(framework::Event &event) { std::vector trigScintHits; // loop over detIDs and simulate number of PEs - int ihit = 0; for (std::map::iterator it = Edep.begin(); it != Edep.end(); ++it) { ldmx::TrigScintID id(it->first); @@ -175,9 +172,9 @@ void TrigScintDigiProducer::produce(framework::Event &event) { // ------------------------------- Noise simulation -----------------------// // ------------------------------------------------------------------------// - int numEmptyCells = stripsPerArray_ - - numRecHits; // only simulating for single array until - // all arrays are merged into one collection + // only simulating for single array until + // all arrays are merged into one collection + int numEmptyCells = stripsPerArray_ - numRecHits; std::vector noiseHits_PE = noiseGenerator_->generateNoiseHits(numEmptyCells); diff --git a/Trigger/Algo/include/Trigger/DumpFileWriter.h b/Trigger/Algo/include/Trigger/DumpFileWriter.h index 5b5636148..026600ce2 100644 --- a/Trigger/Algo/include/Trigger/DumpFileWriter.h +++ b/Trigger/Algo/include/Trigger/DumpFileWriter.h @@ -33,10 +33,6 @@ class DumpFileWriter : public framework::Analyzer { virtual void analyze(const framework::Event& event); - virtual void onFileOpen(); - - virtual void onFileClose(); - virtual void onProcessStart(); virtual void onProcessEnd(); @@ -44,9 +40,6 @@ class DumpFileWriter : public framework::Analyzer { typedef ap_ufixed<16, 14> e_t; // [MeV] (Up to at least 8 GeV) private: - // specific verbosity - int verbose_{0}; - // From: // Tools/python/HgcrocEmulator.py // ECal/python/digi.py diff --git a/Trigger/Algo/include/Trigger/EcalTPSelector.h b/Trigger/Algo/include/Trigger/EcalTPSelector.h index 4d55bad27..d25ffbb7b 100644 --- a/Trigger/Algo/include/Trigger/EcalTPSelector.h +++ b/Trigger/Algo/include/Trigger/EcalTPSelector.h @@ -34,23 +34,12 @@ class EcalTPSelector : public framework::Producer { virtual void produce(framework::Event& event); - virtual void onFileOpen(); - - virtual void onFileClose(); - - virtual void onProcessStart(); - - virtual void onProcessEnd(); - // helpers void decodeTP(ldmx::HgcrocTrigDigi tp, double& x, double& y, double& z, double& e); /* double primitiveToEnergy(int tp, int layer); */ private: - // specific verbosity of this producer - int verbose_{0}; - // name of collection for EcalTPs to be passed as input std::string tpCollName_; // name of output collection diff --git a/Trigger/Algo/include/Trigger/IdealClusterBuilder.h b/Trigger/Algo/include/Trigger/IdealClusterBuilder.h index 9e2f73e9d..779f55ac6 100644 --- a/Trigger/Algo/include/Trigger/IdealClusterBuilder.h +++ b/Trigger/Algo/include/Trigger/IdealClusterBuilder.h @@ -141,6 +141,7 @@ class Cluster { class IdealClusterBuilder { public: + virtual ~IdealClusterBuilder() = default; std::vector all_hits{}; std::vector all_clusters{}; ClusterGeometry* g; diff --git a/Trigger/Algo/include/Trigger/NtupleWriter.h b/Trigger/Algo/include/Trigger/NtupleWriter.h index 09c71a09a..b0a9d093d 100644 --- a/Trigger/Algo/include/Trigger/NtupleWriter.h +++ b/Trigger/Algo/include/Trigger/NtupleWriter.h @@ -26,7 +26,6 @@ class NtupleWriter : public framework::Producer { std::string tag_{"Events"}; std::string outPath_{"./ntuple.root"}; bool writeTruth_{true}; - bool writeCluster_{true}; bool writeEle_{true}; bool writeEcalSums_{true}; bool writeHcalSums_{true}; diff --git a/Trigger/Algo/include/Trigger/TrigEcalClusterProducer.h b/Trigger/Algo/include/Trigger/TrigEcalClusterProducer.h index 5c1673ae3..e9ab468bb 100644 --- a/Trigger/Algo/include/Trigger/TrigEcalClusterProducer.h +++ b/Trigger/Algo/include/Trigger/TrigEcalClusterProducer.h @@ -31,18 +31,7 @@ class TrigEcalClusterProducer : public framework::Producer { virtual void produce(framework::Event& event); - virtual void onFileOpen(); - - virtual void onFileClose(); - - virtual void onProcessStart(); - - virtual void onProcessEnd(); - private: - // specific verbosity of this producer - int verbose_{0}; - // name of collection for trigHits to be passed as input std::string hitCollName_; // name of collection for trigCluster to be output diff --git a/Trigger/Algo/include/Trigger/TrigEcalEnergySum.h b/Trigger/Algo/include/Trigger/TrigEcalEnergySum.h index 5943eedba..929fd9495 100644 --- a/Trigger/Algo/include/Trigger/TrigEcalEnergySum.h +++ b/Trigger/Algo/include/Trigger/TrigEcalEnergySum.h @@ -33,20 +33,9 @@ class TrigEcalEnergySum : public framework::Producer { virtual void produce(framework::Event& event); - virtual void onFileOpen(); - - virtual void onFileClose(); - - virtual void onProcessStart(); - - virtual void onProcessEnd(); - typedef ap_ufixed<16, 14> e_t; // [MeV] (Up to at least 8 GeV) private: - // specific verbosity of this producer - int verbose_{0}; - // name of collection for trigHits to be passed as input std::string hitCollName_; diff --git a/Trigger/Algo/include/Trigger/TrigElectronProducer.h b/Trigger/Algo/include/Trigger/TrigElectronProducer.h index 55d6bad3f..6aad9bc45 100644 --- a/Trigger/Algo/include/Trigger/TrigElectronProducer.h +++ b/Trigger/Algo/include/Trigger/TrigElectronProducer.h @@ -33,10 +33,6 @@ class TrigElectronProducer : public framework::Producer { virtual void produce(framework::Event& event); - virtual void onFileOpen(); - - virtual void onFileClose(); - virtual void onProcessStart(); virtual void onProcessEnd(); @@ -47,11 +43,6 @@ class TrigElectronProducer : public framework::Producer { float getPy(float e, float d) { return getP(false, e, d); } private: - // specific verbosity of this producer - int verbose_{0}; - - bool useTargetScoringPlane_{1}; - // name of collection for target scoring plane inputs std::string spCollName_; // name of collection for calo cluster inputs diff --git a/Trigger/Algo/include/Trigger/TrigHcalEnergySum.h b/Trigger/Algo/include/Trigger/TrigHcalEnergySum.h index b102daa5a..a28c6e661 100644 --- a/Trigger/Algo/include/Trigger/TrigHcalEnergySum.h +++ b/Trigger/Algo/include/Trigger/TrigHcalEnergySum.h @@ -30,20 +30,9 @@ class TrigHcalEnergySum : public framework::Producer { virtual void produce(framework::Event& event); - virtual void onFileOpen(); - - virtual void onFileClose(); - - virtual void onProcessStart(); - - virtual void onProcessEnd(); - typedef ap_ufixed<16, 14> e_t; // [MeV] (Up to at least 8 GeV) private: - // specific verbosity of this producer - int verbose_{0}; - // name of collection for trigger quads to be passed as input std::string inProc_; std::string quadCollName_; diff --git a/Trigger/Algo/src/Trigger/DumpFileWriter.cxx b/Trigger/Algo/src/Trigger/DumpFileWriter.cxx index 036299953..d1f349302 100644 --- a/Trigger/Algo/src/Trigger/DumpFileWriter.cxx +++ b/Trigger/Algo/src/Trigger/DumpFileWriter.cxx @@ -9,12 +9,6 @@ namespace trigger { void DumpFileWriter::configure(framework::config::Parameters& ps) {} void DumpFileWriter::analyze(const framework::Event& event) { - const ecal::EcalTriggerGeometry& geom = - getCondition( - ecal::EcalTriggerGeometry::CONDITIONS_OBJECT_NAME); - const ldmx::EcalGeometry& hexReadout = getCondition( - ldmx::EcalGeometry::CONDITIONS_OBJECT_NAME); - if (!event.exists("ecalTrigDigis")) return; auto ecalTrigDigis{ event.getObject("ecalTrigDigis")}; @@ -45,18 +39,6 @@ void DumpFileWriter::analyze(const framework::Event& event) { evtNo++; } -void DumpFileWriter::onFileOpen() { - ldmx_log(debug) << "Opening file!"; - - return; -} - -void DumpFileWriter::onFileClose() { - ldmx_log(debug) << "Closing file!"; - - return; -} - void DumpFileWriter::onProcessStart() { ldmx_log(debug) << "Process starts!"; diff --git a/Trigger/Algo/src/Trigger/EcalTPSelector.cxx b/Trigger/Algo/src/Trigger/EcalTPSelector.cxx index 20f594437..e90f566fa 100644 --- a/Trigger/Algo/src/Trigger/EcalTPSelector.cxx +++ b/Trigger/Algo/src/Trigger/EcalTPSelector.cxx @@ -8,12 +8,6 @@ void EcalTPSelector::configure(framework::config::Parameters& ps) { } void EcalTPSelector::produce(framework::Event& event) { - const ecal::EcalTriggerGeometry& geom = - getCondition( - ecal::EcalTriggerGeometry::CONDITIONS_OBJECT_NAME); - const ldmx::EcalGeometry& hexReadout = getCondition( - ldmx::EcalGeometry::CONDITIONS_OBJECT_NAME); - if (!event.exists(tpCollName_)) return; auto ecalTrigDigis{ event.getObject(tpCollName_)}; @@ -137,14 +131,13 @@ void EcalTPSelector::produce(framework::Event& event) { void EcalTPSelector::decodeTP(ldmx::HgcrocTrigDigi tp, double& x, double& y, double& z, double& e) { + ldmx::EcalTriggerID tid(tp.getId()); const ecal::EcalTriggerGeometry& geom = getCondition( ecal::EcalTriggerGeometry::CONDITIONS_OBJECT_NAME); - const ldmx::EcalGeometry& hexReadout = getCondition( - ldmx::EcalGeometry::CONDITIONS_OBJECT_NAME); - - ldmx::EcalTriggerID tid(tp.getId()); // const auto center_ecalID = geom.centerInTriggerCell(tid); + // const ldmx::EcalGeometry& hexReadout = getCondition( + // ldmx::EcalGeometry::CONDITIONS_OBJECT_NAME); // hexReadout.getCellAbsolutePosition(center_ecalID,x,y,z); std::tie(x, y, z) = geom.globalPosition(tid); // e = primitiveToEnergy(tp.linearPrimitive(), tid.layer()); @@ -152,30 +145,6 @@ void EcalTPSelector::decodeTP(ldmx::HgcrocTrigDigi tp, double& x, double& y, e = cvt.calc(tp.linearPrimitive(), tid.layer()); } -void EcalTPSelector::onFileOpen() { - ldmx_log(debug) << "Opening file!"; - - return; -} - -void EcalTPSelector::onFileClose() { - ldmx_log(debug) << "Closing file!"; - - return; -} - -void EcalTPSelector::onProcessStart() { - ldmx_log(debug) << "Process starts!"; - - return; -} - -void EcalTPSelector::onProcessEnd() { - ldmx_log(debug) << "Process ends!"; - - return; -} - } // namespace trigger DECLARE_PRODUCER_NS(trigger, EcalTPSelector); diff --git a/Trigger/Algo/src/Trigger/Event/TrigEnergySum.cxx b/Trigger/Algo/src/Trigger/Event/TrigEnergySum.cxx index 0a08a3139..44b6cac17 100644 --- a/Trigger/Algo/src/Trigger/Event/TrigEnergySum.cxx +++ b/Trigger/Algo/src/Trigger/Event/TrigEnergySum.cxx @@ -2,28 +2,28 @@ #include -ClassImp(trigger::TrigEnergySum) +ClassImp(trigger::TrigEnergySum); - namespace trigger { - TrigEnergySum::TrigEnergySum(int layer, int hwEnergy) - : layer_(layer), module_(0), energy_(0), hwEnergy_{hwEnergy} {} - TrigEnergySum::TrigEnergySum(int layer, int module, float energy) - : layer_(layer), module_(module), energy_(energy), hwEnergy_{0} {} +namespace trigger { +TrigEnergySum::TrigEnergySum(int layer, int hwEnergy) + : layer_{layer}, module_{0}, hwEnergy_{hwEnergy}, energy_{0} {} +TrigEnergySum::TrigEnergySum(int layer, int module, float energy) + : layer_{layer}, module_{module}, hwEnergy_{0}, energy_{energy} {} - void TrigEnergySum::Print() const { std::cout << *this << std::endl; } +void TrigEnergySum::Print() const { std::cout << *this << std::endl; } - std::ostream &operator<<(std::ostream &s, const trigger::TrigEnergySum &sum) { - s << "TrigEnergySum { " - << "(layer " << sum.layer() << ", hwEnergy " << sum.hwEnergy() << " } "; - return s; - } +std::ostream &operator<<(std::ostream &s, const trigger::TrigEnergySum &sum) { + s << "TrigEnergySum { " + << "(layer " << sum.layer() << ", hwEnergy " << sum.hwEnergy() << " } "; + return s; +} - std::ostream &operator<<(std::ostream &s, - const trigger::TrigEnergySumCollection &sums) { - s << "TrigEnergySumCollection { " << std::endl; - for (auto sum : sums) s << " " << sum << std::endl; - s << "}"; - return s; - } +std::ostream &operator<<(std::ostream &s, + const trigger::TrigEnergySumCollection &sums) { + s << "TrigEnergySumCollection { " << std::endl; + for (auto sum : sums) s << " " << sum << std::endl; + s << "}"; + return s; +} } // namespace trigger diff --git a/Trigger/Algo/src/Trigger/IdealClusterBuilder.cxx b/Trigger/Algo/src/Trigger/IdealClusterBuilder.cxx index 23a776f0e..38e723edb 100644 --- a/Trigger/Algo/src/Trigger/IdealClusterBuilder.cxx +++ b/Trigger/Algo/src/Trigger/IdealClusterBuilder.cxx @@ -168,18 +168,17 @@ std::vector IdealClusterBuilder::Build2dClustersLayer( c.yy = 0; c.zz = 0; float sumw = 0; - for (auto h : c.hits) { - auto hit = h; + for (auto hit : c.hits) { // if(debug) cout << hit.x << " " << hit.y << " " << hit.z << endl; - c.e += h.e; + c.e += hit.e; // cout << "2d: " << h.e << " " << log(h.e/MIN_TP_ENERGY) << endl; - float w = std::max(0., log(h.e / MIN_TP_ENERGY)); // use log-e wgt - c.x += h.x * w; - c.y += h.y * w; - c.z += h.z * w; - c.xx += h.x * h.x * w; - c.yy += h.y * h.y * w; - c.zz += h.z * h.z * w; + float w = std::max(0., log(hit.e / MIN_TP_ENERGY)); // use log-e wgt + c.x += hit.x * w; + c.y += hit.y * w; + c.z += hit.z * w; + c.xx += hit.x * hit.x * w; + c.yy += hit.y * hit.y * w; + c.zz += hit.z * hit.z * w; sumw += w; } c.x /= sumw; @@ -349,7 +348,7 @@ void IdealClusterBuilder::Build3dClusters() { c.yy = 0; c.zz = 0; float sumw = 0; - for (auto c2 : c.clusters2d) { + for (auto &c2 : c.clusters2d) { c.e += c2.e; // cout << "3d: " << c2.e << " " << log(c2.e/MIN_TP_ENERGY) << endl; float w = std::max(0., log(c2.e / MIN_TP_ENERGY)); // use log-e wgt @@ -459,7 +458,7 @@ void IdealClusterBuilder::Fit(Cluster &c3) { std::vector x; std::vector y; std::vector z; - for (const auto c2 : c3.clusters2d) { + for (const auto &c2 : c3.clusters2d) { // logE.push_back( log(c2.e) ); x.push_back(c2.x); y.push_back(c2.y); diff --git a/Trigger/Algo/src/Trigger/NtupleWriter.cxx b/Trigger/Algo/src/Trigger/NtupleWriter.cxx index ef5a0cd86..60f75bc76 100644 --- a/Trigger/Algo/src/Trigger/NtupleWriter.cxx +++ b/Trigger/Algo/src/Trigger/NtupleWriter.cxx @@ -93,9 +93,7 @@ void NtupleWriter::produce(framework::Event& event) { inTag = "hcalTrigQuadsBackLayerSums"; if (writeHcalSums_ && event.exists(inTag)) { const auto sums = event.getCollection(inTag); - const int nHcalLayers = 50; - // int nLayers = 0; - vector energyAfterLayer; // (nHcalLayers, 0.); + vector energyAfterLayer; for (const auto& sum : sums) { if (!(sum.hwEnergy() > 0)) continue; if (sum.layer() >= energyAfterLayer.size()) diff --git a/Trigger/Algo/src/Trigger/TrigEcalClusterProducer.cxx b/Trigger/Algo/src/Trigger/TrigEcalClusterProducer.cxx index 710440d91..c11569534 100644 --- a/Trigger/Algo/src/Trigger/TrigEcalClusterProducer.cxx +++ b/Trigger/Algo/src/Trigger/TrigEcalClusterProducer.cxx @@ -18,8 +18,6 @@ void TrigEcalClusterProducer::produce(framework::Event& event) { const ecal::EcalTriggerGeometry& geom = getCondition( ecal::EcalTriggerGeometry::CONDITIONS_OBJECT_NAME); - const ldmx::EcalGeometry& hexReadout = getCondition( - ldmx::EcalGeometry::CONDITIONS_OBJECT_NAME); if (!event.exists(hitCollName_)) return; auto ecalTrigDigis{ @@ -38,6 +36,8 @@ void TrigEcalClusterProducer::produce(framework::Event& event) { double x, y, z; // const auto center_ecalID = geom.centerInTriggerCell(tid); + // const ldmx::EcalGeometry& hexReadout = getCondition( + // ldmx::EcalGeometry::CONDITIONS_OBJECT_NAME); // hexReadout.getCellAbsolutePosition(center_ecalID,x,y,z); // std::tie(x,y) = geom.globalPosition( tid ); std::tie(x, y, z) = geom.globalPosition(tid); @@ -99,31 +99,6 @@ void TrigEcalClusterProducer::produce(framework::Event& event) { event.add(clusterCollName_, trigClusters); } - -void TrigEcalClusterProducer::onFileOpen() { - ldmx_log(debug) << "Opening file!"; - - return; -} - -void TrigEcalClusterProducer::onFileClose() { - ldmx_log(debug) << "Closing file!"; - - return; -} - -void TrigEcalClusterProducer::onProcessStart() { - ldmx_log(debug) << "Process starts!"; - - return; -} - -void TrigEcalClusterProducer::onProcessEnd() { - ldmx_log(debug) << "Process ends!"; - - return; -} - } // namespace trigger DECLARE_PRODUCER_NS(trigger, TrigEcalClusterProducer); diff --git a/Trigger/Algo/src/Trigger/TrigEcalEnergySum.cxx b/Trigger/Algo/src/Trigger/TrigEcalEnergySum.cxx index 0fd0e522e..a49d3ddd9 100644 --- a/Trigger/Algo/src/Trigger/TrigEcalEnergySum.cxx +++ b/Trigger/Algo/src/Trigger/TrigEcalEnergySum.cxx @@ -9,25 +9,16 @@ namespace trigger { void TrigEcalEnergySum::configure(framework::config::Parameters& ps) { - // std::cout << "c++ configuring TrigEcalEnergySum" << std::endl; hitCollName_ = ps.getParameter("hitCollName"); } void TrigEcalEnergySum::produce(framework::Event& event) { - // std::cout << "c++ producing TrigEcalEnergySum" << std::endl; - - const ecal::EcalTriggerGeometry& geom = - getCondition( - ecal::EcalTriggerGeometry::CONDITIONS_OBJECT_NAME); - const ldmx::EcalGeometry& hexReadout = getCondition( - ldmx::EcalGeometry::CONDITIONS_OBJECT_NAME); - if (!event.exists(hitCollName_)) return; auto ecalTrigDigis{ event.getObject(hitCollName_)}; // floating point algorithm - float total_e = 0; + // float total_e = 0; // e_t total_e_trunc=0; // run the firmware (hls) algorithm directly @@ -45,7 +36,7 @@ void TrigEcalEnergySum::produce(framework::Event& event) { // mVtoMeV; // in MeV, before layer corrections // float e = (sie / mipSiEnergy * layerWeights.at(tid.layer()) + sie) * // secondOrderEnergyCorrection; - total_e += e; + // total_e += e; // total_e_trunc = total_e_trunc + e_t(e); if (iTP < N_INPUT_TP) { @@ -61,30 +52,6 @@ void TrigEcalEnergySum::produce(framework::Event& event) { // << " MeV)" << std::endl; } -void TrigEcalEnergySum::onFileOpen() { - ldmx_log(debug) << "Opening file!"; - - return; -} - -void TrigEcalEnergySum::onFileClose() { - ldmx_log(debug) << "Closing file!"; - - return; -} - -void TrigEcalEnergySum::onProcessStart() { - ldmx_log(debug) << "Process starts!"; - - return; -} - -void TrigEcalEnergySum::onProcessEnd() { - ldmx_log(debug) << "Process ends!"; - - return; -} - } // namespace trigger DECLARE_PRODUCER_NS(trigger, TrigEcalEnergySum); diff --git a/Trigger/Algo/src/Trigger/TrigElectronProducer.cxx b/Trigger/Algo/src/Trigger/TrigElectronProducer.cxx index 34eea44cc..7263aaf50 100644 --- a/Trigger/Algo/src/Trigger/TrigElectronProducer.cxx +++ b/Trigger/Algo/src/Trigger/TrigElectronProducer.cxx @@ -102,17 +102,6 @@ void TrigElectronProducer::produce(framework::Event& event) { event.add(eleCollName_, eles); } -void TrigElectronProducer::onFileOpen() { - ldmx_log(debug) << "Opening file!"; - - return; -} - -void TrigElectronProducer::onFileClose() { - ldmx_log(debug) << "Closing file!"; - - return; -} void TrigElectronProducer::setupMaps(bool isX) { TProfile2D* prof = isX ? propMapx_ : propMapy_; const int N = prof->GetXaxis()->GetNbins(); @@ -145,7 +134,7 @@ float TrigElectronProducer::getP(bool isX, float e, float d) { if (debug) std::cout << "null pointer" << std::endl; return 0; } - int bin1, bin2; + int bin1{-9999}, bin2{-9999}; bin1 = prof->GetXaxis()->FindBin(e); float frac = e - prof->GetXaxis()->GetBinCenter(bin1); float diff = fabs(prof->GetXaxis()->GetBinCenter(bin1) - @@ -173,7 +162,6 @@ float TrigElectronProducer::getP(bool isX, float e, float d) { void TrigElectronProducer::onProcessStart() { ldmx_log(debug) << "Process starts!"; - auto d = gDirectory; TFile* f = new TFile(propMapName_.c_str(), "read"); propMapx_ = (TProfile2D*)f->Get("profx"); propMapx_->SetDirectory(0); diff --git a/Trigger/Algo/src/Trigger/TrigHcalEnergySum.cxx b/Trigger/Algo/src/Trigger/TrigHcalEnergySum.cxx index 3c930b527..9fffee237 100644 --- a/Trigger/Algo/src/Trigger/TrigHcalEnergySum.cxx +++ b/Trigger/Algo/src/Trigger/TrigHcalEnergySum.cxx @@ -19,27 +19,24 @@ void TrigHcalEnergySum::produce(framework::Event& event) { // PE/MIP: 68 (summed over BOTH ends, based on 1808.05219, p38) // mV/PE: 5 // mV/MeV: 72.961 (= 5*68/4.66) - const float mV_per_adc = 1.2; + // const float mV_per_adc = 1.2; // adc gain - const float pe_per_adc = mV_per_adc / 5; - const float MeV_per_adc = mV_per_adc / 72.961; + // these are unused, should they be? FIXME + // const float pe_per_adc = mV_per_adc / 5; + // const float MeV_per_adc = mV_per_adc / 72.961; // const float samp_frac = 371/4e3; // ad-hoc, from a 4 GeV neutron sample // interaction length in Fe ('steel') = 16.77 cm (132.1 g/cm2) // polystyrene = 77.07 cm (81.7 g/cm2) // back hcal is 20mm bar, 25mm absorber - const float had_samp_frac = - (20 / 77.07) / (20 / 77.07 + 25 / 16.77); // 0.148266 - const float em_samp_frac = - (20 / 41.31) / (20 / 41.31 + 25 / 1.757); // 0.032906 - const float samp_frac = (em_samp_frac + 2 * had_samp_frac) / 3; // 0.109813 - const float attenuation = exp(-1 / 5.); // 5m attenuation length, 1m half-bar - - const ldmx::HcalGeometry& hcalGeom = getCondition( - ldmx::HcalGeometry::CONDITIONS_OBJECT_NAME); - const hcal::HcalTriggerGeometry& trigGeom = - getCondition( - hcal::HcalTriggerGeometry::CONDITIONS_OBJECT_NAME); + // these are unused, should they be? FIXME + // const float had_samp_frac = + // (20 / 77.07) / (20 / 77.07 + 25 / 16.77); // 0.148266 + // const float em_samp_frac = + // (20 / 41.31) / (20 / 41.31 + 25 / 1.757); // 0.032906 + // const float samp_frac = (em_samp_frac + 2 * had_samp_frac) / 3; // + // 0.109813 const float attenuation = exp(-1 / 5.); // 5m attenuation length, + // 1m half-bar // for(auto t : event.searchProducts("","","")) std::cout << t.name() << " " // << t.passname() << " " << t.type() << std::endl; @@ -128,30 +125,6 @@ void TrigHcalEnergySum::produce(framework::Event& event) { event.add(combinedQuadCollName_ + "Sum", totalSum); } -void TrigHcalEnergySum::onFileOpen() { - ldmx_log(debug) << "Opening file!"; - - return; -} - -void TrigHcalEnergySum::onFileClose() { - ldmx_log(debug) << "Closing file!"; - - return; -} - -void TrigHcalEnergySum::onProcessStart() { - ldmx_log(debug) << "Process starts!"; - - return; -} - -void TrigHcalEnergySum::onProcessEnd() { - ldmx_log(debug) << "Process ends!"; - - return; -} - } // namespace trigger DECLARE_PRODUCER_NS(trigger, TrigHcalEnergySum); From c29cfd3861f9e3f597297557a3b51ce78f5b0716 Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Tue, 10 Sep 2024 14:38:20 -0700 Subject: [PATCH 10/14] Additional changes --- Biasing/include/Biasing/TargetProcessFilter.h | 2 +- Biasing/src/Biasing/TargetProcessFilter.cxx | 2 -- Conditions/src/Conditions/URLStreamer.cxx | 7 +++---- Conditions/test/TestConditions.cxx | 12 ++++-------- Ecal/include/Ecal/EcalDigiProducer.h | 3 ++- 5 files changed, 10 insertions(+), 16 deletions(-) diff --git a/Biasing/include/Biasing/TargetProcessFilter.h b/Biasing/include/Biasing/TargetProcessFilter.h index 1fca4f56d..f26c4266e 100644 --- a/Biasing/include/Biasing/TargetProcessFilter.h +++ b/Biasing/include/Biasing/TargetProcessFilter.h @@ -32,7 +32,7 @@ class TargetProcessFilter : public simcore::UserAction { framework::config::Parameters ¶meters); /// Destructor - ~TargetProcessFilter(); + virtual ~TargetProcessFilter() = default; /** * Implementmthe stepping action which performs the target volume biasing. diff --git a/Biasing/src/Biasing/TargetProcessFilter.cxx b/Biasing/src/Biasing/TargetProcessFilter.cxx index 991ab9a5e..a85c6e53b 100644 --- a/Biasing/src/Biasing/TargetProcessFilter.cxx +++ b/Biasing/src/Biasing/TargetProcessFilter.cxx @@ -28,8 +28,6 @@ TargetProcessFilter::TargetProcessFilter( process_ = parameters.getParameter("process"); } -TargetProcessFilter::~TargetProcessFilter() {} - G4ClassificationOfNewTrack TargetProcessFilter::ClassifyNewTrack( const G4Track* track, const G4ClassificationOfNewTrack& currentTrackClass) { if (track == currentTrack_) { diff --git a/Conditions/src/Conditions/URLStreamer.cxx b/Conditions/src/Conditions/URLStreamer.cxx index 8b8fb7a30..30e275f23 100644 --- a/Conditions/src/Conditions/URLStreamer.cxx +++ b/Conditions/src/Conditions/URLStreamer.cxx @@ -22,7 +22,7 @@ void urlstatistics(unsigned int& http_requests, unsigned int& http_failures) { } std::unique_ptr urlstream(const std::string& url) { - if ((url.find("file://") == 0 || url.length() > 0) && (url[0] == '/')) { + if (url.find("file://") == 0 || (url.length() > 0 && url[0] == '/')) { std::string fname = url; if (fname.find("file://") == 0) fname = url.substr(url.find("file://") + strlen("file://")); @@ -46,9 +46,8 @@ std::unique_ptr urlstream(const std::string& url) { execl("/usr/bin/wget", "wget", "-q", "--no-check-certificate", "-O", fname, "-o", "/tmp/wget.log", url.c_str(), (char*)0); } else { - int wstatus{9999}; - // wrv is not used, was it meant to be? FIXME - // int wrv = waitpid(apid, &wstatus, 0); + int wstatus; + waitpid(apid, &wstatus, 0); // std::cout << "EXITED: " << WIFEXITED(wstatus) << " STATUS: " << // WEXITSTATUS(wstatus) << std::endl; if (WIFEXITED(wstatus) != 1 || WEXITSTATUS(wstatus) != 0) { diff --git a/Conditions/test/TestConditions.cxx b/Conditions/test/TestConditions.cxx index 5ecb8b4b4..cb95c9742 100644 --- a/Conditions/test/TestConditions.cxx +++ b/Conditions/test/TestConditions.cxx @@ -309,10 +309,8 @@ TEST_CASE("Conditions", "[Conditions]") { cxt.setRun(128); - // This is unused, should it be? FIXME - // const DoubleTableCondition& httpTable128 = - // hp->getConditions().getCondition( - // "testbeam22_pedestals"); + hp->getConditions().getCondition( + "testbeam22_pedestals"); hp->getConditions().getCondition( "testbeam22_pedestals"); @@ -332,10 +330,8 @@ TEST_CASE("Conditions", "[Conditions]") { cxt.setRun(140); - // this is unused, should it be? FIXME - // const DoubleTableCondition& httpTable140 = - // hp->getConditions().getCondition( - // "testbeam22_pedestals"); + hp->getConditions().getCondition( + "testbeam22_pedestals"); conditions::urlstatistics(http_requests[1], http_failures[1]); REQUIRE(((http_requests[1] - http_requests[0]) == 2 && diff --git a/Ecal/include/Ecal/EcalDigiProducer.h b/Ecal/include/Ecal/EcalDigiProducer.h index 5fc07421f..0e3aa18f7 100644 --- a/Ecal/include/Ecal/EcalDigiProducer.h +++ b/Ecal/include/Ecal/EcalDigiProducer.h @@ -109,7 +109,8 @@ class EcalDigiProducer : public framework::Producer { std::unique_ptr hgcroc_; /// Total number of channels in the ECal - [[maybe_unused]] int nTotalChannels_; + // Currently unused, should it be? FIXME + // int nTotalChannels_; /// Conversion from time in ns to ticks of the internal clock double ns_; From 0a920a63231ac05827f79707c633bf2a009a7c9b Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Tue, 10 Sep 2024 15:25:25 -0700 Subject: [PATCH 11/14] Clang format --- Conditions/test/TestConditions.cxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Conditions/test/TestConditions.cxx b/Conditions/test/TestConditions.cxx index cb95c9742..35da0a441 100644 --- a/Conditions/test/TestConditions.cxx +++ b/Conditions/test/TestConditions.cxx @@ -309,8 +309,8 @@ TEST_CASE("Conditions", "[Conditions]") { cxt.setRun(128); - hp->getConditions().getCondition( - "testbeam22_pedestals"); + hp->getConditions().getCondition( + "testbeam22_pedestals"); hp->getConditions().getCondition( "testbeam22_pedestals"); @@ -330,8 +330,8 @@ TEST_CASE("Conditions", "[Conditions]") { cxt.setRun(140); - hp->getConditions().getCondition( - "testbeam22_pedestals"); + hp->getConditions().getCondition( + "testbeam22_pedestals"); conditions::urlstatistics(http_requests[1], http_failures[1]); REQUIRE(((http_requests[1] - http_requests[0]) == 2 && From f1f2eae4fd365fba6b0d45c6a03df24a968004d6 Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Wed, 11 Sep 2024 09:52:01 -0700 Subject: [PATCH 12/14] Delete unused code in Ecal, Frameworks, Biasing as agreed with Tom --- Biasing/include/Biasing/TargetProcessFilter.h | 4 -- CMakeLists.txt | 2 +- .../src/Conditions/SimpleTableStreamers.cxx | 2 - Ecal/include/Ecal/EcalDigiProducer.h | 4 +- Ecal/include/Ecal/MyClusterWeight.h | 4 -- Ecal/src/Ecal/EcalRawDecoder.cxx | 54 +++++++++++-------- Ecal/src/Ecal/EcalTriggerGeometry.cxx | 5 -- Ecal/src/Ecal/Event/EcalDigiCollection.cxx | 5 -- Framework/test/ConfigurePythonTest.cxx | 16 ------ Hcal/include/Hcal/MyClusterWeight.h | 18 +++---- Tools/src/Tools/HgcrocEmulator.cxx | 6 --- .../src/Tracking/geo/EcalTrackingGeometry.cxx | 3 -- 12 files changed, 44 insertions(+), 79 deletions(-) diff --git a/Biasing/include/Biasing/TargetProcessFilter.h b/Biasing/include/Biasing/TargetProcessFilter.h index f26c4266e..5c4a7df6b 100644 --- a/Biasing/include/Biasing/TargetProcessFilter.h +++ b/Biasing/include/Biasing/TargetProcessFilter.h @@ -65,10 +65,6 @@ class TargetProcessFilter : public simcore::UserAction { /** Pointer to the current track being processed. */ G4Track *currentTrack_{nullptr}; - /** Flag indicating if the reaction of intereset occurred. */ - // It is usused, should it be? FIXME - // bool reactionOccurred_{false}; - /// The process to bias std::string process_{""}; }; diff --git a/CMakeLists.txt b/CMakeLists.txt index 83d02f9be..4eb70a23c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,7 +30,7 @@ list(APPEND CMAKE_MODULE_PATH ${LDMX_SW_SOURCE_DIR}/cmake/) # is set to NOTFOUND. include(BuildMacros RESULT_VARIABLE build_macros_found) -# Adding ROOT, ACTS, HLT arb prec as "SYSTEM" +# Adding ROOT, ACTS, HLS arb prec as "SYSTEM" # which will silence compiler warnings from these 3rd party softwares include_directories(SYSTEM /usr/local/include/root) include_directories(SYSTEM Tracking/acts/Core/include) diff --git a/Conditions/src/Conditions/SimpleTableStreamers.cxx b/Conditions/src/Conditions/SimpleTableStreamers.cxx index 37d875548..ccbb32ced 100644 --- a/Conditions/src/Conditions/SimpleTableStreamers.cxx +++ b/Conditions/src/Conditions/SimpleTableStreamers.cxx @@ -19,8 +19,6 @@ static void storeIdFields(unsigned int id, std::ostream& s) { template void storeT(const T& t, std::ostream& s, bool expandIds) { - // buffer is unused, was it meant to be? FIXME - // char buffer[100]; // write the header line s << "\"DetID\""; if (expandIds && t.getRowCount() > 0) { diff --git a/Ecal/include/Ecal/EcalDigiProducer.h b/Ecal/include/Ecal/EcalDigiProducer.h index 0e3aa18f7..cd4ade679 100644 --- a/Ecal/include/Ecal/EcalDigiProducer.h +++ b/Ecal/include/Ecal/EcalDigiProducer.h @@ -109,8 +109,8 @@ class EcalDigiProducer : public framework::Producer { std::unique_ptr hgcroc_; /// Total number of channels in the ECal - // Currently unused, should it be? FIXME - // int nTotalChannels_; + // In a real data, zero-compression scenario the number of channels per event + // will change. Unused now, but keeping if for future dev int nTotalChannels_; /// Conversion from time in ns to ticks of the internal clock double ns_; diff --git a/Ecal/include/Ecal/MyClusterWeight.h b/Ecal/include/Ecal/MyClusterWeight.h index 7680b25b5..beec350cf 100644 --- a/Ecal/include/Ecal/MyClusterWeight.h +++ b/Ecal/include/Ecal/MyClusterWeight.h @@ -30,13 +30,9 @@ class MyClusterWeight { double bZ = b.centroid().Pz(); double dijz; - // eFrac is set but not used, should it be? FIXME - // double eFrac; if (aE >= bE) { - // eFrac = bE / aE; dijz = bZ - aZ; } else { - // eFrac = aE / bE; dijz = aZ - bZ; } diff --git a/Ecal/src/Ecal/EcalRawDecoder.cxx b/Ecal/src/Ecal/EcalRawDecoder.cxx index 14f50b9ef..1d9cc3697 100644 --- a/Ecal/src/Ecal/EcalRawDecoder.cxx +++ b/Ecal/src/Ecal/EcalRawDecoder.cxx @@ -56,11 +56,12 @@ void EcalRawDecoder::produce(framework::Event& event) { */ uint32_t whole_event_header; reader >> whole_event_header; - /* these are unused, should they have been? FIXME - uint32_t version = (whole_event_header >> 28) & packing::utility::mask<4>; - uint32_t fpga = (whole_event_header >> 20) & packing::utility::mask<8>; - uint32_t eventlen = whole_event_header & packing::utility::mask<16>; -*/ + /* event length not currently used but is stored in header as defined by + the spec uint32_t version = (whole_event_header >> 28) & + packing::utility::mask<4>; uint32_t fpga = (whole_event_header >> 20) & + packing::utility::mask<8>; uint32_t eventlen = whole_event_header & + packing::utility::mask<16>; + */ uint32_t nsamples = (whole_event_header >> 16) & packing::utility::mask<4>; // sample counters @@ -109,6 +110,7 @@ void EcalRawDecoder::produce(framework::Event& event) { uint32_t fpga = (head1 >> 20) & packing::utility::mask<8>; uint32_t nlinks = (head1 >> 14) & packing::utility::mask<6>; + // total length not used but is available in header as defined by spec // uint32_t len = head1 & packing::utility::mask<12>; // std::cout << ", fpga: " << fpga << ", nlinks: " << nlinks << ", len: " << @@ -116,9 +118,11 @@ void EcalRawDecoder::produce(framework::Event& event) { fpga_crc << head2; // std::cout << hex(head2) << " : "; - // uint32_t bx_id = (head2 >> 20) & packing::utility::mask<12>; - // uint32_t rreq = (head2 >> 10) & packing::utility::mask<10>; - // uint32_t orbit = head2 & packing::utility::mask<10>; + // bunch ID (bx), read request (rreq) and orbit counter (orbit) are defined + // in header but not used right now uint32_t bx_id = (head2 >> 20) & + // packing::utility::mask<12>; uint32_t rreq = (head2 >> 10) & + // packing::utility::mask<10>; uint32_t orbit = head2 & + // packing::utility::mask<10>; // std::cout << "bx_id: " << bx_id << ", rreq: " << rreq << ", orbit: " << // orbit << std::endl; @@ -131,7 +135,8 @@ void EcalRawDecoder::produce(framework::Event& event) { // std::cout << hex(w) << " : Four Link Pack " << std::endl; } uint32_t shift_in_word = 8 * (i_link % 4); - // These are unused, should they be? FIXME + // the check sums performed by the HGCROC and downstream chips can + // confirm the validity of the data, not used here (yet) // bool rid_ok = ((w >> (shift_in_word + 7)) & packing::utility::mask<1>) // == 1; bool cdc_ok = ((w >> (shift_in_word + 6)) & // packing::utility::mask<1>) == 1; @@ -158,6 +163,7 @@ void EcalRawDecoder::produce(framework::Event& event) { fpga_crc << w; link_crc << w; uint32_t roc_id = (w >> 16) & packing::utility::mask<16>; + // checksum for this chunk of data can be used to confirm data validity // bool crc_ok = (w >> 15) & packing::utility::mask<1> == 1; // std::cout << hex(w) << " : roc_id " << roc_id << ", crc_ok (v2 // always false) " << std::boolalpha << crc_ok << std::endl; @@ -199,7 +205,10 @@ void EcalRawDecoder::produce(framework::Event& event) { */ // std::cout << " : ROC Header"; /* - * These are unused, should they be? FIXME + * These are unused, but are available as defined by the spec + * additionally, we are calculating our own copy of the link checksum + for later comparison + * if the data packet has readout its own checksum link_crc << w; uint32_t bx_id = (w >> 16) & packing::utility::mask<12>; uint32_t short_event = (w >> 10) & packing::utility::mask<6>; @@ -214,16 +223,18 @@ void EcalRawDecoder::produce(framework::Event& event) { // std::cout << " : Common Mode"; } else if (channel_id == 39) { /* - // CRC checksum from ROC - uint32_t crc = w; - // std::cout << " : CRC checksum : " << hex(link_crc.get()) << - // " =? " << hex(crc); - if (link_crc.get() != crc) { - EXCEPTION_RAISE("BadCRC", - "Our calculated link checksum doesn't match the " - "one from raw data."); - } - */ + // CRC checksum from ROC + uint32_t crc = w; + // std::cout << " : CRC checksum : " << hex(link_crc.get()) << " =? " + << hex(crc); + // keeping this there in comment is helpful when the HGCROC is sending + along its own CRC checksum of the data for us to compare against. + // We had to comment it out because the v2 HGCROC we were using was + not sending a checksum and thus the check would always fail. if + (link_crc.get() != crc) { EXCEPTION_RAISE("BadCRC", "Our calculated + link checksum doesn't match the " "one from raw data."); + } + */ } else { /// DAQ Channels @@ -263,7 +274,8 @@ void EcalRawDecoder::produce(framework::Event& event) { } // loop over links // another CRC checksum from FPGA - // reader >> w; + reader >> w; + // this word would be the CRC checksum as deteremined by the chip // uint32_t crc = w; // std::cout << "FPGA Checksum : " << hex(fpga_crc.get()) << " =? " // << hex(crc) << std::endl; diff --git a/Ecal/src/Ecal/EcalTriggerGeometry.cxx b/Ecal/src/Ecal/EcalTriggerGeometry.cxx index 675d0e12f..b972084f8 100644 --- a/Ecal/src/Ecal/EcalTriggerGeometry.cxx +++ b/Ecal/src/Ecal/EcalTriggerGeometry.cxx @@ -8,12 +8,7 @@ #include "Framework/EventHeader.h" namespace ecal { - -// These are unused, should they be? FIXME -// static const int LAYERS_MASK = 0xFF; -// static const int LAYERS_ODDEVEN = 0x02; static const int LAYERS_IDENTICAL = 0x01; - static const int MODULES_MASK = 0xFF00; static const int INPLANE_IDENTICAL = 0x0100; diff --git a/Ecal/src/Ecal/Event/EcalDigiCollection.cxx b/Ecal/src/Ecal/Event/EcalDigiCollection.cxx index 75ada50e0..75361f5cc 100644 --- a/Ecal/src/Ecal/Event/EcalDigiCollection.cxx +++ b/Ecal/src/Ecal/Event/EcalDigiCollection.cxx @@ -36,11 +36,6 @@ ClassImp(ldmx::EcalDigiCollection) int32_t word = samples_.at(digiIndex * numSamplesPerDigi_ + sampleIndex); // this is where the word --> measurements translation occurs - - // Unused, should it be? FIXME - // bool firstFlag = ONE_BIT_MASK & (word >> FIRSTFLAG_POS); - // Unused, should it be? FIXME - // bool seconFlag = ONE_BIT_MASK & (word >> SECONFLAG_POS); int firstMeas = TEN_BIT_MASK & (word >> FIRSTMEAS_POS); int seconMeas = TEN_BIT_MASK & (word >> SECONMEAS_POS); int lastMeas = TEN_BIT_MASK & (word); diff --git a/Framework/test/ConfigurePythonTest.cxx b/Framework/test/ConfigurePythonTest.cxx index a2a421951..67a65ab12 100644 --- a/Framework/test/ConfigurePythonTest.cxx +++ b/Framework/test/ConfigurePythonTest.cxx @@ -101,22 +101,6 @@ class TestConfig : public framework::Producer { virtual void produce(framework::Event &) final override {} }; -/** - * @func removeFile - * Deletes the file and returns whether the deletion was successful. - * - * This is just a helper function during development. - * Sometimes it is helpful to leave the generated files, so - * maybe we can make the removal optional? - */ -// static bool removeFile(const char *filepath) { return remove(filepath) == 0; -// } - -} // namespace test -} // namespace framework - -DECLARE_PRODUCER_NS(framework::test, TestConfig) - /** * Test for Configure Python class * diff --git a/Hcal/include/Hcal/MyClusterWeight.h b/Hcal/include/Hcal/MyClusterWeight.h index a0c1933fa..3760e1b67 100644 --- a/Hcal/include/Hcal/MyClusterWeight.h +++ b/Hcal/include/Hcal/MyClusterWeight.h @@ -29,21 +29,19 @@ class MyClusterWeight { double bZ = b.centroid().Pz(); double dijz; - // This is unused, should it be? FIXME - // double eFrac; if (aE >= bE) { - // eFrac = bE / aE; // ratio of energies - dijz = bZ - aZ; // differences in Z + // differences in Z + dijz = bZ - aZ; } else { - // eFrac = aE / bE; dijz = aZ - bZ; } - double dijT = - pow(pow(aX - bX, 2) + pow(aY - bY, 2), 0.5); // Transverse Difference - - double weightT = exp(pow(dijT / rmol, 2)) - 1; // Trans --> massive - double weightZ = (exp(abs(dijz) / dzchar) - 1); // Long + // Transverse Difference + double dijT = pow(pow(aX - bX, 2) + pow(aY - bY, 2), 0.5); + // Trans --> massive + double weightT = exp(pow(dijT / rmol, 2)) - 1; + // Long + double weightZ = (exp(abs(dijz) / dzchar) - 1); // Return the highest of the two weights if (weightT <= weightZ) { diff --git a/Tools/src/Tools/HgcrocEmulator.cxx b/Tools/src/Tools/HgcrocEmulator.cxx index d21d4beda..2de27fdf9 100644 --- a/Tools/src/Tools/HgcrocEmulator.cxx +++ b/Tools/src/Tools/HgcrocEmulator.cxx @@ -84,8 +84,6 @@ bool HgcrocEmulator::digitize( /// the time here is nominal (zero gives peak if hit.second is zero) // step 3: go through each BX sample one by one - // doReadout is unused, should it be? FIXME - // bool doReadout = false; bool wasTOA = false; for (int iADC = 0; iADC < nADCs_; iADC++) { double startBX = (iADC - iSOI_) * clockCycle_ - measTime; @@ -146,10 +144,6 @@ bool HgcrocEmulator::digitize( // actual time over threshold using the real signal voltage amplitude double tot = charge_deposited / drainRate; - // calculate the index that tot will complete on - // Unused, should it be? - // int num_whole_clocks = int(tot / clockCycle_); - // calculate the TDC counts for this tot measurement // internally, the chip uses 12 bits (2^12 = 4096) // to measure a maximum of tot Max [ns] diff --git a/Tracking/src/Tracking/geo/EcalTrackingGeometry.cxx b/Tracking/src/Tracking/geo/EcalTrackingGeometry.cxx index 96ca3cd11..65b1aed16 100644 --- a/Tracking/src/Tracking/geo/EcalTrackingGeometry.cxx +++ b/Tracking/src/Tracking/geo/EcalTrackingGeometry.cxx @@ -46,9 +46,6 @@ EcalTrackingGeometry::EcalTrackingGeometry(std::string gdmlfile, std::cout << "World position" << std::endl; std::cout << fWorldPhysVol->GetTranslation() << std::endl; } - // Get the logical volume from the world - // fWorldLogicalVol is unused, should it be? FIXME - // G4LogicalVolume* fWorldLogicalVol = fWorldPhysVol->GetLogicalVolume(); if (debug) std::cout << "Loop on world daughters" << std::endl; From 00e9b495f452203ef76a3b5c3d149be1477cf830 Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Wed, 11 Sep 2024 10:32:27 -0700 Subject: [PATCH 13/14] Delete unused code in HCAL as agreed with Einar --- DQM/src/DQM/HcalInefficiencyDQM.cxx | 1 - Framework/test/ConfigurePythonTest.cxx | 6 +- Hcal/test/HcalDigiPipelineTest.cxx | 12 ---- Hcal/test/HcalGeometryTest.cxx | 90 -------------------------- 4 files changed, 5 insertions(+), 104 deletions(-) diff --git a/DQM/src/DQM/HcalInefficiencyDQM.cxx b/DQM/src/DQM/HcalInefficiencyDQM.cxx index 61af8e20f..74fa652b8 100644 --- a/DQM/src/DQM/HcalInefficiencyDQM.cxx +++ b/DQM/src/DQM/HcalInefficiencyDQM.cxx @@ -18,7 +18,6 @@ void HcalInefficiencyAnalyzer::analyze(const framework::Event &event) { for (const auto &hit : hcalRecHits) { const ldmx::HcalID id{static_cast(hit.getID())}; const auto section{id.section()}; - // const auto z{hit.getZPos()}; const auto layer{id.layer()}; if (hitPassesVeto(hit, section)) { if (layer < firstLayersHit[section]) { diff --git a/Framework/test/ConfigurePythonTest.cxx b/Framework/test/ConfigurePythonTest.cxx index 67a65ab12..1a82f1575 100644 --- a/Framework/test/ConfigurePythonTest.cxx +++ b/Framework/test/ConfigurePythonTest.cxx @@ -98,8 +98,12 @@ class TestConfig : public framework::Producer { } // I don't do anything. - virtual void produce(framework::Event &) final override {} + virtual void produce(framework::Event &) override {} }; +} // namespace test +} // namespace framework + +DECLARE_PRODUCER_NS(framework::test, TestConfig) /** * Test for Configure Python class diff --git a/Hcal/test/HcalDigiPipelineTest.cxx b/Hcal/test/HcalDigiPipelineTest.cxx index cde6fd85a..e6af3de4a 100644 --- a/Hcal/test/HcalDigiPipelineTest.cxx +++ b/Hcal/test/HcalDigiPipelineTest.cxx @@ -46,18 +46,6 @@ static const double MAX_PE_ERROR_DAQ = 40; // large percentage error for now static const double MAX_PE_PERCENT_ERROR_DAQ = 0.4; -/** - * Maximum error that a single hit position along the bar - * can be reconstructed with before failing the test - * if in the back Hcal - * - * Comparing simulated position vs - * reconstructed position along the bar for even/odd layers in the back Hcal. - */ -// mm // scintillator length/2 -// static const double MAX_POSITION_ERROR_DAQ = 50. / 2; -// static const double MAX_POSITION_PERCENT_ERROR_DAQ = 0.3; - /** * Number of sim hits to create. * diff --git a/Hcal/test/HcalGeometryTest.cxx b/Hcal/test/HcalGeometryTest.cxx index bf535705f..1897917ce 100644 --- a/Hcal/test/HcalGeometryTest.cxx +++ b/Hcal/test/HcalGeometryTest.cxx @@ -13,16 +13,6 @@ using Catch::Approx; namespace hcal { namespace test { -/** - * Maximum percent error that a single hit position - * can be reconstructed/mapped. - * 20% for now. - */ -// static const double MAX_ENERGY_PERCENT_ERROR_ZPOSITION = 20; -// static const double MAX_ENERGY_PERCENT_ERROR_XPOSITION = 50; -// static const double MAX_ENERGY_PERCENT_ERROR_YPOSITION = 50; -// static const double MAX_ENERGY_PERCENT_ERROR_ZPOSITION_SIDE = 100; - /** * @class HcalCheckPositionMap * Checks: @@ -43,86 +33,6 @@ class HcalCheckPositionMap : public framework::Analyzer { event.getCollection("HcalSimHits"); CHECK(simHits.size() > 0); - /* - for (int ihit = 0; ihit < simHits.size(); ihit++) { - auto hit = simHits.at(ihit); - double x = hit.getPosition()[0]; - double y = hit.getPosition()[1]; - double z = hit.getPosition()[2]; - - unsigned int hitID = hit.getID(); - ldmx::HcalID detID(hitID); - int section = detID.section(); - int layer = detID.layer(); - int strip = detID.strip(); - // std::cout << "ihit " << ihit << " section " << detID.section() << - // " layer " << detID.layer() << " strip " << detID.strip() << - std::endl; - - // get the Hcal geometry - const auto &hcalGeometry = getCondition( - ldmx::HcalGeometry::CONDITIONS_OBJECT_NAME); - - // get position - auto positionMap = hcalGeometry.getStripCenterPosition(detID); - - if (section == ldmx::HcalID::HcalSection::BACK) { - auto target_z = - Approx(z).epsilon(MAX_ENERGY_PERCENT_ERROR_ZPOSITION / 100); - // CHECK(positionMap.Z() == target_z); - // std::cout << " BACK " << "x sim hit " << x << " (Sec,strip,layer) - " - // << section << " " << strip << " " << layer << " x " << - // positionMap.X() << std::endl; std::cout << " BACK " << "y sim hit - " - // << y << " (Sec,strip,layer) " << section << " " << strip << " " - << - // layer << " y " << positionMap.Y() << std::endl; std::cout << " - BACK " - // << "z sim hit " << z << " (Sec,strip,layer) " << section << " " - << - // strip << " " << layer << " z " << positionMap.Z() << std::endl; - - if (layer % 2 == 1) { - auto target_y = - Approx(y).epsilon(MAX_ENERGY_PERCENT_ERROR_YPOSITION / 100); - // CHECK(positionMap.Y() == target_y); - } else { - auto target_x = - Approx(x).epsilon(MAX_ENERGY_PERCENT_ERROR_YPOSITION / 100); - // CHECK(positionMap.X() == target_x); - } - } else { - auto target_z = - Approx(z).epsilon(MAX_ENERGY_PERCENT_ERROR_ZPOSITION_SIDE / - 100); - // CHECK(positionMap.Z() == target_z); - // std::cout << " SIDE " << "x sim hit " << x << " (Sec,strip,layer) - " - // << section << " " << strip << " " << layer << " x " << - // positionMap.X() << std::endl; std::cout << " SIDE " << "y sim hit - " - // << y << " (Sec,strip,layer) " << section << " " << strip << " " - << - // layer << " y " << positionMap.Y() << std::endl; std::cout << " - SIDE " - // << "z sim hit " << z << " (Sec,strip,layer) " << section << " " - << - // strip << " " << layer << " z " << positionMap.Z() << std::endl; - - if ((section == ldmx::HcalID::HcalSection::TOP) || - (section == ldmx::HcalID::HcalSection::BOTTOM)) { - auto target_y = - Approx(y).epsilon(MAX_ENERGY_PERCENT_ERROR_YPOSITION / 100); - // CHECK(positionMap.Y() == target_y); - } else if ((section == ldmx::HcalID::HcalSection::LEFT) || - (section == ldmx::HcalID::HcalSection::RIGHT)) { - auto target_x = - Approx(x).epsilon(MAX_ENERGY_PERCENT_ERROR_YPOSITION / 100); - // CHECK(positionMap.X() == target_x); - } - } - } */ return; } }; // HcalCheckPositionMap From 35e6434b17d9fcdeb1e07a60ca572f465a47176b Mon Sep 17 00:00:00 2001 From: Tamas Vami Date: Fri, 13 Sep 2024 08:34:15 -0700 Subject: [PATCH 14/14] Delete unused code in Tracking, as agreed with Matt --- Tracking/include/Tracking/Reco/CKFProcessor.h | 5 -- .../Tracking/Reco/SeedFinderProcessor.h | 3 -- .../Tracking/Reco/TrackExtrapolatorTool.h | 9 ---- Tracking/include/Tracking/Reco/Vertexer.h | 8 +-- .../include/Tracking/Sim/LdmxSpacePoint.h | 2 - .../Tracking/Sim/SeedToTrackParamMaker.ipp | 3 -- .../src/Tracking/Reco/SeedFinderProcessor.cxx | 51 +------------------ 7 files changed, 4 insertions(+), 77 deletions(-) diff --git a/Tracking/include/Tracking/Reco/CKFProcessor.h b/Tracking/include/Tracking/Reco/CKFProcessor.h index 0a691488c..6ac96299e 100644 --- a/Tracking/include/Tracking/Reco/CKFProcessor.h +++ b/Tracking/include/Tracking/Reco/CKFProcessor.h @@ -194,16 +194,11 @@ class CKFProcessor final : public TrackingGeometryUser { // 1DOF pvalues: 0.1 = 2.706 0.05 = 3.841 0.025 = 5.024 0.01 = 6.635 0.005 // = 7.879 The probability to reject a good measurement is pvalue The // probability to reject an outlier is given in NIM A262 (1987) 444-450 - double outlier_pval_{3.84}; // The output track collection std::string out_trk_collection_{"Tracks"}; - // Mass for the propagator hypothesis in MeV - // This is unused, should it be? FIXME - // double mass_{0.511}; - // The seed track collection std::string seed_coll_name_{"seedTracks"}; diff --git a/Tracking/include/Tracking/Reco/SeedFinderProcessor.h b/Tracking/include/Tracking/Reco/SeedFinderProcessor.h index fdb6e168a..0503f431c 100644 --- a/Tracking/include/Tracking/Reco/SeedFinderProcessor.h +++ b/Tracking/include/Tracking/Reco/SeedFinderProcessor.h @@ -167,10 +167,7 @@ class SeedFinderProcessor : public TrackingGeometryUser { long nfailtheta_{0}; // The measurements groups - std::map> groups_map; - // groups_array is unused, should it be? FIXME - // std::array groups_array; // Truth Matching tool std::shared_ptr truthMatchingTool_ = diff --git a/Tracking/include/Tracking/Reco/TrackExtrapolatorTool.h b/Tracking/include/Tracking/Reco/TrackExtrapolatorTool.h index c305cad29..a00783256 100644 --- a/Tracking/include/Tracking/Reco/TrackExtrapolatorTool.h +++ b/Tracking/include/Tracking/Reco/TrackExtrapolatorTool.h @@ -98,11 +98,6 @@ class TrackExtrapolatorTool { template std::optional extrapolate( track_t track, const std::shared_ptr& target_surface) { - // get first and last track state on surface - // auto& tsc = track.container().trackStateContainer(); - // auto ts_first = tsc.getTrackState(0); - // size_t nstates = track.nTrackStates(); - // auto ts_last = tsc.getTrackState(nstates-1); auto outermost = *(track.trackStates().begin()); auto begin = track.trackStates().begin(); std::advance(begin, track.nTrackStates() - 1); @@ -130,8 +125,6 @@ class TrackExtrapolatorTool { const auto& surface = ts.referenceSurface(); const auto& smoothed = ts.smoothed(); - // bool hasSmoothed = ts.hasSmoothed(); - // const auto& filtered = ts.filtered(); const auto& cov = ts.smoothedCovariance(); // std::cout<<"Surface::"<< @@ -156,8 +149,6 @@ class TrackExtrapolatorTool { // Now.. I'm taking whatever it is. I'm not checking here if it is a // measurement. - // size_t nstates = track.nTrackStates(); - // auto& tsc = track.container().trackStateContainer(); auto begin = track.trackStates().begin(); auto ts_last = *begin; const auto& surface = (ts_last).referenceSurface(); diff --git a/Tracking/include/Tracking/Reco/Vertexer.h b/Tracking/include/Tracking/Reco/Vertexer.h index a41aea039..fab00d8c2 100644 --- a/Tracking/include/Tracking/Reco/Vertexer.h +++ b/Tracking/include/Tracking/Reco/Vertexer.h @@ -49,7 +49,7 @@ class Vertexer : public framework::Producer { public: Vertexer(const std::string& name, framework::Process& process); - ~Vertexer() = default; + virtual ~Vertexer() = default; void onProcessStart() override; void onProcessEnd() override; @@ -77,14 +77,12 @@ class Vertexer : public framework::Producer { std::string trk_c_name_1{"TaggerTracks"}; std::string trk_c_name_2{"RecoilTracks"}; std::shared_ptr propagator_; - // double processing_time_{0.}; // Monitoring histograms - TH1F* h_delta_d0; TH1F* h_delta_z0; TH1F* h_delta_p; - ; + TH1F* h_delta_phi; TH1F* h_delta_theta; @@ -93,8 +91,6 @@ class Vertexer : public framework::Producer { TH2F* h_td0_vs_rd0; TH2F* h_tz0_vs_rz0; - - // pT and photon direction }; } // namespace reco diff --git a/Tracking/include/Tracking/Sim/LdmxSpacePoint.h b/Tracking/include/Tracking/Sim/LdmxSpacePoint.h index a4bea901c..7bfffb702 100644 --- a/Tracking/include/Tracking/Sim/LdmxSpacePoint.h +++ b/Tracking/include/Tracking/Sim/LdmxSpacePoint.h @@ -133,8 +133,6 @@ class LdmxSpacePoint { float m_varianceR; float m_varianceZ; int m_id; - // m_surfaceId is unused, should it be? FIXME - // int m_surfaceId; int m_layer; }; diff --git a/Tracking/include/Tracking/Sim/SeedToTrackParamMaker.ipp b/Tracking/include/Tracking/Sim/SeedToTrackParamMaker.ipp index 794bae2fe..8fb3281ff 100644 --- a/Tracking/include/Tracking/Sim/SeedToTrackParamMaker.ipp +++ b/Tracking/include/Tracking/Sim/SeedToTrackParamMaker.ipp @@ -202,9 +202,6 @@ bool SeedToTrackParamMaker::FitSeedAtlas( double y0 = sp[0]->y(); double z0 = sp[0]->z(); - // r0 is unused, should it be? FIXME - // double r0 = sqrt(x0 * x0 + y0 * y0); - double x1 = sp[1]->x() - x0; double y1 = sp[1]->y() - y0; double x2 = sp[2]->x() - x0; diff --git a/Tracking/src/Tracking/Reco/SeedFinderProcessor.cxx b/Tracking/src/Tracking/Reco/SeedFinderProcessor.cxx index 60b23db0a..126539c5c 100644 --- a/Tracking/src/Tracking/Reco/SeedFinderProcessor.cxx +++ b/Tracking/src/Tracking/Reco/SeedFinderProcessor.cxx @@ -216,10 +216,6 @@ ldmx::Track SeedFinderProcessor::SeedTracker( // Fit a straight line in the non-bending plane and a parabola in the bending // plane - // N is unused, should it be? FIXME - // also it should be not a one-letter variable - // const int N = vmeas.size(); - // Each measurement is treated as a 3D point, where the v direction is in the // center of the strip with sigma equal to the length of the strip / sqrt(12). // In this way it's easier to incorporate the tagger track extrapolation to @@ -296,7 +292,6 @@ ldmx::Track SeedFinderProcessor::SeedTracker( Acts::ActsVector<5> hlx = Acts::ActsVector<5>::Zero(); Acts::ActsVector<3> ref{0., 0., 0.}; - LineParabolaToHelix(B, hlx, ref); double relativePerigeeX = perigee_location(0) - xOrigin; @@ -386,40 +381,6 @@ ldmx::Track SeedFinderProcessor::SeedTracker( return trk; } -// To augment with the covariance matrix. -// The following formulas only works if the center if x0 where the -// linear+parabola function is evaluated is 0. It also assumes the axes -// orientation according to the ACTS needs. phi0 is the angle of the tangent of -// the track at the perigee. Positive counterclockwise. Theta is positive from -// the z axis to the bending plane. (pi/2 for tracks along the x axis) - -void SeedFinderProcessor::LineParabolaToHelix( - const Acts::ActsVector<5> parameters, Acts::ActsVector<5>& helix_parameters, - Acts::Vector3 ref) { - /* - * this function only made a printout, but the prinout was commented out - * FIXME - double R = 0.5 / abs(parameters(2)); - double xc = R * parameters(1); - double p2 = 0.5 * parameters(1) * parameters(1); - double factor = parameters(2) < 0 ? -1 : 1; - // plus or minus solution. I chose beta0 - R ( ... ), - // because the curvature is negative for electrons. - // The sign need to be chosen by the sign of B(2) - double yc = parameters(0) + factor *(R * (1 - p2)); - double theta0 = atan2(yc, xc); - double k = parameters(2) < 0 ? (-1. / R) : 1. / R; - double d0 = R - xc / cos(theta0); - double phi0 = theta0 + 1.57079632679; - double tanL = parameters(4) * cos(theta0); - double theta = 1.57079632679 - atan(tanL); - double z0 = parameters(3) + (d0 * tanL * tan(theta0)); - double qOp = factor / (0.3 * bfield_ * R * 0.001); - - std::cout<cd(); // outputTree_->Write(); @@ -474,17 +435,9 @@ bool SeedFinderProcessor::GroupStrips( void SeedFinderProcessor::FindSeedsFromMap(ldmx::Tracks& seeds, const ldmx::Measurements& pmeas) { - // These seemd like code dupplication and is not needed FIXME - // std::vector meas_for_seeds; - // meas_for_seeds.reserve(5); - std::map>::iterator groups_iter = groups_map.begin(); - // meas_iter is unused, should it be? FIXME - // std::vector::iterator meas_iter; - // Vector of iterators - constexpr size_t K = 5; std::vector::iterator> it; it.reserve(K); @@ -580,8 +533,8 @@ void SeedFinderProcessor::FindSeedsFromMap(ldmx::Tracks& seeds, // A seed is rejected if it is found incompatible with all the target // extrapolations - // This is set but unused, FIXME - // bool tgt_compatible = false; + // This is set but unused, eventually we will use tagger track position at + // target to inform recoil tracking bool tgt_compatible = false; for (auto tgt_pseudomeas : pmeas) { // The d0/z0 are in a frame with the same orientation of the target // surface