From be7a6530c74b9363e2595ed23ad82da9609c999b Mon Sep 17 00:00:00 2001
From: John Derek Chapman <chapman@hep.phy.cam.ac.uk>
Date: Wed, 5 Jul 2023 12:19:07 +0200
Subject: [PATCH] SimKernelMT: avoid separate ToolHandle for ParticleKiller
 (ATLASSIM-6535)

SimKernelMT: avoid separate ToolHandle for ParticleKiller (ATLASSIM-6535)
---
 .../ISF/ISF_Config/python/ISF_MainConfig.py   |  6 --
 .../ISF_Config/python/ISF_MainConfigLegacy.py |  1 -
 .../ISF_Algorithms/src/SimKernelMT.cxx        | 18 ++---
 .../ISF_Core/ISF_Algorithms/src/SimKernelMT.h |  2 +-
 .../ISF_Algorithms/test/SimKernelMT_test.cxx  | 76 +++++++++++++++----
 5 files changed, 69 insertions(+), 34 deletions(-)

diff --git a/Simulation/ISF/ISF_Config/python/ISF_MainConfig.py b/Simulation/ISF/ISF_Config/python/ISF_MainConfig.py
index bf9c15520617..aa41e077303e 100644
--- a/Simulation/ISF/ISF_Config/python/ISF_MainConfig.py
+++ b/Simulation/ISF/ISF_Config/python/ISF_MainConfig.py
@@ -59,12 +59,6 @@ def Kernel_GenericSimulatorMTCfg(flags, name="ISF_Kernel_GenericSimulatorMT", **
         from ISF_HepMC_Tools.ISF_HepMC_ToolsConfig import TruthPreselectionToolCfg
         kwargs.setdefault( "TruthPreselectionTool", acc.popToolsAndMerge(TruthPreselectionToolCfg(flags)) )
 
-    if "ParticleKillerTool" not in kwargs:
-        tool = acc.popToolsAndMerge(ParticleKillerToolCfg(flags))
-        acc.addPublicTool(tool)
-        pubTool = acc.getPublicTool(tool.name)
-        kwargs.setdefault("ParticleKillerTool", pubTool) # public toolHandle
-
     if "GeoIDSvc" not in kwargs:
         kwargs.setdefault("GeoIDSvc", acc.getPrimaryAndMerge(GeoIDSvcCfg(flags)).name)
 
diff --git a/Simulation/ISF/ISF_Config/python/ISF_MainConfigLegacy.py b/Simulation/ISF/ISF_Config/python/ISF_MainConfigLegacy.py
index 14b593434f47..0cf68a43c063 100644
--- a/Simulation/ISF/ISF_Config/python/ISF_MainConfigLegacy.py
+++ b/Simulation/ISF/ISF_Config/python/ISF_MainConfigLegacy.py
@@ -203,7 +203,6 @@ def getKernel_GenericSimulator(name="ISF_Kernel_GenericSimulator", **kwargs):
 def getKernel_GenericSimulatorMT(name="ISF_Kernel_GenericSimulatorMT", **kwargs):
     kwargs.setdefault("InputEvgenCollection", "BeamTruthEvent" )
     kwargs.setdefault("OutputTruthCollection", "TruthEvent" )
-    kwargs.setdefault("ParticleKillerTool", "ISF_ParticleKillerTool" )
     kwargs.setdefault("GeoIDSvc", "ISF_GeoIDSvc" )
     if ISF_Flags.Simulator.isQuasiStable():
         kwargs.setdefault('InputConverter', 'ISF_LongLivedInputConverter')
diff --git a/Simulation/ISF/ISF_Core/ISF_Algorithms/src/SimKernelMT.cxx b/Simulation/ISF/ISF_Core/ISF_Algorithms/src/SimKernelMT.cxx
index 520456e45d1b..176594f1721b 100644
--- a/Simulation/ISF/ISF_Core/ISF_Algorithms/src/SimKernelMT.cxx
+++ b/Simulation/ISF/ISF_Core/ISF_Algorithms/src/SimKernelMT.cxx
@@ -57,20 +57,18 @@ StatusCode ISF::SimKernelMT::initialize() {
       }
       // New flavour add it to the map.
       m_simToolMap[flavor] = &*curSimTool;
+      if (flavor == ISF::ParticleKiller) {
+        m_particleKillerTool = &*curSimTool;
+      }
     }
   }
