From 9df14972fc54b5d37fe43bef01c46a7a0f57349e Mon Sep 17 00:00:00 2001 From: Nils Krah Date: Fri, 20 Sep 2024 18:16:44 +0200 Subject: [PATCH 01/19] Implement setter_hook for output_filename for StatsActor and remove property --- opengate/actors/miscactors.py | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/opengate/actors/miscactors.py b/opengate/actors/miscactors.py index 4d1ac60a4..50fb1e63b 100644 --- a/opengate/actors/miscactors.py +++ b/opengate/actors/miscactors.py @@ -8,6 +8,15 @@ from ..exception import warning +def _setter_hook_output_filename(self, output_filename): + # By default, write_to_disk is False. + # However, if user actively sets the output_filename + # s/he most likely wants to write to disk also + if output_filename != "" and output_filename is not None: + self.write_to_disk = True + return output_filename + + class ActorOutputStatisticsActor(ActorOutputBase): """This is a hand-crafted ActorOutput specifically for the SimulationStatisticsActor.""" @@ -28,9 +37,9 @@ class ActorOutputStatisticsActor(ActorOutputBase): "auto", { "doc": "Filename for the data represented by this actor output. " - "Relative paths and filenames are taken " - "relative to the global simulation output folder " - "set via the Simulation.output_dir option. ", + "Relative paths and filenames are taken " + "relative to the global simulation output folder " + "set via the Simulation.output_dir option. ", }, ), "write_to_disk": ( @@ -215,16 +224,6 @@ def __str__(self): def counts(self): return self.user_output.stats.merged_data - @ActorBase.output_filename.setter - def output_filename(self, val): - # special behavior: - # By default write_to_disk is False. - # However, if user set the output_filename while it is the default (auto) - # we set write_to_disk to True. - if self.output_filename == "auto": - self.write_to_disk = True - ActorBase.output_filename.fset(self, val) - def store_output_data(self, output_name, run_index, *data): raise NotImplementedError @@ -311,7 +310,6 @@ def _setter_hook_particles(self, value): class SplittingActorBase(ActorBase): - # hints for IDE splitting_factor: int bias_primary_only: bool @@ -350,7 +348,6 @@ class SplittingActorBase(ActorBase): class ComptSplittingActor(SplittingActorBase, g4.GateOptrComptSplittingActor): - # hints for IDE weight_threshold: float min_weight_of_particle: float @@ -415,7 +412,6 @@ def initialize(self): class BremSplittingActor(SplittingActorBase, g4.GateBOptrBremSplittingActor): - # hints for IDE processes: list From 9f6765c9a2002d3bf39bad5e64c1b301ead331f8 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 20 Sep 2024 16:20:54 +0000 Subject: [PATCH 02/19] [pre-commit.ci] Automatic python and c++ formatting --- opengate/actors/miscactors.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/opengate/actors/miscactors.py b/opengate/actors/miscactors.py index 50fb1e63b..7e5ed62d4 100644 --- a/opengate/actors/miscactors.py +++ b/opengate/actors/miscactors.py @@ -37,9 +37,9 @@ class ActorOutputStatisticsActor(ActorOutputBase): "auto", { "doc": "Filename for the data represented by this actor output. " - "Relative paths and filenames are taken " - "relative to the global simulation output folder " - "set via the Simulation.output_dir option. ", + "Relative paths and filenames are taken " + "relative to the global simulation output folder " + "set via the Simulation.output_dir option. ", }, ), "write_to_disk": ( From 1d25696a6b804343fcf47a36d6fcbf5d66e52c43 Mon Sep 17 00:00:00 2001 From: Nils Krah Date: Fri, 20 Sep 2024 18:33:57 +0200 Subject: [PATCH 03/19] Implement a warning cache iin the Simulation class --- opengate/base.py | 4 ++++ opengate/engines.py | 14 +++++++++++--- opengate/managers.py | 6 ++++++ opengate/physics.py | 2 +- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/opengate/base.py b/opengate/base.py index 6ccc18072..838d8b776 100644 --- a/opengate/base.py +++ b/opengate/base.py @@ -550,6 +550,10 @@ def from_dictionary(self, d): f"In that case, simply ignore the warning. " ) + def warning(self, message): + self.simulation._warnings.append(message) + warning(message) + class DynamicGateObject(GateObject): diff --git a/opengate/engines.py b/opengate/engines.py index 7412b21cf..fae1fcb88 100644 --- a/opengate/engines.py +++ b/opengate/engines.py @@ -101,7 +101,7 @@ def initialize(self, run_timing_intervals, progress_bar=False): self.run_timing_intervals = run_timing_intervals assert_run_timing(self.run_timing_intervals) if len(self.simulation_engine.simulation.source_manager.user_info_sources) == 0: - warning(f"No source: no particle will be generated") + self.simulation_engine.simulation.warning(f"No source: no particle will be generated") self.progress_bar = progress_bar def initialize_actors(self): @@ -323,7 +323,7 @@ def initialize_global_cuts(self): # range if ui.energy_range_min is not None and ui.energy_range_max is not None: - warning(f"WARNING ! SetEnergyRange only works in MT mode") + self.simulation_engine.simulation.warning(f"WARNING ! SetEnergyRange only works in MT mode") pct = g4.G4ProductionCutsTable.GetProductionCutsTable() pct.SetEnergyRange(ui.energy_range_min, ui.energy_range_max) @@ -424,7 +424,7 @@ def initialize_optical_material_properties(self): self.g4_optical_material_tables[str(material_name)] ) else: - warning( + self.simulation_engine.simulation.warning( f"Could not load the optical material properties for material {material_name} " f"found in volume {vol.name} from file {self.physics_manager.optical_properties_file}." ) @@ -1153,6 +1153,14 @@ def run_engine(self): output.current_random_seed = self.current_random_seed output.expected_number_of_events = self.source_engine.expected_number_of_events + if len(self.simulation.warnings) > 0: + print("*" * 20) + print(f"{len(self.simulation.warnings)} warnings occurred in this simulation: ") + for w in self.simulation.warnings: + print(w) + print("-" * 10) + print("*" * 20) + return output def start_and_stop(self): diff --git a/opengate/managers.py b/opengate/managers.py index 1b31a0591..b53aa00cb 100644 --- a/opengate/managers.py +++ b/opengate/managers.py @@ -1422,6 +1422,8 @@ def __init__(self, name="simulation"): # read-only info self._current_random_seed = None + self._warnings = [] # list to store warning messages issued somewhere in the simulation + def __str__(self): s = ( @@ -1453,6 +1455,10 @@ def world(self): def current_random_seed(self): return self._current_random_seed + @property + def warnings(self): + return self._warnings + def to_dictionary(self): d = super().to_dictionary() d["volume_manager"] = self.volume_manager.to_dictionary() diff --git a/opengate/physics.py b/opengate/physics.py index 822a32b0b..76a01e5e9 100644 --- a/opengate/physics.py +++ b/opengate/physics.py @@ -87,7 +87,7 @@ def ConstructProcess(self): ] if len(particle_keys_to_consider) == 0: - warning( + self.physics_engine.simulation_engine.simulation.warning( "user_limits_particles is False for all particles. No tracking cuts will be applied. Use sim.physics_manager.set_user_limits_particles()." ) From 49f87ae5ae4f732a21b260fb3b4085e29c4a6a6e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 20 Sep 2024 16:41:11 +0000 Subject: [PATCH 04/19] [pre-commit.ci] Automatic python and c++ formatting --- opengate/engines.py | 12 +++++++++--- opengate/managers.py | 5 +++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/opengate/engines.py b/opengate/engines.py index fae1fcb88..6dea928f8 100644 --- a/opengate/engines.py +++ b/opengate/engines.py @@ -101,7 +101,9 @@ def initialize(self, run_timing_intervals, progress_bar=False): self.run_timing_intervals = run_timing_intervals assert_run_timing(self.run_timing_intervals) if len(self.simulation_engine.simulation.source_manager.user_info_sources) == 0: - self.simulation_engine.simulation.warning(f"No source: no particle will be generated") + self.simulation_engine.simulation.warning( + f"No source: no particle will be generated" + ) self.progress_bar = progress_bar def initialize_actors(self): @@ -323,7 +325,9 @@ def initialize_global_cuts(self): # range if ui.energy_range_min is not None and ui.energy_range_max is not None: - self.simulation_engine.simulation.warning(f"WARNING ! SetEnergyRange only works in MT mode") + self.simulation_engine.simulation.warning( + f"WARNING ! SetEnergyRange only works in MT mode" + ) pct = g4.G4ProductionCutsTable.GetProductionCutsTable() pct.SetEnergyRange(ui.energy_range_min, ui.energy_range_max) @@ -1155,7 +1159,9 @@ def run_engine(self): if len(self.simulation.warnings) > 0: print("*" * 20) - print(f"{len(self.simulation.warnings)} warnings occurred in this simulation: ") + print( + f"{len(self.simulation.warnings)} warnings occurred in this simulation: " + ) for w in self.simulation.warnings: print(w) print("-" * 10) diff --git a/opengate/managers.py b/opengate/managers.py index b53aa00cb..44f7cffb7 100644 --- a/opengate/managers.py +++ b/opengate/managers.py @@ -1422,8 +1422,9 @@ def __init__(self, name="simulation"): # read-only info self._current_random_seed = None - self._warnings = [] # list to store warning messages issued somewhere in the simulation - + self._warnings = ( + [] + ) # list to store warning messages issued somewhere in the simulation def __str__(self): s = ( From ac76bd8b9b1a691eb1d974189ef56c022fee35bf Mon Sep 17 00:00:00 2001 From: David Sarrut Date: Fri, 20 Sep 2024 18:40:40 +0200 Subject: [PATCH 05/19] update tolerance --- ...Nthreads.py => test041_dose_actor_mt_cp_images_n_threads.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename opengate/tests/src/{test041_dose_actor_mt_cp_images_Nthreads.py => test041_dose_actor_mt_cp_images_n_threads.py} (99%) diff --git a/opengate/tests/src/test041_dose_actor_mt_cp_images_Nthreads.py b/opengate/tests/src/test041_dose_actor_mt_cp_images_n_threads.py similarity index 99% rename from opengate/tests/src/test041_dose_actor_mt_cp_images_Nthreads.py rename to opengate/tests/src/test041_dose_actor_mt_cp_images_n_threads.py index 7a48754b3..97a607bfe 100755 --- a/opengate/tests/src/test041_dose_actor_mt_cp_images_Nthreads.py +++ b/opengate/tests/src/test041_dose_actor_mt_cp_images_n_threads.py @@ -121,7 +121,7 @@ def run_test(doseFpath_IDD_singleImage, doseFpath_IDD_NthreadImages, stat): expected_ratio, doseFpath_IDD_singleImage, doseFpath_IDD_NthreadImages, - abs_tolerance=0.03, + abs_tolerance=0.05, ) return is_ok From 96b1708c3967a32ae4ec345f7fe3b912c6996e15 Mon Sep 17 00:00:00 2001 From: David Sarrut Date: Fri, 20 Sep 2024 18:41:35 +0200 Subject: [PATCH 06/19] error message updated --- opengate/managers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengate/managers.py b/opengate/managers.py index 1b31a0591..08b164695 100644 --- a/opengate/managers.py +++ b/opengate/managers.py @@ -395,7 +395,7 @@ def dump_actor_types(self): def get_actor_user_info(self, name): warning( - f"Deprecation warning: This function will soon be removed." + f"Deprecation warning: The function 'get_actor_user_info' will soon be removed." f"Use my_actor.user_info instead, where 'my_actor' " f"should be replace by your actor object. " f"You can also access user input parameters directly, e.g. my_actor.attached_to=..." From b1c07364069d597352ff530836afd4c87393f7fe Mon Sep 17 00:00:00 2001 From: David Sarrut Date: Tue, 24 Sep 2024 20:58:26 +0200 Subject: [PATCH 07/19] Refactor digitizer actor to use properties for size and spacing Removed the setter hooks for size and spacing and replaced them with output_size and output_spacing properties. This change simplifies the code and maintains the functionality of ensuring that size and spacing are treated as 2D vectors with an appended third dimension of 1. --- opengate/actors/digitizers.py | 62 ++++++++++++++++------------------- 1 file changed, 28 insertions(+), 34 deletions(-) diff --git a/opengate/actors/digitizers.py b/opengate/actors/digitizers.py index baac7b6c7..7dc8c677f 100644 --- a/opengate/actors/digitizers.py +++ b/opengate/actors/digitizers.py @@ -762,28 +762,6 @@ def EndSimulationAction(self): g4.GateDigitizerHitsCollectionActor.EndSimulationAction(self) -def _setter_hook_size_projection_actor(self, size): - size = list(size) - if len(size) != 2: - fatal( - f"Error, the size must be a 2-vector (2D) while it is {size}. " - f"Note: The size along the third dimension is automatically set to 1." - ) - size.append(1) - return size - - -def _setter_hook_spacing_projection_actor(self, spacing): - spacing = list(spacing) - if len(spacing) != 2: - fatal( - f"Error, the spacing must be a 2-vector (2D) while it is {spacing}. " - f"Note: The spacing along the third dimension is automatically determined." - ) - spacing.append(1) - return spacing - - class DigitizerProjectionActor(ActorBase, g4.GateDigitizerProjectionActor): """ This actor takes as input HitsCollections and performed binning in 2D images. @@ -802,17 +780,11 @@ class DigitizerProjectionActor(ActorBase, g4.GateDigitizerProjectionActor): ), "spacing": ( [4 * g4_units.mm, 4 * g4_units.mm], - { - "doc": "FIXME", - "setter_hook": _setter_hook_spacing_projection_actor, - }, + {"doc": "FIXME"}, ), "size": ( [128, 128], - { - "doc": "FIXME", - "setter_hook": _setter_hook_size_projection_actor, - }, + {"doc": "FIXME"}, ), "physical_volume_index": ( 0, @@ -853,12 +825,34 @@ def initialize(self): f"Sorry, cannot (yet) use several attached_to volumes for " f"DigitizerProjectionActor {self.user_info.name}" ) - ActorBase.initialize(self) - self.InitializeUserInput(self.user_info) self.InitializeCpp() + @property + def output_size(self): + # consider 3D images, third dimension can be the energy windows + output_size = list(self.size) + if len(output_size) != 2: + fatal( + f"Error, the size must be a 2-vector (2D) while it is {output_size}. " + f"Note: The size along the third dimension is automatically set to 1." + ) + output_size.append(1) + return output_size + + @property + def output_spacing(self): + # consider 3D images, third dimension can be the energy windows + output_spacing = list(self.spacing) + if len(output_spacing) != 2: + fatal( + f"Error, the spacing must be a 2-vector (2D) while it is {output_spacing}. " + f"Note: The spacing along the third dimension is automatically set to 1." + ) + output_spacing.append(1) + return output_spacing + def compute_thickness(self, volume, channels): """ Get the thickness of the detector volume, in the correct direction. @@ -887,8 +881,8 @@ def StartSimulationAction(self): # define the new size and spacing according to the nb of channels # and according to the volume shape - size = self.size - spacing = self.spacing + size = self.output_size + spacing = self.output_spacing size[2] = len(self.input_digi_collections) * len( self.simulation.run_timing_intervals ) From 226451010eac43315c44e859e47e7eacfeb4920e Mon Sep 17 00:00:00 2001 From: David Sarrut Date: Tue, 24 Sep 2024 20:58:47 +0200 Subject: [PATCH 08/19] Update .gitignore to exclude save_tests directory --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 1ec8a5d0c..856bb7270 100644 --- a/.gitignore +++ b/.gitignore @@ -91,6 +91,7 @@ opengate_temporaire /opengate/tests/src/demo_code_*.py /opengate/tests/src/test030_save /opengate/tests/src/test_log* +/opengate/tests/src/save_tests /opengate/data/isomeric_transition/save /opengate/data/isomeric_transition/save2 From a275a2bf69e2d64f6af70ebab8be1dd94b577392 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 25 Sep 2024 10:24:46 +0200 Subject: [PATCH 09/19] Add explicit option for authorizing repeated volumes in digitizers (false by default) --- .../opengate_lib/GateHelpersGeometry.cpp | 11 +++++ opengate/actors/digitizers.py | 47 +++++++++++++++++-- .../src/test028_ge_nm670_spect_2_helpers.py | 2 +- .../tests/src/test036_adder_depth_helpers.py | 4 +- 4 files changed, 58 insertions(+), 6 deletions(-) diff --git a/core/opengate_core/opengate_lib/GateHelpersGeometry.cpp b/core/opengate_core/opengate_lib/GateHelpersGeometry.cpp index 4febe8557..bd789b2f3 100644 --- a/core/opengate_core/opengate_lib/GateHelpersGeometry.cpp +++ b/core/opengate_core/opengate_lib/GateHelpersGeometry.cpp @@ -19,6 +19,17 @@ void ComputeTransformationFromVolumeToWorld(const std::string &phys_volume_name, auto pvs = G4PhysicalVolumeStore::GetInstance(); while (name != "world") { auto phys = pvs->GetVolume(name); + if (phys == nullptr) { + std::ostringstream oss; + oss << "The volume '" << name + << "' is not found. Here is the list of volumes: "; + auto map = pvs->GetMap(); + for (auto m : map) { + oss << m.first << " "; + } + oss << std::endl; + Fatal(oss.str()); + } auto tr = phys->GetObjectTranslation(); // auto rot = *phys->GetObjectRotation(); auto rot = phys->GetObjectRotationValue(); diff --git a/opengate/actors/digitizers.py b/opengate/actors/digitizers.py index 7dc8c677f..a1e5d95b5 100644 --- a/opengate/actors/digitizers.py +++ b/opengate/actors/digitizers.py @@ -196,6 +196,18 @@ def find_first_module(self, s): class DigitizerBase(ActorBase): _output_name_root = "root_output" + # hints for IDE + authorize_repeated_volumes: bool + + user_info_defaults = { + "authorize_repeated_volumes": ( + False, + { + "doc": "User must say explicitly that the digitizer can work with repeated volumes", + }, + ), + } + def __init__(self, *args, **kwargs): super().__init__(self, *args, **kwargs) @@ -215,6 +227,22 @@ def _add_user_output_root(self, **kwargs): ) return self._add_user_output(ActorOutputRoot, self._output_name_root, **kwargs) + def initialize(self): + ActorBase.initialize(self) + if self.authorize_repeated_volumes is True: + return + vm = self.simulation.volume_manager + current = vm.volumes[self.attached_to] + while current.name != "world": + if len(current.g4_transform) > 1: + fatal( + f"This digitizer actor name '{self.name}' is attached to the volume '{self.attached_to}. " + f"However, this volume is a daughter of the repeated volume '{current.name}'. It means it will " + f"gather data from all repeated instances. If you are " + f"sure, enable the option 'authorize_repeated_volumes'." + ) + current = current.parent + class DigitizerAdderActor(DigitizerBase, g4.GateDigitizerAdderActor): """ @@ -563,11 +591,12 @@ def initialize_blurring_parameters(self): def initialize(self): self.initialize_blurring_parameters() - ActorBase.initialize(self) + DigitizerBase.initialize(self) self.InitializeUserInput(self.user_info) self.InitializeCpp() def StartSimulationAction(self): + DigitizerBase.StartSimulationAction(self) g4.GateDigitizerSpatialBlurringActor.StartSimulationAction(self) def EndSimulationAction(self): @@ -629,11 +658,12 @@ def initialize_blurring_parameters(self): def initialize(self): self.initialize_blurring_parameters() - ActorBase.initialize(self) + DigitizerBase.initialize(self) self.InitializeUserInput(self.user_info) self.InitializeCpp() def StartSimulationAction(self): + DigitizerBase.StartSimulationAction(self) g4.GateDigitizerEfficiencyActor.StartSimulationAction(self) def EndSimulationAction(self): @@ -762,7 +792,7 @@ def EndSimulationAction(self): g4.GateDigitizerHitsCollectionActor.EndSimulationAction(self) -class DigitizerProjectionActor(ActorBase, g4.GateDigitizerProjectionActor): +class DigitizerProjectionActor(DigitizerBase, g4.GateDigitizerProjectionActor): """ This actor takes as input HitsCollections and performed binning in 2D images. If there are several HitsCollection as input, the slices will correspond to each HC. @@ -825,7 +855,12 @@ def initialize(self): f"Sorry, cannot (yet) use several attached_to volumes for " f"DigitizerProjectionActor {self.user_info.name}" ) - ActorBase.initialize(self) + if self.authorize_repeated_volumes is True: + fatal( + f"Sorry, cannot (yet) use ProjectionActor with repeated volumes, " + f"set 'authorize_repeated_volumes' to False" + ) + DigitizerBase.initialize(self) self.InitializeUserInput(self.user_info) self.InitializeCpp() @@ -870,6 +905,7 @@ def compute_thickness(self, volume, channels): return thickness def StartSimulationAction(self): + DigitizerBase.StartSimulationAction(self) # for the moment, we cannot use this actor with several volumes if hasattr(self.attached_to, "__len__") and not isinstance( self.attached_to, str @@ -907,6 +943,7 @@ def StartSimulationAction(self): self.attached_to_volume, self.user_output.projection.data_per_run[0].image ) self.fPhysicalVolumeName = str(pv.GetName()) + print(self.fPhysicalVolumeName) # update the cpp image and start update_image_py_to_cpp( @@ -976,6 +1013,7 @@ def __initcpp__(self): self.AddActions({"StartSimulationAction", "EndSimulationAction"}) def StartSimulationAction(self): + DigitizerBase.StartSimulationAction(self) DigitizerAdderActor.set_group_by_depth(self) if self.user_info.discretize_volume is None: fatal(f'Please, set the option "discretize_volume"') @@ -1045,6 +1083,7 @@ def initialize(self): self.InitializeCpp() def StartSimulationAction(self): + DigitizerBase.StartSimulationAction(self) g4.GatePhaseSpaceActor.StartSimulationAction(self) def EndSimulationAction(self): diff --git a/opengate/tests/src/test028_ge_nm670_spect_2_helpers.py b/opengate/tests/src/test028_ge_nm670_spect_2_helpers.py index 25353cccb..9924a716e 100644 --- a/opengate/tests/src/test028_ge_nm670_spect_2_helpers.py +++ b/opengate/tests/src/test028_ge_nm670_spect_2_helpers.py @@ -342,7 +342,7 @@ def test_spect_proj(sim, paths, proj, version="3"): print("Compare images (old spacing/origin)") # read image and force change the offset to be similar to old Gate img = itk.imread(str(paths.output / "proj028.mhd")) - spacing = np.array(proj.user_info.spacing) + spacing = np.array(proj.projection.image.GetSpacing()) # user_info.spacing) origin = spacing / 2.0 origin[2] = 0.5 spacing[2] = 1 diff --git a/opengate/tests/src/test036_adder_depth_helpers.py b/opengate/tests/src/test036_adder_depth_helpers.py index e67193d46..09376d29a 100644 --- a/opengate/tests/src/test036_adder_depth_helpers.py +++ b/opengate/tests/src/test036_adder_depth_helpers.py @@ -118,6 +118,7 @@ def create_simulation(geom, paths): # hits collection hc = sim.add_actor("DigitizerHitsCollectionActor", "Hits") hc.attached_to = crystal.name + hc.authorize_repeated_volumes = True hc.output_filename = "test036.root" hc.attributes = [ "KineticEnergy", @@ -134,6 +135,7 @@ def create_simulation(geom, paths): # singles collection sc = sim.add_actor("DigitizerAdderActor", "Singles") sc.attached_to = crystal.name + sc.authorize_repeated_volumes = True sc.input_digi_collection = "Hits" # sc.policy = 'EnergyWinnerPosition' sc.policy = "EnergyWeightedCentroidPosition" @@ -148,7 +150,7 @@ def create_simulation(geom, paths): # print cuts print(sim.physics_manager.dump_production_cuts()) - # add a user hook function to dump production cuts frmo Geant4 + # add a user hook function to dump production cuts from Geant4 sim.user_hook_after_init = check_production_cuts return sim From 2442af081ee9b3a7cc82c4f6ee649fa858f42b35 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 25 Sep 2024 16:02:17 +0200 Subject: [PATCH 10/19] Handle null process and empty collections in digitizer actor. Added null check for process in GateDigitizerHitsCollectionActor to prevent potential crashes. Additionally, included a safeguard for empty collections in GateDigiCollection::DumpLastDigi to avoid returning invalid data. --- .../opengate_lib/digitizer/GateDigiCollection.cpp | 2 ++ .../digitizer/GateDigitizerHitsCollectionActor.cpp | 12 +++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/core/opengate_core/opengate_lib/digitizer/GateDigiCollection.cpp b/core/opengate_core/opengate_lib/digitizer/GateDigiCollection.cpp index fdd7780b6..8880be26b 100644 --- a/core/opengate_core/opengate_lib/digitizer/GateDigiCollection.cpp +++ b/core/opengate_core/opengate_lib/digitizer/GateDigiCollection.cpp @@ -224,6 +224,8 @@ GateDigiCollection::Iterator GateDigiCollection::NewIterator() { } std::string GateDigiCollection::DumpLastDigi() const { + if (GetSize() == 0) + return ""; std::ostringstream oss; auto n = GetSize() - 1; for (auto *att : fDigiAttributes) { diff --git a/core/opengate_core/opengate_lib/digitizer/GateDigitizerHitsCollectionActor.cpp b/core/opengate_core/opengate_lib/digitizer/GateDigitizerHitsCollectionActor.cpp index a54d178a8..d5ba13da3 100644 --- a/core/opengate_core/opengate_lib/digitizer/GateDigitizerHitsCollectionActor.cpp +++ b/core/opengate_core/opengate_lib/digitizer/GateDigitizerHitsCollectionActor.cpp @@ -98,11 +98,13 @@ void GateDigitizerHitsCollectionActor::SteppingAction(G4Step *step) { // nb transportation auto post = step->GetPostStepPoint(); auto process = post->GetProcessDefinedStep(); - auto pname = process->GetProcessName(); - static int nb_tr = 0; - if (pname == "Transportation") { - nb_tr++; - std::cout << pname << " " << nb_tr << " " << id << x << std::endl; + if (process != nullptr) { + auto pname = process->GetProcessName(); + static int nb_tr = 0; + if (pname == "Transportation") { + nb_tr++; + std::cout << pname << " " << nb_tr << " " << id << x << std::endl; + } } } } From 6e32ccf2910d9e9f0cb27f67e70554af32c11715 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 26 Sep 2024 13:09:16 +0200 Subject: [PATCH 11/19] Add tests for DigitizerProjectionActor handling repeated volumes (prevent errors) Introduced new test cases for the DigitizerProjectionActor, including scenarios with repeated volumes and invalid configurations. Enhanced the actor to handle lists/tuples for the `attached_to` property and added validations to prevent inappropriate attachment to parameterized volumes. --- opengate/actors/digitizers.py | 60 +++++++++++++++---- .../tests/src/test036_adder_depth_helpers.py | 25 ++++---- opengate/tests/src/test036_proj_param_1.py | 58 ++++++++++++++++++ .../src/test036_proj_param_2_with_index.py | 45 ++++++++++++++ .../test036_proj_param_3_no_parent_repeat.py | 33 ++++++++++ ...t036_proj_param_4_not_attached_to_param.py | 39 ++++++++++++ .../src/test037_pet_hits_singles_helpers.py | 1 + 7 files changed, 237 insertions(+), 24 deletions(-) create mode 100755 opengate/tests/src/test036_proj_param_1.py create mode 100755 opengate/tests/src/test036_proj_param_2_with_index.py create mode 100755 opengate/tests/src/test036_proj_param_3_no_parent_repeat.py create mode 100755 opengate/tests/src/test036_proj_param_4_not_attached_to_param.py diff --git a/opengate/actors/digitizers.py b/opengate/actors/digitizers.py index a1e5d95b5..0fb7d3843 100644 --- a/opengate/actors/digitizers.py +++ b/opengate/actors/digitizers.py @@ -1,11 +1,13 @@ +from typing import List + import numpy as np from scipy.spatial.transform import Rotation import opengate_core as g4 +from anytree import NodeMixin from .base import ActorBase from ..exception import fatal, warning from ..definitions import fwhm_to_sigma - from ..utility import g4_units from ..image import ( align_image_with_physical_volume, @@ -232,16 +234,20 @@ def initialize(self): if self.authorize_repeated_volumes is True: return vm = self.simulation.volume_manager - current = vm.volumes[self.attached_to] - while current.name != "world": - if len(current.g4_transform) > 1: - fatal( - f"This digitizer actor name '{self.name}' is attached to the volume '{self.attached_to}. " - f"However, this volume is a daughter of the repeated volume '{current.name}'. It means it will " - f"gather data from all repeated instances. If you are " - f"sure, enable the option 'authorize_repeated_volumes'." - ) - current = current.parent + att = self.attached_to + if not isinstance(self.attached_to, (list, tuple)): + att = [self.attached_to] + for a in att: + current = self.simulation.volume_manager.get_volume(a).parent + while current.name != "world" and not isinstance(current, NodeMixin): + if len(current.g4_transform) > 1: + fatal( + f"This digitizer actor name '{self.name}' is attached to the volume '{self.attached_to}. " + f"However, this volume is a daughter of the repeated volume '{current.name}'. It means it will " + f"gather data from all repeated instances. If you are " + f"sure, enable the option 'authorize_repeated_volumes'." + ) + current = current.parent class DigitizerAdderActor(DigitizerBase, g4.GateDigitizerAdderActor): @@ -799,6 +805,14 @@ class DigitizerProjectionActor(DigitizerBase, g4.GateDigitizerProjectionActor): If there are several runs, images will also be slice-stacked. """ + # hints for IDE + input_digi_collections: List[str] + spacing: List[float] + size: List[int] + physical_volume_index: int + origin_as_image_center: bool + detector_orientation_matrix: np.ndarray + user_info_defaults = { # FIXME: implement a setter hook so the user can provided digitizer instances instead of their name, # like in attached_to @@ -817,9 +831,9 @@ class DigitizerProjectionActor(DigitizerBase, g4.GateDigitizerProjectionActor): {"doc": "FIXME"}, ), "physical_volume_index": ( - 0, + -1, { - "doc": "FIXME", + "doc": "When attached to a repeated volume, this option indicate which copy is used", }, ), "origin_as_image_center": ( @@ -894,6 +908,13 @@ def compute_thickness(self, volume, channels): By default, it is Z. We use the 'projection_orientation' to get the correct one. """ vol = self.actor_engine.simulation_engine.volume_engine.get_volume(volume) + if len(vol.g4_physical_volumes) < 1: + fatal( + f"The actor {self.name} is attached to '{self.attached_to}' which has " + f"no associated g4_physical_volumes (probably a parameterized volume?) and cannot " + f"be used. Try to attach it to the mother volume of the parameterized volume." + ) + solid = vol.g4_physical_volumes[0].GetLogicalVolume().GetSolid() pMin = g4.G4ThreeVector() pMax = g4.G4ThreeVector() @@ -929,6 +950,19 @@ def StartSimulationAction(self): # to be able to work on a per-run basis self.user_output.projection.create_empty_image(0, size, spacing) + # check physical_volume_index and number of repeating + n = len(self.attached_to_volume.g4_physical_volumes) + if n > 1: + if self.physical_volume_index >= n or self.physical_volume_index < 0: + fatal( + f"The actor '{self.name}' is attached to '{self.attached_to}' which is repeated {n} times. " + f"You must set a valid 'physical_volume_index' between O to {n-1} while " + f"it is {self.physical_volume_index}." + ) + else: + # force the index to be zero + self.physical_volume_index = 0 + # initial position (will be anyway updated in BeginOfRunSimulation) try: pv = self.attached_to_volume.g4_physical_volumes[self.physical_volume_index] diff --git a/opengate/tests/src/test036_adder_depth_helpers.py b/opengate/tests/src/test036_adder_depth_helpers.py index 09376d29a..ede0281ca 100644 --- a/opengate/tests/src/test036_adder_depth_helpers.py +++ b/opengate/tests/src/test036_adder_depth_helpers.py @@ -187,17 +187,20 @@ def test_output(sim, paths): scalings = [1.0] * len(scalings2) tols[2] = 2 # Z # tols[4] = 0.01 # energy - is_ok = utility.compare_root3( - gate_file, - hc.get_output_path(), - "Hits", - "Hits", - keys1, - keys2, - tols, - scalings, - scalings2, - paths.output / "test036_hits.png", + is_ok = ( + utility.compare_root3( + gate_file, + hc.get_output_path(), + "Hits", + "Hits", + keys1, + keys2, + tols, + scalings, + scalings2, + paths.output / "test036_hits.png", + ) + and is_ok ) # Root compare SINGLES diff --git a/opengate/tests/src/test036_proj_param_1.py b/opengate/tests/src/test036_proj_param_1.py new file mode 100755 index 000000000..82450b199 --- /dev/null +++ b/opengate/tests/src/test036_proj_param_1.py @@ -0,0 +1,58 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +import test036_adder_depth_helpers as t036 +from opengate.tests import utility +from opengate import g4_units + +if __name__ == "__main__": + paths = utility.get_default_test_paths( + __file__, "gate_test036_adder_depth", "test036" + ) + + # create and run the simulation + mm = g4_units.mm + sim = t036.create_simulation("param", paths) + + # sim.visu = True + sim.visu_type = "qt" + + # remove one head + head = sim.volume_manager.get_volume("SPECThead") + head.translation = head.translation[0] + head.rotation = head.rotation[0] + + # enlarge the source + source = sim.source_manager.get_source_info("src2") + source.position.radius = 150 * mm + + if sim.visu: + source = sim.source_manager.get_source_info("src2") + source.activity = source.activity / 1000 + source = sim.source_manager.get_source_info("src1") + source.activity = source.activity / 1000 + + # add a proj actor + proj = sim.add_actor("DigitizerProjectionActor", "proj") + proj.attached_to = "crystal" + proj.output_filename = "proj1.mha" + proj.size = [128, 128] + proj.spacing = [5 * mm, 5 * mm] + + # start simulation + sim.run() + + # test the output + stats = sim.get_actor("Stats") + is_ok = utility.assert_images( + paths.output_ref / proj.output_filename, + proj.get_output_path(), + stats, + tolerance=38, + ignore_value=0, + axis="y", + sum_tolerance=1.5, + fig_name=paths.output / f"proj.png", + ) + utility.print_test(is_ok, f"Compare image proj:") + utility.test_ok(is_ok) diff --git a/opengate/tests/src/test036_proj_param_2_with_index.py b/opengate/tests/src/test036_proj_param_2_with_index.py new file mode 100755 index 000000000..fa999108f --- /dev/null +++ b/opengate/tests/src/test036_proj_param_2_with_index.py @@ -0,0 +1,45 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +import test036_adder_depth_helpers as t036 +from opengate.tests import utility +from opengate import g4_units + +if __name__ == "__main__": + paths = utility.get_default_test_paths( + __file__, "gate_test036_adder_depth", "test036" + ) + + # create and run the simulation + mm = g4_units.mm + sim = t036.create_simulation("param", paths) + + # enlarge the source + source = sim.source_manager.get_source_info("src2") + source.position.radius = 150 * mm + + # add a proj actor to a repeated volume + proj = sim.add_actor("DigitizerProjectionActor", "proj2") + proj.attached_to = "SPECThead" # <-- there is two copy + proj.physical_volume_index = 0 # <-- the first one is considered + proj.output_filename = "proj2.mha" + proj.size = [128, 128] + proj.spacing = [5 * mm, 5 * mm] + + # start simulation + sim.run() + + # test the output + stats = sim.get_actor("Stats") + is_ok = utility.assert_images( + paths.output_ref / "proj1.mha", + proj.get_output_path(), + stats, + tolerance=38, + ignore_value=0, + axis="y", + sum_tolerance=1.5, + fig_name=paths.output / f"proj_index.png", + ) + utility.print_test(is_ok, f"Compare image proj:") + utility.test_ok(is_ok) diff --git a/opengate/tests/src/test036_proj_param_3_no_parent_repeat.py b/opengate/tests/src/test036_proj_param_3_no_parent_repeat.py new file mode 100755 index 000000000..2058af678 --- /dev/null +++ b/opengate/tests/src/test036_proj_param_3_no_parent_repeat.py @@ -0,0 +1,33 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +import test036_adder_depth_helpers as t036 +from opengate.tests import utility +from opengate import g4_units + +if __name__ == "__main__": + paths = utility.get_default_test_paths( + __file__, "gate_test036_adder_depth", "test036" + ) + + # create and run the simulation + mm = g4_units.mm + sim = t036.create_simulation("param", paths) + + # add a proj actor: it should not run because one of its parent is repeated + proj = sim.add_actor("DigitizerProjectionActor", "proj") + proj.attached_to = "crystal" + proj.output_filename = "proj1.mha" + proj.size = [128, 128] + proj.spacing = [5 * mm, 5 * mm] + + # start simulation + is_ok = True + try: + sim.run() + is_ok = False + except: + pass + utility.print_test(is_ok, f"The simulation should not run") + + utility.test_ok(is_ok) diff --git a/opengate/tests/src/test036_proj_param_4_not_attached_to_param.py b/opengate/tests/src/test036_proj_param_4_not_attached_to_param.py new file mode 100755 index 000000000..a8fac4949 --- /dev/null +++ b/opengate/tests/src/test036_proj_param_4_not_attached_to_param.py @@ -0,0 +1,39 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +import test036_adder_depth_helpers as t036 +from opengate.tests import utility +from opengate import g4_units + +if __name__ == "__main__": + paths = utility.get_default_test_paths( + __file__, "gate_test036_adder_depth", "test036" + ) + + # create and run the simulation + mm = g4_units.mm + sim = t036.create_simulation("param", paths) + + # sim.visu = True + sim.visu_type = "qt" + + # remove one head + head = sim.volume_manager.get_volume("SPECThead") + head.translation = head.translation[0] + head.rotation = head.rotation[0] + + # test another case that should fail + proj2 = sim.add_actor("DigitizerProjectionActor", "proj2") + proj2.attached_to = "crystal_pixel" + proj2.output_filename = "proj2.mha" + proj2.size = [128, 128] + proj2.spacing = [5 * mm, 5 * mm] + + is_ok = True + try: + sim.run(start_new_process=True) + is_ok = False + except: + pass + utility.print_test(is_ok, f"The run should NOT be executed: ") + utility.test_ok(is_ok) diff --git a/opengate/tests/src/test037_pet_hits_singles_helpers.py b/opengate/tests/src/test037_pet_hits_singles_helpers.py index e006beb21..0a1ea511b 100644 --- a/opengate/tests/src/test037_pet_hits_singles_helpers.py +++ b/opengate/tests/src/test037_pet_hits_singles_helpers.py @@ -90,6 +90,7 @@ def add_digitizer(sim, paths, nb, crystal): # hits collection hc = sim.add_actor("DigitizerHitsCollectionActor", "Hits") hc.attached_to = crystal.name + hc.authorize_repeated_volumes = True print("Crystal :", crystal.name) hc.output_filename = f"test037_test{nb}.root" hc.attributes = [ From 4af9e80cdb121ee3434d1995f000b1d6958c821e Mon Sep 17 00:00:00 2001 From: David Date: Thu, 26 Sep 2024 13:10:35 +0200 Subject: [PATCH 12/19] data for t036 --- opengate/tests/data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengate/tests/data b/opengate/tests/data index 714c1390a..e3d68b9c3 160000 --- a/opengate/tests/data +++ b/opengate/tests/data @@ -1 +1 @@ -Subproject commit 714c1390a3af3f70504bc263d237fd7c29e5eb41 +Subproject commit e3d68b9c3966f990c33ff3237965792c44407e73 From 399246572f49195fcaeb0783ab5ea3f0484ad4a1 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 26 Sep 2024 16:39:55 +0200 Subject: [PATCH 13/19] remove debug message --- opengate/base.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/opengate/base.py b/opengate/base.py index 838d8b776..4551a56d0 100644 --- a/opengate/base.py +++ b/opengate/base.py @@ -411,11 +411,11 @@ def __getstate__(self): def __setstate__(self, d): """Method needed for pickling. May be overridden in inheriting classes.""" self.__dict__ = d - print( + """print( f"DEBUG: in __setstate__ of {type(self).__name__}: {type(self).known_attributes}" ) print(f"DEBUG: type(self).known_attributes: {type(self).known_attributes}") - print(f"DEBUG: list(self.__dict__.keys()): {list(self.__dict__.keys())}") + print(f"DEBUG: list(self.__dict__.keys()): {list(self.__dict__.keys())}")""" def __reduce__(self): """This method is called when the object is pickled. @@ -450,7 +450,7 @@ def __setattr__(self, key, value): if len(self.known_attributes) > 0: if key not in self.known_attributes: s = ", ".join(str(a) for a in self.known_attributes) - warning( + self.warn_user( f'For object "{self.name}", attribute "{key}" is not known. Maybe a typo?\n' f"Known attributes of this object are: {s}" ) @@ -474,7 +474,9 @@ def __finalize_init__(self): # we define this at the class-level type(self).known_attributes = set( - list(self.user_info.keys()) + list(self.__dict__.keys()) + list(self.user_info.keys()) + + list(self.__dict__.keys()) + + list(["__dict__"]) ) def __add_to_simulation__(self): @@ -550,8 +552,12 @@ def from_dictionary(self, d): f"In that case, simply ignore the warning. " ) - def warning(self, message): - self.simulation._warnings.append(message) + def warn_user(self, message): + # this may be called without simulation object, so we guard with try/except + try: + self.simulation._user_warnings.append(message) + except: + pass warning(message) From a7305724365e6ee34fd89273f96d658f4c4080b1 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 26 Sep 2024 16:41:12 +0200 Subject: [PATCH 14/19] remove fake attributes (checked by test 78) --- opengate/tests/src/test006_runs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/opengate/tests/src/test006_runs.py b/opengate/tests/src/test006_runs.py index d1c628701..a5e781a28 100755 --- a/opengate/tests/src/test006_runs.py +++ b/opengate/tests/src/test006_runs.py @@ -55,7 +55,6 @@ source3.start_time = 0.50 * sec source3.direction.type = "momentum" source3.direction.momentum = [0, 0, 1] - source3.toto = 120 # raise a warning # Expected total of events # 100 + 175 + 120 = 395 From 42fdd0b2d0d1cf0662f30365c8570dc504631bc8 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 26 Sep 2024 16:41:36 +0200 Subject: [PATCH 15/19] user spacing is 2D. We need the real 3D spacing from the output image --- opengate/tests/src/test073_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengate/tests/src/test073_helpers.py b/opengate/tests/src/test073_helpers.py index 229b99ee5..56658c98b 100644 --- a/opengate/tests/src/test073_helpers.py +++ b/opengate/tests/src/test073_helpers.py @@ -162,7 +162,7 @@ def compare_proj_images(crystal, sim, stats, image_filename, path, n=1): f1 = path / f"projections_test{n}.mhd" f2 = path / f"projections_test{n}_offset.mhd" img = itk.imread(f1) - spacing = np.array(proj.user_info.spacing) + spacing = np.array(proj.projection.image.GetSpacing()) origin = spacing / 2.0 origin[2] = 0.5 spacing[2] = 1 From 3609e07634bd1f17e939a0646c4c2dd5a6c507d4 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 26 Sep 2024 16:42:06 +0200 Subject: [PATCH 16/19] Refactor warning calls to warn_user method Replaced all instances of the deprecated warning function with the new warn_user method for consistency and clarity. Additionally, updated attribute handling in several classes to avoid unnecessary warnings and cleaned up unused imports and commented-out code. --- opengate/actors/actoroutput.py | 2 +- opengate/actors/base.py | 16 +++++++--------- opengate/actors/digitizers.py | 20 +++++++++++++------- opengate/actors/miscactors.py | 12 ++++++------ opengate/engines.py | 8 ++++---- opengate/managers.py | 10 +++++----- opengate/physics.py | 2 +- 7 files changed, 37 insertions(+), 33 deletions(-) diff --git a/opengate/actors/actoroutput.py b/opengate/actors/actoroutput.py index deedb86cd..011716734 100644 --- a/opengate/actors/actoroutput.py +++ b/opengate/actors/actoroutput.py @@ -808,7 +808,7 @@ class ActorOutputRoot(ActorOutputBase): def get_output_path(self, *args, **kwargs): if "which" in kwargs and kwargs["which"] != "merged": - warning( + self.warn_user( "Currently, GATE 10 only stores cumulative ROOT output per simulation ('merged'), " "not data per run. Showing you the path to the ROOT file with cumulative data." ) diff --git a/opengate/actors/base.py b/opengate/actors/base.py index 1110e83aa..e8ed5c28f 100644 --- a/opengate/actors/base.py +++ b/opengate/actors/base.py @@ -149,15 +149,13 @@ def __setstate__(self, state): self.__initcpp__() self.__update_interface_properties__() - # def __finalize_init__(self): - # super().__finalize_init__() - # # The following attributes exist. They are declared here to avoid warning - # self.known_attributes.append("fFilters") - # self.known_attributes.append("actor_engine") - # self.known_attributes.append("user_output") - # self.known_attributes.append("simulation") - # self.known_attributes.append("__dict__") - # self.known_attributes.append("output_filename") + def __finalize_init__(self): + super().__finalize_init__() + # The following attributes exist. They are declared here to avoid warning + # fFilters is not known here because ActorBase does not inherit from a cpp counterpart. + self.known_attributes.add("fFilters") + # output_filename is a property + self.known_attributes.add("output_filename") def to_dictionary(self): d = super().to_dictionary() diff --git a/opengate/actors/digitizers.py b/opengate/actors/digitizers.py index 0fb7d3843..2092bf90f 100644 --- a/opengate/actors/digitizers.py +++ b/opengate/actors/digitizers.py @@ -4,9 +4,8 @@ from scipy.spatial.transform import Rotation import opengate_core as g4 -from anytree import NodeMixin from .base import ActorBase -from ..exception import fatal, warning +from ..exception import fatal from ..definitions import fwhm_to_sigma from ..utility import g4_units from ..image import ( @@ -233,13 +232,12 @@ def initialize(self): ActorBase.initialize(self) if self.authorize_repeated_volumes is True: return - vm = self.simulation.volume_manager att = self.attached_to if not isinstance(self.attached_to, (list, tuple)): att = [self.attached_to] for a in att: current = self.simulation.volume_manager.get_volume(a).parent - while current.name != "world" and not isinstance(current, NodeMixin): + while current.name != "world" and hasattr(current, "g4_transform"): if len(current.g4_transform) > 1: fatal( f"This digitizer actor name '{self.name}' is attached to the volume '{self.attached_to}. " @@ -660,7 +658,9 @@ def __initcpp__(self): def initialize_blurring_parameters(self): if not (0.0 <= self.efficiency <= 1.0): - warning(f"Efficency set to {self.efficiency}, which is not in [0;1].") + self.warn_user( + f"Efficency set to {self.efficiency}, which is not in [0;1]." + ) def initialize(self): self.initialize_blurring_parameters() @@ -851,7 +851,7 @@ class DigitizerProjectionActor(DigitizerBase, g4.GateDigitizerProjectionActor): } def __init__(self, *args, **kwargs): - ActorBase.__init__(self, *args, **kwargs) + DigitizerBase.__init__(self, *args, **kwargs) self._add_user_output(ActorOutputSingleImage, "projection") self.start_output_origin = None self.__initcpp__() @@ -861,6 +861,10 @@ def __initcpp__(self): g4.GateDigitizerProjectionActor.__init__(self, self.user_info) self.AddActions({"StartSimulationAction", "EndSimulationAction"}) + """def __finalize_init__(self): + super().__finalize_init__() + self.known_attributes.add("fPhysicalVolumeName")""" + def initialize(self): # for the moment, we cannot use this actor with several volumes m = self.attached_to @@ -1124,5 +1128,7 @@ def EndSimulationAction(self): self.number_of_absorbed_events = self.GetNumberOfAbsorbedEvents() self.total_number_of_entries = self.GetTotalNumberOfEntries() if self.total_number_of_entries == 0: - warning(f"Empty output, no particles stored in {self.get_output_path()}") + self.warn_user( + f"Empty output, no particles stored in {self.get_output_path()}" + ) g4.GatePhaseSpaceActor.EndSimulationAction(self) diff --git a/opengate/actors/miscactors.py b/opengate/actors/miscactors.py index 7e5ed62d4..d8bc9d49a 100644 --- a/opengate/actors/miscactors.py +++ b/opengate/actors/miscactors.py @@ -202,7 +202,7 @@ class SimulationStatisticsActor(ActorBase, g4.GateSimulationStatisticsActor): def __init__(self, *args, **kwargs): ActorBase.__init__(self, *args, **kwargs) - output = self._add_user_output(ActorOutputStatisticsActor, "stats") + self._add_user_output(ActorOutputStatisticsActor, "stats") self.__initcpp__() self.__finalize_init__() @@ -210,11 +210,11 @@ def __initcpp__(self): g4.GateSimulationStatisticsActor.__init__(self, self.user_info) self.AddActions({"StartSimulationAction", "EndSimulationAction"}) - # def __finalize_init__(self): - # super().__finalize_init__() - # # this attribute is considered sometimes in the read_stat_file - # # we declare it here to avoid warning - # self.known_attributes.append("date") + def __finalize_init__(self): + super().__finalize_init__() + # this attribute is considered sometimes in the read_stat_file + # we declare it here to avoid warning + self.known_attributes.add("date") def __str__(self): s = self.user_output["stats"].__str__() diff --git a/opengate/engines.py b/opengate/engines.py index 6dea928f8..881a6d4c3 100644 --- a/opengate/engines.py +++ b/opengate/engines.py @@ -101,7 +101,7 @@ def initialize(self, run_timing_intervals, progress_bar=False): self.run_timing_intervals = run_timing_intervals assert_run_timing(self.run_timing_intervals) if len(self.simulation_engine.simulation.source_manager.user_info_sources) == 0: - self.simulation_engine.simulation.warning( + self.simulation_engine.simulation.warn_user( f"No source: no particle will be generated" ) self.progress_bar = progress_bar @@ -325,7 +325,7 @@ def initialize_global_cuts(self): # range if ui.energy_range_min is not None and ui.energy_range_max is not None: - self.simulation_engine.simulation.warning( + self.physics_manager.warn_user( f"WARNING ! SetEnergyRange only works in MT mode" ) pct = g4.G4ProductionCutsTable.GetProductionCutsTable() @@ -428,7 +428,7 @@ def initialize_optical_material_properties(self): self.g4_optical_material_tables[str(material_name)] ) else: - self.simulation_engine.simulation.warning( + self.simulation_engine.simulation.warn_user( f"Could not load the optical material properties for material {material_name} " f"found in volume {vol.name} from file {self.physics_manager.optical_properties_file}." ) @@ -865,7 +865,7 @@ def initialize_visualisation(self): def initialize_visualisation_gdml(self): # Check when GDML is activated, if G4 was compiled with GDML if not g4.GateInfo.get_G4GDML(): - warning( + self.simulation.warn_user( "Visualization with GDML not available in Geant4. Check G4 compilation." ) if self.current_visu_filename is None: diff --git a/opengate/managers.py b/opengate/managers.py index edd4c0773..47cdbc406 100644 --- a/opengate/managers.py +++ b/opengate/managers.py @@ -394,7 +394,7 @@ def dump_actor_types(self): return "\n".join(list(actor_types.keys())) def get_actor_user_info(self, name): - warning( + self.warn_user( f"Deprecation warning: The function 'get_actor_user_info' will soon be removed." f"Use my_actor.user_info instead, where 'my_actor' " f"should be replace by your actor object. " @@ -1422,9 +1422,9 @@ def __init__(self, name="simulation"): # read-only info self._current_random_seed = None - self._warnings = ( - [] - ) # list to store warning messages issued somewhere in the simulation + + # list to store warning messages issued somewhere in the simulation + self._user_warnings = [] def __str__(self): s = ( @@ -1458,7 +1458,7 @@ def current_random_seed(self): @property def warnings(self): - return self._warnings + return self._user_warnings def to_dictionary(self): d = super().to_dictionary() diff --git a/opengate/physics.py b/opengate/physics.py index 76a01e5e9..f25a9559d 100644 --- a/opengate/physics.py +++ b/opengate/physics.py @@ -87,7 +87,7 @@ def ConstructProcess(self): ] if len(particle_keys_to_consider) == 0: - self.physics_engine.simulation_engine.simulation.warning( + self.physics_engine.simulation_engine.simulation.warn_user( "user_limits_particles is False for all particles. No tracking cuts will be applied. Use sim.physics_manager.set_user_limits_particles()." ) From 02f77484598341344f4e105d24033d72f4c6454a Mon Sep 17 00:00:00 2001 From: David Date: Thu, 26 Sep 2024 16:50:40 +0200 Subject: [PATCH 17/19] Fix indentation error in digitizers.py --- opengate/actors/digitizers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengate/actors/digitizers.py b/opengate/actors/digitizers.py index 2092bf90f..5f95ab800 100644 --- a/opengate/actors/digitizers.py +++ b/opengate/actors/digitizers.py @@ -245,7 +245,7 @@ def initialize(self): f"gather data from all repeated instances. If you are " f"sure, enable the option 'authorize_repeated_volumes'." ) - current = current.parent + current = current.parent class DigitizerAdderActor(DigitizerBase, g4.GateDigitizerAdderActor): From 956119fbed00ad5721dec42d06d2a9575065f09a Mon Sep 17 00:00:00 2001 From: David Sarrut Date: Fri, 27 Sep 2024 09:00:06 +0200 Subject: [PATCH 18/19] Update physical volume name handling in DigitizerProjectionActor Replaced direct access to `fPhysicalVolumeName` with a setter method `SetPhysicalVolumeName`. This change enhances encapsulation and allows for more controlled modifications of the `fPhysicalVolumeName` attribute. Also initialized `fPhysicalVolumeName` to "None" by default in the constructor. --- .../opengate_lib/digitizer/GateDigitizerProjectionActor.cpp | 5 +++++ .../opengate_lib/digitizer/GateDigitizerProjectionActor.h | 6 ++++-- .../digitizer/pyGateDigitizerProjectionActor.cpp | 4 ++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/core/opengate_core/opengate_lib/digitizer/GateDigitizerProjectionActor.cpp b/core/opengate_core/opengate_lib/digitizer/GateDigitizerProjectionActor.cpp index 41668b195..445b5ced9 100644 --- a/core/opengate_core/opengate_lib/digitizer/GateDigitizerProjectionActor.cpp +++ b/core/opengate_core/opengate_lib/digitizer/GateDigitizerProjectionActor.cpp @@ -19,6 +19,7 @@ GateDigitizerProjectionActor::GateDigitizerProjectionActor(py::dict &user_info) fActions.insert("StartSimulationAction"); fActions.insert("EndOfEventAction"); fActions.insert("BeginOfRunAction"); + fPhysicalVolumeName = "None"; } GateDigitizerProjectionActor::~GateDigitizerProjectionActor() = default; @@ -35,6 +36,10 @@ void GateDigitizerProjectionActor::InitializeCpp() { fImage = ImageType::New(); } +void GateDigitizerProjectionActor::SetPhysicalVolumeName(std::string name) { + fPhysicalVolumeName = name; +} + // Called when the simulation start void GateDigitizerProjectionActor::StartSimulationAction() { // Get input hits collection diff --git a/core/opengate_core/opengate_lib/digitizer/GateDigitizerProjectionActor.h b/core/opengate_core/opengate_lib/digitizer/GateDigitizerProjectionActor.h index f1482254b..ddfef7188 100644 --- a/core/opengate_core/opengate_lib/digitizer/GateDigitizerProjectionActor.h +++ b/core/opengate_core/opengate_lib/digitizer/GateDigitizerProjectionActor.h @@ -29,9 +29,9 @@ class GateDigitizerProjectionActor : public GateVActor { ~GateDigitizerProjectionActor() override; - virtual void InitializeUserInput(py::dict &user_info) override; + void InitializeUserInput(py::dict &user_info) override; - virtual void InitializeCpp() override; + void InitializeCpp() override; // Called when the simulation start (master thread only) void StartSimulationAction() override; @@ -42,6 +42,8 @@ class GateDigitizerProjectionActor : public GateVActor { // Called every time an Event ends (all threads) void EndOfEventAction(const G4Event *event) override; + void SetPhysicalVolumeName(std::string name); + // Image type is 3D float by default typedef itk::Image ImageType; ImageType::Pointer fImage; diff --git a/core/opengate_core/opengate_lib/digitizer/pyGateDigitizerProjectionActor.cpp b/core/opengate_core/opengate_lib/digitizer/pyGateDigitizerProjectionActor.cpp index f3e4ae31d..6b6a12709 100644 --- a/core/opengate_core/opengate_lib/digitizer/pyGateDigitizerProjectionActor.cpp +++ b/core/opengate_core/opengate_lib/digitizer/pyGateDigitizerProjectionActor.cpp @@ -19,6 +19,6 @@ void init_GateDigitizerProjectionActor(py::module &m) { GateVActor>(m, "GateDigitizerProjectionActor") .def(py::init()) .def_readwrite("fImage", &GateDigitizerProjectionActor::fImage) - .def_readwrite("fPhysicalVolumeName", - &GateDigitizerProjectionActor::fPhysicalVolumeName); + .def("SetPhysicalVolumeName", + &GateDigitizerProjectionActor::SetPhysicalVolumeName); } From 0b97605c98c42c737a17f54a478fe71d55bccd5c Mon Sep 17 00:00:00 2001 From: David Sarrut Date: Fri, 27 Sep 2024 09:00:46 +0200 Subject: [PATCH 19/19] Fix typos and enhance digitizer configurability. Correct typos in the code, standardize filename and output property names, and enable support for repeated volumes in multiple digitizer actors. --- opengate/actors/digitizers.py | 12 ++++-------- opengate/contrib/pet/siemensbiograph.py | 2 ++ opengate/tests/src/test017_repeater.py | 2 -- .../src/test029_volume_time_rotation_helpers.py | 2 +- opengate/tests/src/test037_pet_hits_singles_2.py | 2 ++ .../src/test040_gan_phsp_pet_training_dataset.py | 2 +- opengate/tests/src/test043_garf_training_dataset.py | 2 +- opengate/tests/src/test053_phid_helpers2.py | 2 +- opengate/tests/src/test059_tpsource_weights.py | 2 +- opengate/tests/src/test072_coinc_sorter_1.py | 2 ++ opengate/tests/src/test073_test3_intevo_tc99m_mt.py | 2 +- opengate/tests/src/test073_test4_intevo_lu177_mt.py | 2 +- .../tests/src/test073_test5_discovery_lu177_mt.py | 2 +- .../tests/src/test073_test5_discovery_tc99m_mt.py | 2 +- 14 files changed, 19 insertions(+), 19 deletions(-) diff --git a/opengate/actors/digitizers.py b/opengate/actors/digitizers.py index 5f95ab800..59e5038b1 100644 --- a/opengate/actors/digitizers.py +++ b/opengate/actors/digitizers.py @@ -177,7 +177,8 @@ def add_module(self, module_type, module_name=None): mod.attached_to = self.actors[index - 1].attached_to if "input_digi_collection" in mod.user_info: mod.input_digi_collection = self.actors[index - 1].name - mod.write_to_disk = False + first_key = next(iter(mod.user_output)) + mod.user_output[first_key].write_to_disk = False self.actors.append(mod) return mod @@ -240,7 +241,7 @@ def initialize(self): while current.name != "world" and hasattr(current, "g4_transform"): if len(current.g4_transform) > 1: fatal( - f"This digitizer actor name '{self.name}' is attached to the volume '{self.attached_to}. " + f"This digitizer actor name '{self.name}' is attached to the volume '{self.attached_to}'. " f"However, this volume is a daughter of the repeated volume '{current.name}'. It means it will " f"gather data from all repeated instances. If you are " f"sure, enable the option 'authorize_repeated_volumes'." @@ -861,10 +862,6 @@ def __initcpp__(self): g4.GateDigitizerProjectionActor.__init__(self, self.user_info) self.AddActions({"StartSimulationAction", "EndSimulationAction"}) - """def __finalize_init__(self): - super().__finalize_init__() - self.known_attributes.add("fPhysicalVolumeName")""" - def initialize(self): # for the moment, we cannot use this actor with several volumes m = self.attached_to @@ -980,8 +977,7 @@ def StartSimulationAction(self): align_image_with_physical_volume( self.attached_to_volume, self.user_output.projection.data_per_run[0].image ) - self.fPhysicalVolumeName = str(pv.GetName()) - print(self.fPhysicalVolumeName) + self.SetPhysicalVolumeName(str(pv.GetName())) # update the cpp image and start update_image_py_to_cpp( diff --git a/opengate/contrib/pet/siemensbiograph.py b/opengate/contrib/pet/siemensbiograph.py index 4b9707034..529880bc0 100644 --- a/opengate/contrib/pet/siemensbiograph.py +++ b/opengate/contrib/pet/siemensbiograph.py @@ -81,6 +81,7 @@ def add_digitizer( # hits collection hc = sim.add_actor("DigitizerHitsCollectionActor", hits_name) hc.attached_to = crystal.name + hc.authorize_repeated_volumes = True hc.output_filename = output_filename hc.attributes = [ "PostPosition", @@ -91,6 +92,7 @@ def add_digitizer( # singles collection sc = sim.add_actor("DigitizerReadoutActor", singles_name) + sc.authorize_repeated_volumes = True sc.attached_to = crystal.name sc.input_digi_collection = hc.name sc.group_volume = block.name diff --git a/opengate/tests/src/test017_repeater.py b/opengate/tests/src/test017_repeater.py index 7f7321636..fd0afb173 100755 --- a/opengate/tests/src/test017_repeater.py +++ b/opengate/tests/src/test017_repeater.py @@ -4,7 +4,6 @@ import opengate as gate from opengate.tests import utility import opengate_core as g4 -from scipy.spatial.transform import Rotation if __name__ == "__main__": paths = utility.get_default_test_paths(__file__, "", "test017") @@ -84,7 +83,6 @@ dose.size = [150, 150, 150] dose.spacing = [1 * mm, 1 * mm, 1 * mm] dose.translation = [5 * mm, 0 * mm, 0 * mm] - dose.physical_volume_index = 0 print( "The Dose actor is triggered every time a hit occurs in the (logical volume) " '"crystal" (and any of its associated repeated physical volumes).' diff --git a/opengate/tests/src/test029_volume_time_rotation_helpers.py b/opengate/tests/src/test029_volume_time_rotation_helpers.py index a5f314a3d..1ec5b0b96 100644 --- a/opengate/tests/src/test029_volume_time_rotation_helpers.py +++ b/opengate/tests/src/test029_volume_time_rotation_helpers.py @@ -109,7 +109,7 @@ def create_simulation(sim, aa_flag, paths): hc.attached_to = "spect_crystal" hc.output_filename = "test029.root" if sim.number_of_threads == 1: - hc.write_to_disk = False + hc.root_output.write_to_disk = False hc.attributes = [ "PostPosition", "TotalEnergyDeposit", diff --git a/opengate/tests/src/test037_pet_hits_singles_2.py b/opengate/tests/src/test037_pet_hits_singles_2.py index 813fb3144..ed5e9acf8 100755 --- a/opengate/tests/src/test037_pet_hits_singles_2.py +++ b/opengate/tests/src/test037_pet_hits_singles_2.py @@ -21,6 +21,7 @@ # digitizer hits hc = sim.add_actor("DigitizerHitsCollectionActor", "Hits") hc.attached_to = crystal.name + hc.authorize_repeated_volumes = True hc.output_filename = f"test037_test{v}.root" hc.attributes = [ "PostPosition", @@ -31,6 +32,7 @@ # Readout (not need for adder) sc = sim.add_actor("DigitizerReadoutActor", "Singles2_1") + sc.authorize_repeated_volumes = True sc.output_filename = f"test037_test{v}.root" sc.input_digi_collection = "Hits" sc.group_volume = stack.name # should be depth=1 in Gate diff --git a/opengate/tests/src/test040_gan_phsp_pet_training_dataset.py b/opengate/tests/src/test040_gan_phsp_pet_training_dataset.py index 2a063e364..39b3bacc8 100755 --- a/opengate/tests/src/test040_gan_phsp_pet_training_dataset.py +++ b/opengate/tests/src/test040_gan_phsp_pet_training_dataset.py @@ -77,7 +77,7 @@ # add stat actor stats = sim.add_actor("SimulationStatisticsActor", "Stats") - stats.output = paths.output / "test040_train_stats.txt" + stats.output_filename = "test040_train_stats.txt" # filter gamma only f = sim.add_filter("ParticleFilter", "f") diff --git a/opengate/tests/src/test043_garf_training_dataset.py b/opengate/tests/src/test043_garf_training_dataset.py index 68a164968..57cbf2a8c 100755 --- a/opengate/tests/src/test043_garf_training_dataset.py +++ b/opengate/tests/src/test043_garf_training_dataset.py @@ -88,7 +88,7 @@ stats = sim.add_actor("SimulationStatisticsActor", "stats") stats.track_types_flag = True stats.output_filename = "test043_arf_training_dataset_stats.txt" - stats.write_to_disk = True + stats.stats.write_to_disk = True # start simulation sim.run(start_new_process=True) diff --git a/opengate/tests/src/test053_phid_helpers2.py b/opengate/tests/src/test053_phid_helpers2.py index 99ce02a6b..d72736edc 100644 --- a/opengate/tests/src/test053_phid_helpers2.py +++ b/opengate/tests/src/test053_phid_helpers2.py @@ -34,7 +34,7 @@ def create_sim_test053(sim, sim_name, output=paths.output): # add stat actor s = sim.add_actor("SimulationStatisticsActor", "stats") s.track_types_flag = True - s.output = output / f"test053_{sim_name}.txt" + s.output_filename = output / f"test053_{sim_name}.txt" # phsp actor phsp = sim.add_actor("PhaseSpaceActor", "phsp") diff --git a/opengate/tests/src/test059_tpsource_weights.py b/opengate/tests/src/test059_tpsource_weights.py index afccb464f..2e61f3034 100755 --- a/opengate/tests/src/test059_tpsource_weights.py +++ b/opengate/tests/src/test059_tpsource_weights.py @@ -66,7 +66,7 @@ # add dose actor dose = sim.add_actor("DoseActor", "doseInYZ_1") filename = "phantom_a_1.mhd" - dose.output_filename_filename = filename + dose.output_filename = filename dose.attached_to = "phantom_a_1" dose.size = [250, 250, 1] dose.spacing = [0.4, 0.4, 2] diff --git a/opengate/tests/src/test072_coinc_sorter_1.py b/opengate/tests/src/test072_coinc_sorter_1.py index a2d05253a..24a89f011 100755 --- a/opengate/tests/src/test072_coinc_sorter_1.py +++ b/opengate/tests/src/test072_coinc_sorter_1.py @@ -115,6 +115,7 @@ # Hits hc = sim.add_actor("DigitizerHitsCollectionActor", f"Hits_{crystal.name}") hc.attached_to = crystal + hc.authorize_repeated_volumes = True hc.output_filename = "test72_output_1.root" hc.attributes = [ "EventID", @@ -127,6 +128,7 @@ # Singles sc = sim.add_actor("DigitizerAdderActor", f"Singles_{crystal.name}") sc.attached_to = hc.attached_to + sc.authorize_repeated_volumes = True sc.input_digi_collection = hc.name sc.policy = "EnergyWinnerPosition" sc.output_filename = hc.output_filename diff --git a/opengate/tests/src/test073_test3_intevo_tc99m_mt.py b/opengate/tests/src/test073_test3_intevo_tc99m_mt.py index 10e56b0e4..b177ef922 100755 --- a/opengate/tests/src/test073_test3_intevo_tc99m_mt.py +++ b/opengate/tests/src/test073_test3_intevo_tc99m_mt.py @@ -24,7 +24,7 @@ c = {"name": f"spectrum", "min": 3 * keV, "max": 160 * keV} ew = digit.find_first_module("energy_window") ew.output_filename = "output_tc99m.root" - ew.write_to_disk = True + ew.root_output.write_to_disk = True ew.channels.append(c) # output diff --git a/opengate/tests/src/test073_test4_intevo_lu177_mt.py b/opengate/tests/src/test073_test4_intevo_lu177_mt.py index 8bdb1e135..75c626347 100755 --- a/opengate/tests/src/test073_test4_intevo_lu177_mt.py +++ b/opengate/tests/src/test073_test4_intevo_lu177_mt.py @@ -24,7 +24,7 @@ c = {"name": "spectrum", "min": 35 * keV, "max": 588 * keV} ew = digit.find_first_module("energy_window") ew.output_filename = "output_intevo_lu177.root" - ew.write_to_disk = True + ew.root_output.write_to_disk = True ew.channels.append(c) # output diff --git a/opengate/tests/src/test073_test5_discovery_lu177_mt.py b/opengate/tests/src/test073_test5_discovery_lu177_mt.py index 8b27bd031..f9e28b3a0 100755 --- a/opengate/tests/src/test073_test5_discovery_lu177_mt.py +++ b/opengate/tests/src/test073_test5_discovery_lu177_mt.py @@ -21,7 +21,7 @@ digit = discovery.add_digitizer_lu177(sim, crystal.name, "digit_lu177") ew = digit.find_first_module("energy_window") ew.output_filename = "output_discovery_lu177.root" - ew.write_to_disk = True + ew.root_output.write_to_disk = True # output stats.output_filename = "stats_discovery_lu177.txt" diff --git a/opengate/tests/src/test073_test5_discovery_tc99m_mt.py b/opengate/tests/src/test073_test5_discovery_tc99m_mt.py index bcd4dfe6a..9b3741d1c 100755 --- a/opengate/tests/src/test073_test5_discovery_tc99m_mt.py +++ b/opengate/tests/src/test073_test5_discovery_tc99m_mt.py @@ -21,7 +21,7 @@ digit = discovery.add_digitizer_tc99m(sim, crystal.name, "digit_tc99m") ew = digit.find_first_module("energy_window") ew.output_filename = "output_discovery_tc99m.root" - ew.write_to_disk = True + ew.root_output.write_to_disk = True # output stats.output_filename = "stats_discovery_tc99m.txt"