Apply suggestions from Tom's code review

Co-authored-by: Tom Eichlersmith <31970302+tomeichlersmith@users.noreply.github.com>
LDMX-Software · Jun 3, 2024 · 04662ac · 04662ac
1 parent e0c2b23
commit 04662ac
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 11 deletions.
diff --git a/Biasing/include/Biasing/DeepEcalProcessFilter.h b/Biasing/include/Biasing/DeepEcalProcessFilter.h
@@ -75,7 +75,10 @@ class DeepEcalProcessFilter : public simcore::UserAction {
   bool require_photon_fromTarget_{false};
   /// Enable logging
   enableLogging("DeepEcalProcessFilter")
-
+  /// member used to help tag events where the photon comes from the target
+  bool photonFromTarget_{false};
+  /// member used to help tag events that have a deep-ecal process ocurr
+  bool hasDeepEcalProcess_{false};
 };  // DeepEcalProcessFilter
 }  // namespace biasing
 

diff --git a/Biasing/src/Biasing/DeepEcalProcessFilter.cxx b/Biasing/src/Biasing/DeepEcalProcessFilter.cxx
@@ -17,9 +17,6 @@
 
 namespace biasing {
 
-bool hasProcessNeeded_{false};
-bool photonFromTarget_{false};
-bool hasDeepEcalProcess_{false};
 
 DeepEcalProcessFilter::DeepEcalProcessFilter(const std::string& name,
                                      framework::config::Parameters& parameters)
@@ -37,8 +34,6 @@ void DeepEcalProcessFilter::BeginOfEventAction(const G4Event* event) {
 }
 
 void DeepEcalProcessFilter::stepping(const G4Step* step) {
-  hasProcessNeeded_ = false;
-
   // Get the track associated with this step.
   auto track{step->GetTrack()};
 
@@ -78,14 +73,17 @@ void DeepEcalProcessFilter::stepping(const G4Step* step) {
   }
 
   // Tag if the event has the processes we are looking for
+  bool hasProcessNeeded{false};
   for (auto& process : processes_) {
     // ldmx_log(debug) << "Allowed processed " << process << " now we have " << processName;
     if (processName.contains(process)) {
-      hasProcessNeeded_ = true;
+      hasProcessNeeded = true;
       break;
     }
   }
-
+  // skip this step if it does not have any of the processes needed
+  if (not hasProcessNeeded) return;
+
   // isInEcal should be taken from
   // simcore::logical_volume_tests::isInEcal(volume) but for now it's under
   // its own namespace so I cannot reach it here, see issue
@@ -99,7 +97,7 @@ void DeepEcalProcessFilter::stepping(const G4Step* step) {
 
   // Skip this step if it does not have the processes needed
   // or if it's not in the ECAL
-  if (!hasProcessNeeded_ or !isInEcal) return;
+  if (not isInEcal) return;
 
   // Check the z position of the particle, and
   // flag if it is deeper than the min Z we are considering (but in ECAL)

diff --git a/Biasing/test/resim_ecal_conv_deep.py b/Biasing/test/resim_ecal_conv_deep.py
@@ -10,6 +10,7 @@
 from LDMX.Biasing import filters
 
 det = 'ldmx-det-v14-8gev'
+# Please make sure that the simulator here matches the simulator in the original config
 #mysim = ecal.deep_photo_nuclear(det, generators.single_8gev_e_upstream_tagger(), bias_threshold = 5010., processes=['conv','phot)'], ecal_min_Z = 300.)
 mysim = ecal.deep_photo_nuclear(det, generators.single_8gev_e_upstream_tagger(), bias_threshold = 3000., processes=['conv','phot)'], ecal_min_Z = 400.)
 mysim.description = "ECal Deep Donversion Test Re-Simulation"

diff --git a/DQM/include/DQM/SampleValidation.h b/DQM/include/DQM/SampleValidation.h
@@ -20,8 +20,6 @@ class SampleValidation : public framework::Analyzer {
     int pdgid_label(const int pdgid);
     /// Method executed before processing of events begins.
     void onProcessStart() override;
-    private:
-
 };
 } // namespace dqm