-  ATH_CHECK( m_particleKillerTool.retrieve() );
-  const ISF::SimulationFlavor pkFlavor = m_particleKillerTool->simFlavor();
-  if ( m_simToolMap.find(pkFlavor) == m_simToolMap.end() )
-  {
-    m_simToolMap[pkFlavor] = &*m_particleKillerTool;
-  }
-  else
-  {
-    ATH_MSG_WARNING("Two ISimulatorTool instances (" << m_simToolMap.find(ISF::ParticleKiller)->second->name() << "," << m_particleKillerTool->name() << ") with the same flavor in this job!\n Check your configuration!");
+  // Check that a particle killer simulator tool was provided
+  if (!m_particleKillerTool) {
+    ATH_MSG_FATAL("No fallback ParticleKiller Simulator Tool provided in SimulationTools, the job will bail out now.");
+    return StatusCode::FAILURE;
   }
 
-  ATH_MSG_INFO("The following Simulators will be used in this job: \t" << m_simulationTools << "\n" << m_particleKillerTool);
+  ATH_MSG_INFO("The following Simulators will be used in this job: \t" << m_simulationTools);
   // retrieve simulation selectors (i.e. the "routing chain")
   for ( auto& selectorsToolHandleArray: m_simSelectors )
   {
diff --git a/Simulation/ISF/ISF_Core/ISF_Algorithms/src/SimKernelMT.h b/Simulation/ISF/ISF_Core/ISF_Algorithms/src/SimKernelMT.h
index d76e44715187..865fcd36ac2f 100644
--- a/Simulation/ISF/ISF_Core/ISF_Algorithms/src/SimKernelMT.h
+++ b/Simulation/ISF/ISF_Core/ISF_Algorithms/src/SimKernelMT.h
@@ -116,7 +116,7 @@ private:
   ToolHandleArray<ISimulatorTool> m_simulationTools{this, "SimulationTools", {}, ""};
 
   /// When no appropriate simulator can be found for a given particle, the particle is sent to this "particle killer":
-  PublicToolHandle<ISimulatorTool> m_particleKillerTool{this, "ParticleKillerTool", "", ""};
+  ISimulatorTool *m_particleKillerTool{};
 
   /// AthenaTool responsible for writing Calo/Muon Entry/Exit Layer collection
   PublicToolHandle<IEntryLayerTool> m_entryLayerTool{this, "EntryLayerTool", "ISF_EntryLayerToolMT", ""};
diff --git a/Simulation/ISF/ISF_Core/ISF_Algorithms/test/SimKernelMT_test.cxx b/Simulation/ISF/ISF_Core/ISF_Algorithms/test/SimKernelMT_test.cxx
index d9d573f3fbc6..7f380bc314c4 100644
--- a/Simulation/ISF/ISF_Core/ISF_Algorithms/test/SimKernelMT_test.cxx
+++ b/Simulation/ISF/ISF_Core/ISF_Algorithms/test/SimKernelMT_test.cxx
@@ -197,6 +197,37 @@ public:
 
 DECLARE_COMPONENT( MockSimulatorTool )
 
+const std::string mockParticleKillerToolName = "ISFTesting::MockParticleKillerTool/MyTestParticleKillerTool";
+
+class MockParticleKillerTool : public extends<AthAlgTool, ISF::ISimulatorTool> {
+
+public:
+  MockParticleKillerTool(const std::string& type, const std::string& name, const IInterface* svclocator)
+    : base_class(type, name, svclocator)
+  {
+  };
+  virtual ~MockParticleKillerTool() { };
+
+  MOCK_METHOD0(finalize, StatusCode());
+  MOCK_METHOD1(setupEvent, StatusCode(const EventContext&));
+  MOCK_METHOD3(simulate, StatusCode(ISF::ISFParticle&, ISF::ISFParticleContainer&, McEventCollection*));
+  MOCK_METHOD4(simulateVector, StatusCode(const ISF::ISFParticleVector&, ISF::ISFParticleContainer&, McEventCollection*, McEventCollection*));
+  MOCK_METHOD1(releaseEvent, StatusCode(const EventContext&));
+  MOCK_CONST_METHOD1(bid, int(const ISF::ISFParticle&));
+
+  // dummy methods implementing in pure virtual interface methods (to make class non-abstract)
+  virtual StatusCode initialize() {
+    ATH_MSG_INFO ("initializing MockParticleKillerTool: " << name());
+    return StatusCode::SUCCESS;
+  };
+  virtual StatusCode setupEventST() { return StatusCode::FAILURE; };
+  virtual StatusCode releaseEventST() { return StatusCode::FAILURE; };
+  virtual ISF::SimulationFlavor simFlavor() const { return ISF::ParticleKiller; };
+
+}; // MockParticleKillerTool
+
+DECLARE_COMPONENT( MockParticleKillerTool )
+
 
 // Athena Tool to mock a SimulatorTool
 // //
@@ -291,7 +322,7 @@ protected:
       // the tested AthAlgorithm
       m_alg = new ISF::SimKernelMT{"SimKernelMT", m_svcLoc};
       m_alg->addRef();
-      EXPECT_TRUE( m_alg->setProperty("ParticleKillerTool", particleKillerSimulatorToolName).isSuccess() );
+      EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+particleKillerSimulatorToolName+"']").isSuccess() );
       EXPECT_TRUE( m_alg->setProperty("GeoIDSvc", mockGeoIDSvcName).isSuccess() );
       EXPECT_TRUE( m_alg->setProperty("TruthRecordService", mockTruthSvcName).isSuccess() );
       EXPECT_TRUE( m_alg->setProperty("EntryLayerTool", mockEntryLayerToolName).isSuccess() );
