refactor: no return for add* python helpers (#1404)

after working on the full chain doc I wondered why we do `s = addSomething(s, ...)` all the time. to me it looks kind of confusing as the reader might think `s` is changing every time we add something. maybe we want to remove the return value of the add functions completely? @paulgessinger @timadye
acts-project · Aug 16, 2022 · afb91a5 · afb91a5
1 parent 4e9aa53
commit afb91a5
Show file tree

Hide file tree

Showing 17 changed files with 80 additions and 80 deletions.
diff --git a/CI/physmon/physmon.py b/CI/physmon/physmon.py
@@ -111,7 +111,7 @@
 
         rnd = acts.examples.RandomNumbers(seed=42)
 
-        s = addParticleGun(
+        addParticleGun(
             s,
             EtaConfig(-4.0, 4.0),
             ParticleConfig(4, acts.PdgParticle.eMuon, True),
@@ -120,22 +120,22 @@
             rnd=rnd,
         )
 
-        s = addFatras(
+        addFatras(
             s,
             trackingGeometry,
             field,
             rnd=rnd,
         )
 
-        s = addDigitization(
+        addDigitization(
             s,
             trackingGeometry,
             field,
             digiConfigFile=digiConfig,
             rnd=rnd,
         )
 
-        s = addSeeding(
+        addSeeding(
             s,
             trackingGeometry,
             field,
@@ -166,7 +166,7 @@
             rnd=rnd,  # only used by SeedingAlgorithm.TruthSmeared
         )
 
-        s = addCKFTracks(
+        addCKFTracks(
             s,
             trackingGeometry,
             field,

diff --git a/Examples/Python/python/acts/examples/reconstruction.py b/Examples/Python/python/acts/examples/reconstruction.py
@@ -140,7 +140,7 @@ def addSeeding(
     outputDirRoot: Optional[Union[Path, str]] = None,
     logLevel: Optional[acts.logging.Level] = None,
     rnd: Optional[acts.examples.RandomNumbers] = None,
-) -> acts.examples.Sequencer:
+) -> None:
     """This function steers the seeding
     Parameters
     ----------
@@ -545,7 +545,7 @@ def addKalmanTracks(
     field: acts.MagneticFieldProvider,
     directNavigation=False,
     reverseFilteringMomThreshold=0 * u.GeV,
-):
+) -> None:
     truthTrkFndAlg = acts.examples.TruthTrackFinder(
         level=acts.logging.INFO,
         inputParticles="truth_seeds_selected",
@@ -600,7 +600,7 @@ def addTruthTrackingGsf(
     s: acts.examples.Sequencer,
     trackingGeometry: acts.TrackingGeometry,
     field: acts.MagneticFieldProvider,
-):
+) -> None:
     gsfOptions = {
         "maxComponents": 12,
         "abortOnError": False,
@@ -645,7 +645,7 @@ def addCKFTracks(
     outputDirCsv: Optional[Union[Path, str]] = None,
     outputDirRoot: Optional[Union[Path, str]] = None,
     selectedParticles: str = "truth_seeds_selected",
-) -> acts.examples.Sequencer:
+) -> None:
     """This function steers the seeding
 
     Parameters
@@ -762,7 +762,7 @@ def addExaTrkx(
     geometrySelection: Union[Path, str],
     onnxModelDir: Union[Path, str],
     outputDirRoot: Optional[Union[Path, str]] = None,
-) -> acts.examples.Sequencer:
+) -> None:
 
     # Run the particle selection
     # The pre-selection will select truth particles satisfying provided criteria
@@ -843,7 +843,7 @@ def addVertexFitting(
     trackParameters: str = "trackparameters",
     vertexFinder: VertexFinder = VertexFinder.Truth,
     logLevel: Optional[acts.logging.Level] = None,
-):
+) -> None:
     """This function steers the vertex fitting
 
     Parameters

diff --git a/Examples/Python/python/acts/examples/simulation.py b/Examples/Python/python/acts/examples/simulation.py
@@ -3,10 +3,8 @@
 from collections import namedtuple
 from collections.abc import Iterable
 
-
 import acts
 from acts.examples import (
-    Sequencer,
     RandomNumbers,
     EventGenerator,
     FixedMultiplicityGenerator,
@@ -55,7 +53,7 @@
     particleConfig=ParticleConfig,
 )
 def addParticleGun(
-    s: Sequencer,
+    s: acts.examples.Sequencer,
     outputDirCsv: Optional[Union[Path, str]] = None,
     outputDirRoot: Optional[Union[Path, str]] = None,
     momentumConfig: MomentumConfig = MomentumConfig(),
@@ -66,7 +64,7 @@ def addParticleGun(
     vtxGen: Optional[EventGenerator.VertexGenerator] = None,
     printParticles: bool = False,
     rnd: Optional[RandomNumbers] = None,
-) -> Sequencer:
+) -> None:
     """This function steers the particle generation using the particle gun
 
     Parameters
@@ -183,12 +181,9 @@ def addPythia8(
     outputDirCsv: Optional[Union[Path, str]] = None,
     outputDirRoot: Optional[Union[Path, str]] = None,
     printParticles: bool = False,
-    returnEvGen: bool = False,
-) -> Union[acts.examples.Sequencer, acts.examples.EventGenerator]:
+) -> None:
     """This function steers the particle generation using Pythia8
 
-    NB. this is a reimplementation of common.addPythia8, which is maintained for now for compatibility.
-
     Parameters
     ----------
     s: Sequencer
@@ -211,9 +206,6 @@ def addPythia8(
         the output folder for the Root output, None triggers no output
     printParticles : bool, False
         print generated particles
-    returnEvGen: bool, False
-        returns EventGenerator instead of Sequencer.
-        This option  is included for compatibility and will be removed when common.addPythia8 is removed.
     """
 
     if int(s.config.logLevel) <= int(acts.logging.DEBUG):
@@ -305,7 +297,7 @@ def addPythia8(
             )
         )
 
-    return evGen if returnEvGen else s
+    return s
 
 
 @acts.examples.NamedTypeArgs(
@@ -319,7 +311,7 @@ def addFatras(
     outputDirRoot: Optional[Union[Path, str]] = None,
     rnd: Optional[acts.examples.RandomNumbers] = None,
     preselectParticles: Optional[ParticleSelectorConfig] = ParticleSelectorConfig(),
-) -> acts.examples.Sequencer:
+) -> None:
     """This function steers the detector simulation using Fatras
 
     Parameters
@@ -404,7 +396,7 @@ def addSimWriters(
     inputSimHits: Optional[str] = None,
     outputDirCsv: Optional[Union[Path, str]] = None,
     outputDirRoot: Optional[Union[Path, str]] = None,
-) -> acts.examples.Sequencer:
+) -> None:
     if outputDirCsv is not None:
         outputDirCsv = Path(outputDirCsv)
         if not outputDirCsv.exists():
@@ -480,7 +472,7 @@ def addGeant4(
     outputDirRoot: Optional[Union[Path, str]] = None,
     seed: Optional[int] = None,
     preselectParticles: bool = True,
-) -> acts.examples.Sequencer:
+) -> None:
     """This function steers the detector simulation using Geant4
 
     Parameters

diff --git a/Examples/Python/tests/test_examples.py b/Examples/Python/tests/test_examples.py
@@ -384,7 +384,7 @@ def test_itk_seeding(tmp_path, trk_geo, field, assert_root_hash):
         addDigitization,
     )
 
-    seq = addParticleGun(
+    addParticleGun(
         seq,
         MomentumConfig(1.0 * u.GeV, 10.0 * u.GeV, True),
         EtaConfig(-4.0, 4.0, True),
@@ -394,7 +394,7 @@ def test_itk_seeding(tmp_path, trk_geo, field, assert_root_hash):
         rnd=rnd,
     )
 
-    seq = addFatras(
+    addFatras(
         seq,
         trk_geo,
         field,
@@ -404,7 +404,7 @@ def test_itk_seeding(tmp_path, trk_geo, field, assert_root_hash):
     )
 
     srcdir = Path(__file__).resolve().parent.parent.parent.parent
-    seq = addDigitization(
+    addDigitization(
         seq,
         trk_geo,
         field,
@@ -423,7 +423,7 @@ def test_itk_seeding(tmp_path, trk_geo, field, assert_root_hash):
     )
     from acts.examples.itk import itkSeedingAlgConfig
 
-    seq = addSeeding(
+    addSeeding(
         seq,
         trk_geo,
         field,
@@ -434,7 +434,9 @@ def test_itk_seeding(tmp_path, trk_geo, field, assert_root_hash):
         / "Examples/Algorithms/TrackFinding/share/geoSelection-genericDetector.json",
         inputParticles="particles_final",  # use this to reproduce the original root_file_hashes.txt - remove to fix
         outputDirRoot=str(tmp_path),
-    ).run()
+    )
+
+    seq.run()
 
     del seq
 

diff --git a/Examples/Scripts/Python/ckf_tracks.py b/Examples/Scripts/Python/ckf_tracks.py
@@ -53,7 +53,7 @@ def runCKFTracks(
     outputDir = Path(outputDir)
 
     if inputParticlePath is None:
-        s = addParticleGun(
+        addParticleGun(
             s,
             EtaConfig(-2.0, 2.0),
             ParticleConfig(4, acts.PdgParticle.eMuon, True),
@@ -75,22 +75,22 @@ def runCKFTracks(
             )
         )
 
-    s = addFatras(
+    addFatras(
         s,
         trackingGeometry,
         field,
         rnd=rnd,
     )
 
-    s = addDigitization(
+    addDigitization(
         s,
         trackingGeometry,
         field,
         digiConfigFile=digiConfigFile,
         rnd=rnd,
     )
 
-    s = addSeeding(
+    addSeeding(
         s,
         trackingGeometry,
         field,
@@ -119,7 +119,7 @@ def runCKFTracks(
         rnd=rnd,  # only used by SeedingAlgorithm.TruthSmeared
     )
 
-    s = addCKFTracks(
+    addCKFTracks(
         s,
         trackingGeometry,
         field,

diff --git a/Examples/Scripts/Python/digitization.py b/Examples/Scripts/Python/digitization.py
@@ -34,7 +34,7 @@ def configureDigitization(
     rnd = acts.examples.RandomNumbers(seed=42)
 
     if particlesInput is None:
-        s = addParticleGun(
+        addParticleGun(
             s,
             EtaConfig(-2.0, 2.0),
             ParticleConfig(4, acts.PdgParticle.eMuon, True),
@@ -53,15 +53,15 @@ def configureDigitization(
         s.addReader(evGen)
 
     outputDir = Path(outputDir)
-    s = addFatras(
+    addFatras(
         s,
         trackingGeometry,
         field,
         rnd=rnd,
     )
     from acts.examples.simulation import addDigitization
 
-    s = addDigitization(
+    addDigitization(
         s,
         trackingGeometry,
         field,

diff --git a/Examples/Scripts/Python/exatrkx.py b/Examples/Scripts/Python/exatrkx.py
@@ -52,6 +52,6 @@
         s=s,
     )
 
-    s = addExaTrkx(s, trackingGeometry, geometrySelection, onnxdir, outputDir)
+    addExaTrkx(s, trackingGeometry, geometrySelection, onnxdir, outputDir)
 
     s.run()
diff --git a/Examples/Scripts/Python/fatras.py b/Examples/Scripts/Python/fatras.py
@@ -12,20 +12,21 @@ def runFatras(trackingGeometry, field, outputDir, s: acts.examples.Sequencer = N
     s = s or acts.examples.Sequencer(events=100, numThreads=-1)
     s.config.logLevel = acts.logging.INFO
     rnd = acts.examples.RandomNumbers()
-    s = addParticleGun(
+    addParticleGun(
         s,
         EtaConfig(-2.0, 2.0),
         rnd=rnd,
     )
     outputDir = Path(outputDir)
-    return addFatras(
+    addFatras(
         s,
         trackingGeometry,
         field,
         outputDirCsv=outputDir / "csv",
         outputDirRoot=outputDir,
         rnd=rnd,
     )
+    return s
 
 
 if "__main__" == __name__:

diff --git a/Examples/Scripts/Python/full_chain_itk.py b/Examples/Scripts/Python/full_chain_itk.py
@@ -33,15 +33,15 @@
 from acts.examples.itk import itkSeedingAlgConfig
 
 s = acts.examples.Sequencer(events=100, numThreads=-1)
-s = addParticleGun(
+addParticleGun(
     s,
-    MomentumConfig(1.0 * u.GeV, 10.0 * u.GeV, True),
-    EtaConfig(-4.0, 4.0, True),
-    ParticleConfig(1, acts.PdgParticle.eMuon, True),
+    MomentumConfig(1.0 * u.GeV, 10.0 * u.GeV, transverse=True),
+    EtaConfig(-4.0, 4.0, uniform=True),
+    ParticleConfig(1, acts.PdgParticle.eMuon, randomizeCharge=True),
     rnd=rnd,
 )
 # # Uncomment addPythia8 and ParticleSelectorConfig, instead of addParticleGun, to generate ttbar with mu=200 pile-up.
-# s = addPythia8(
+# addPythia8(
 #     s,
 #     hardProcess=["Top:qqbar2ttbar=on"],
 #     vtxGen=acts.examples.GaussianVertexGenerator(
@@ -51,23 +51,23 @@
 #     rnd=rnd,
 #     outputDirRoot=outputDir,
 # )
-s = addFatras(
+addFatras(
     s,
     trackingGeometry,
     field,
     # ParticleSelectorConfig(eta=(-4.0, 4.0), pt=(150 * u.MeV, None), removeNeutral=True),
     outputDirRoot=outputDir,
     rnd=rnd,
 )
-s = addDigitization(
+addDigitization(
     s,
     trackingGeometry,
     field,
     digiConfigFile=geo_dir / "itk-hgtd/itk-smearing-config.json",
     outputDirRoot=outputDir,
     rnd=rnd,
 )
-s = addSeeding(
+addSeeding(
     s,
     trackingGeometry,
     field,
@@ -78,7 +78,7 @@
     geoSelectionConfigFile=geo_dir / "itk-hgtd/geoSelection-ITk.json",
     outputDirRoot=outputDir,
 )
-s = addCKFTracks(
+addCKFTracks(
     s,
     trackingGeometry,
     field,