@@ -301,6 +332,7 @@ protected:
       m_mockTruthSvc = retrieveService<MockTruthSvc>(mockTruthSvcName);
       m_mockInputConverter = retrieveService<MockInputConverter>(mockInputConverterName);
       m_mockSimulatorTool = retrieveTool<MockSimulatorTool>(mockSimulatorToolName);
+      m_mockParticleKillerTool = retrieveTool<MockParticleKillerTool>(mockParticleKillerToolName);
       m_mockSimulationSelector = retrieveTool<MockSimulationSelector>(mockSimulationSelectorName);
       m_mockEntryLayerTool = retrieveTool<MockEntryLayerTool>(mockEntryLayerToolName);
       ASSERT_TRUE( m_svcLoc->service("StoreGateSvc", m_sg) );
@@ -388,6 +420,11 @@ protected:
       return m_alg->m_simulationTools;
     }
 
+    ISF::ISimulatorTool* getParticleKillerTool() const
+    {
+      return m_alg->m_particleKillerTool;
+    }
+
     // the tested AthAlgorithm
     ISF::SimKernelMT* m_alg;
 
@@ -398,6 +435,7 @@ protected:
     ISFTesting::MockTruthSvc* m_mockTruthSvc = nullptr;
     ISFTesting::MockInputConverter* m_mockInputConverter = nullptr;
     ISFTesting::MockSimulatorTool* m_mockSimulatorTool = nullptr;
+    ISFTesting::MockParticleKillerTool* m_mockParticleKillerTool = nullptr;
     ISFTesting::MockSimulationSelector* m_mockSimulationSelector = nullptr;
     ISFTesting::MockEntryLayerTool* m_mockEntryLayerTool = nullptr;
 
@@ -599,8 +637,15 @@ protected:
   }
 
 
+  TEST_F(SimKernelMT_test, specifyTwoClashingSimulationTools_expectInitializeFailure) {
+    EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+mockParticleKillerToolName+"','"+particleKillerSimulatorToolName+"']").isSuccess() );
+    this->setEmptyInputOutputCollections();
+    EXPECT_TRUE( m_alg->initialize().isFailure() );
+  }
+
+
   TEST_F(SimKernelMT_test, specifyASimulationTool_expectInitializeSuccess) {
-    EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+mockSimulatorToolName+"']").isSuccess() );
+    EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+mockSimulatorToolName+"','"+particleKillerSimulatorToolName+"']").isSuccess() );
     this->setEmptyInputOutputCollections();
     EXPECT_TRUE( m_alg->initialize().isSuccess() );
 
@@ -633,7 +678,7 @@ protected:
 
 
   TEST_F(SimKernelMT_test, filledSimulationSelectors_expectInitializeSuccess) {
-    EXPECT_TRUE( m_alg->setProperty("ParticleKillerTool", mockSimulatorToolName).isSuccess() );
+    EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+mockSimulatorToolName+"','"+particleKillerSimulatorToolName+"']").isSuccess() );
     EXPECT_TRUE( m_alg->setProperty("BeamPipeSimulationSelectors", "['"+mockSimulationSelectorName+"']").isSuccess() ); //mockSimulationSelectorName);
     EXPECT_TRUE( m_alg->setProperty("IDSimulationSelectors", "['"+mockSimulationSelectorName+"']").isSuccess() );
     EXPECT_TRUE( m_alg->setProperty("CaloSimulationSelectors", "['"+mockSimulationSelectorName+"']").isSuccess() );
@@ -646,7 +691,7 @@ protected:
 
 
   TEST_F(SimKernelMT_test, identifySimulator_particleInsideInnerDetectorAndInnerDetectorSimulationSelectorAcceptingParticle_expectInnerDetectorSimulatorReturned) {
-    EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+mockSimulatorToolName+"']").isSuccess() );
+    EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+mockSimulatorToolName+"','"+particleKillerSimulatorToolName+"']").isSuccess() );
     EXPECT_TRUE( m_alg->setProperty("IDSimulationSelectors", "['"+mockSimulationSelectorName+"']").isSuccess() );
     this->setEmptyInputOutputCollections();
     EXPECT_TRUE( m_alg->initialize().isSuccess() );
@@ -671,7 +716,7 @@ protected:
     const auto* actualSimulatorToolPtr = &this->identifySimulator(particle);
 
     ToolHandleArray<ISF::ISimulatorTool>& simulatorTools = this->getSimulatorTools();
-    const unsigned int expectedSize(1);
+    const unsigned int expectedSize(2);
     ASSERT_EQ (simulatorTools.size(), expectedSize);
     ISFTesting::MockSimulatorTool* expectedSimulatorToolPtr = dynamic_cast<ISFTesting::MockSimulatorTool*>(&*(simulatorTools[0]));
     ASSERT_EQ(expectedSimulatorToolPtr, actualSimulatorToolPtr);
@@ -679,7 +724,7 @@ protected:
 
 
   TEST_F(SimKernelMT_test, identifySimulator_particleInsideInnerDetectorAndInnerDetectorSimulationSelectorNotAcceptingParticle_expectParticleKillerSimulatorReturned) {
-    EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+mockSimulatorToolName+"']").isSuccess() );
+    EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+mockSimulatorToolName+"','"+particleKillerSimulatorToolName+"']").isSuccess() );
     EXPECT_TRUE( m_alg->setProperty("IDSimulationSelectors", "['"+mockSimulationSelectorName+"']").isSuccess() );
     this->setEmptyInputOutputCollections();
     EXPECT_TRUE( m_alg->initialize().isSuccess() );
@@ -702,15 +747,14 @@ protected:
       .WillOnce(::testing::Return(false));
 
     const auto* actualSimulatorToolPtr = &this->identifySimulator(particle);
-    ToolHandle<ISF::ISimulatorTool> particleKillerSimulatorTool(particleKillerSimulatorToolName);
-    const ISF::ISimulatorTool* expectedSimulatorToolPtr = &*particleKillerSimulatorTool;
+    const ISF::ISimulatorTool* expectedSimulatorToolPtr = getParticleKillerTool();
 
     ASSERT_EQ(expectedSimulatorToolPtr, actualSimulatorToolPtr);
   }
 
 
   TEST_F(SimKernelMT_test, identifySimulator_particleInsideCaloAndOnlyInnerDetectorSimulationSelectorPresent_expectSimulationSelectorNotCalledAndParticleKillerSimulatorReturned) {
-    EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+mockSimulatorToolName+"']").isSuccess() );
+    EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+mockSimulatorToolName+"','"+particleKillerSimulatorToolName+"']").isSuccess() );
     EXPECT_TRUE( m_alg->setProperty("IDSimulationSelectors", "['"+mockSimulationSelectorName+"']").isSuccess() );
     this->setEmptyInputOutputCollections();
     EXPECT_TRUE( m_alg->initialize().isSuccess() );
@@ -733,15 +777,14 @@ protected:
       .Times(0);
 
     const auto* actualSimulatorToolPtr = &this->identifySimulator(particle);
-    ToolHandle<ISF::ISimulatorTool> particleKillerSimulatorTool(particleKillerSimulatorToolName);
-    const ISF::ISimulatorTool* expectedSimulatorToolPtr = &*particleKillerSimulatorTool;
+    const ISF::ISimulatorTool* expectedSimulatorToolPtr = getParticleKillerTool();
 
     ASSERT_EQ(expectedSimulatorToolPtr, actualSimulatorToolPtr);
   }
 
 
   TEST_F(SimKernelMT_test, identifySimulator_particleInsideCaloAndCaloSimulationSelectorSelectingParticle_expectSimulatorReturned) {
-    EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+mockSimulatorToolName+"']").isSuccess() );
+    EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+mockSimulatorToolName+"','"+particleKillerSimulatorToolName+"']").isSuccess() );
     EXPECT_TRUE( m_alg->setProperty("CaloSimulationSelectors", "['"+mockSimulationSelectorName+"']").isSuccess() );
     this->setEmptyInputOutputCollections();
     EXPECT_TRUE( m_alg->initialize().isSuccess() );
@@ -765,7 +808,7 @@ protected:
 
     const auto* actualSimulatorToolPtr = &this->identifySimulator(particle);
     ToolHandleArray<ISF::ISimulatorTool>& simulatorTools = this->getSimulatorTools();
-    const unsigned int expectedSize(1);
+    const unsigned int expectedSize(2);
     ASSERT_EQ (simulatorTools.size(), expectedSize);
     ISFTesting::MockSimulatorTool* expectedSimulatorToolPtr = dynamic_cast<ISFTesting::MockSimulatorTool*>(&*(simulatorTools[0]));
     ASSERT_EQ(expectedSimulatorToolPtr, actualSimulatorToolPtr);
@@ -800,7 +843,7 @@ protected:
 
     EXPECT_TRUE( m_alg->setProperty("InputEvgenCollection", "testInputEvgenCollection").isSuccess() );
     EXPECT_TRUE( m_alg->setProperty("OutputTruthCollection", "testOutputTruthCollection").isSuccess() );
-    EXPECT_TRUE( m_alg->setProperty("ParticleKillerTool", mockSimulatorToolName).isSuccess() );
+    EXPECT_TRUE( m_alg->setProperty("SimulationTools", "['"+mockParticleKillerToolName+"']").isSuccess() );
     EXPECT_TRUE( m_alg->setProperty("InputConverter", fullInputConverter).isSuccess() );
     EXPECT_TRUE( m_alg->initialize().isSuccess() );
 
@@ -821,8 +864,9 @@ protected:
                                      truthBinding
                                        );
 
-    ASSERT_NE( m_mockSimulatorTool, nullptr );
-    EXPECT_CALL( *m_mockSimulatorTool, simulateVector(::testing::_,::testing::_,::testing::_,::testing::_) )
+    ASSERT_NE( m_mockParticleKillerTool, nullptr );
+    ISFTesting::MockParticleKillerTool *particleKillerTool = dynamic_cast<ISFTesting::MockParticleKillerTool*>(getParticleKillerTool());
+    EXPECT_CALL( *particleKillerTool, simulateVector(::testing::_,::testing::_,::testing::_,::testing::_) )
       .Times(1)
       .WillOnce(::testing::Return(StatusCode::SUCCESS));
 
-- 
GitLab