From 1f73235a6be91b5d98e99782238828b21e40b719 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Mon, 12 Jan 2026 14:47:57 +0100 Subject: [PATCH 01/10] File management for light emission package --- src/simtools/applications/simulate_flasher.py | 9 -- .../applications/simulate_illuminator.py | 11 +- src/simtools/runners/corsika_runner.py | 16 ++- src/simtools/runners/runner_services.py | 37 ++++- src/simtools/runners/simtel_runner.py | 11 +- .../simtel/simulator_light_emission.py | 131 ++++++++---------- .../simtel/test_simulator_light_emission.py | 4 +- 7 files changed, 113 insertions(+), 106 deletions(-) diff --git a/src/simtools/applications/simulate_flasher.py b/src/simtools/applications/simulate_flasher.py index 8bf4adefcb..8e294e745a 100644 --- a/src/simtools/applications/simulate_flasher.py +++ b/src/simtools/applications/simulate_flasher.py @@ -45,8 +45,6 @@ Calibration light source, e.g., MSFx-FlashCam number_of_events (int, optional): Number of events to simulate (default: 1). -output_prefix (str, optional): - Prefix for output files (default: empty). model_version (str, optional) Version of the simulation model. array_layout_name (str, optional) @@ -89,13 +87,6 @@ def _parse(): default=1, required=False, ) - config.parser.add_argument( - "--output_prefix", - help="Prefix for output files", - type=str, - default=None, - required=False, - ) return config.initialize( db_config=True, simulation_model=["site", "layout", "telescope", "model_version"], diff --git a/src/simtools/applications/simulate_illuminator.py b/src/simtools/applications/simulate_illuminator.py index 359cd07af4..753e702bca 100644 --- a/src/simtools/applications/simulate_illuminator.py +++ b/src/simtools/applications/simulate_illuminator.py @@ -49,8 +49,6 @@ m. If not set, the position from the simulation model is used. light_source_pointing (float, float, float, optional) Light source pointing direction. If not set, the pointing from the simulation model is used. -output_prefix (str, optional) - Prefix for output files (default: empty). """ from simtools.application_control import get_application_label, startup_application @@ -100,13 +98,6 @@ def _parse(): default=1, required=False, ) - config.parser.add_argument( - "--output_prefix", - help="Prefix for output files (default: empty)", - type=str, - default=None, - required=False, - ) return config.initialize( db_config=True, simulation_model=["telescope", "model_version"], @@ -119,7 +110,7 @@ def main(): app_context = startup_application(_parse) light_source = SimulatorLightEmission( - light_emission_config=app_context.args, + light_emission_config={**app_context.args, "run_mode": "illuminator"}, label=app_context.args.get("label"), ) light_source.simulate() diff --git a/src/simtools/runners/corsika_runner.py b/src/simtools/runners/corsika_runner.py index 49903b0931..f6253cb89a 100644 --- a/src/simtools/runners/corsika_runner.py +++ b/src/simtools/runners/corsika_runner.py @@ -70,13 +70,17 @@ def prepare_run(self, run_number, sub_script, extra_commands=None, corsika_file= self.corsika_config.generate_corsika_input_file( self._use_multipipe, self._corsika_seeds, - self.file_list["corsika_input"], - self.file_list["corsika_output"] if not self._use_multipipe else corsika_file, + self.runner_service.get_file_name("corsika_input", run_number=run_number), + self.runner_service.get_file_name("corsika_output", run_number=run_number) + if not self._use_multipipe + else corsika_file, ) self._logger.debug(f"Extra commands to be added to the run script: {extra_commands}") - corsika_run_dir = self.file_list["corsika_output"].parent + corsika_run_dir = self.runner_service.get_file_name( + "corsika_output", run_number=run_number + ).parent self._export_run_script(sub_script, corsika_run_dir, extra_commands) @@ -90,7 +94,8 @@ def _corsika_executable(self): def _export_run_script(self, sub_script, corsika_run_dir, extra_commands): """Export CORSIKA run script.""" - corsika_log_file = self.file_list["corsika_log"].with_suffix("") + corsika_log_file = self.runner_service.get_file_name("corsika_log").with_suffix("") + corsika_input = self.runner_service.get_file_name("corsika_input") sub_script = Path(sub_script) with open(sub_script, "w", encoding="utf-8") as file: file.write("#!/usr/bin/env bash\n") @@ -111,8 +116,7 @@ def _export_run_script(self, sub_script, corsika_run_dir, extra_commands): file.write("\n# Running corsika\n") file.write( - f"{self._corsika_executable()} < {self.file_list['corsika_input']} " - f"> {corsika_log_file} 2>&1\n" + f"{self._corsika_executable()} < {corsika_input} > {corsika_log_file} 2>&1\n" ) file.write("\n# Cleanup\n") file.write(f"gzip {corsika_log_file}\n") diff --git a/src/simtools/runners/runner_services.py b/src/simtools/runners/runner_services.py index 1274cd83e6..2509e33ae5 100644 --- a/src/simtools/runners/runner_services.py +++ b/src/simtools/runners/runner_services.py @@ -21,6 +21,11 @@ "suffix": ".corsika.log.gz", "sub_dir_type": "run_number", }, + # Generic iact output + "iact_output": { + "suffix": ".iact.gz", + "sub_dir_type": "run_number", + }, # sim_telarray "sim_telarray_output": { "suffix": ".simtel.zst", @@ -103,17 +108,20 @@ class RunnerServices: ---------- corsika_config : CorsikaConfig, list of CorsikaConfig Configuration parameters for CORSIKA. + core_config : dict + Core configuration parameters (less specific than CorsikaConfig). run_type : str Type of simulation runner. label : str Label. """ - def __init__(self, corsika_config, run_type, label=None): + def __init__(self, corsika_config, core_config, run_type, label=None): """Initialize RunnerServices.""" self._logger = logging.getLogger(__name__) self.label = label self.corsika_config = corsika_config + self.core_config = core_config self.run_type = run_type self.directory = self.load_data_directory() @@ -171,8 +179,31 @@ def _get_file_basename(self, run_number, is_multi_pipe=False): str Base name for the simulation files. """ - vc_high = self.corsika_config.get_config_parameter("VIEWCONE")[1] + if self.corsika_config: + return self._get_file_base_name_from_corsika_config(run_number, is_multi_pipe) + if self.core_config: + return self._get_file_base_name_from_core_config() + raise ValueError("Either corsika_config or core_config must be provided.") + + def _get_file_base_name_from_core_config(self): + """Get file base name from core configuration.""" + cfg = self.core_config + parts = [ + f"{cfg.get('run_mode', '')}_", + f"{self._get_run_number_string(cfg.get('run_number'))}_" if "run_number" in cfg else "", + f"za{round(cfg['zenith_angle']):02}deg_" if "zenith_angle" in cfg else "", + f"azm{cfg['azimuth_angle']:03}deg_" if "azimuth_angle" in cfg else "", + f"{cfg['site']}_" if "site" in cfg else "", + f"{cfg['layout']}_" if "layout" in cfg else "", + cfg.get("model_version", ""), + f"_{self.label}" if self.label else "", + ] + return "".join(parts) + + def _get_file_base_name_from_corsika_config(self, run_number, is_multi_pipe=False): + """Get file base name from CORSIKA configuration.""" zenith = self.corsika_config.get_config_parameter("THETAP")[0] + vc_high = self.corsika_config.get_config_parameter("VIEWCONE")[1] if self.corsika_config.run_mode is not None and self.corsika_config.run_mode != "": primary_name = self.corsika_config.run_mode @@ -264,6 +295,8 @@ def _get_run_number_string(run_number): str Run number string. """ + if run_number is None: + return "" return f"run{validate_corsika_run_number(run_number):06d}" def get_resources(self, sub_out_file): diff --git a/src/simtools/runners/simtel_runner.py b/src/simtools/runners/simtel_runner.py index 00058ad0d8..c988e9a867 100644 --- a/src/simtools/runners/simtel_runner.py +++ b/src/simtools/runners/simtel_runner.py @@ -28,11 +28,13 @@ class SimtelRunner: Instance label. Important for output file naming. corsika_config: CorsikaConfig CORSIKA configuration. + core_config: dict + Core configuration parameters (less specific than CorsikaConfig). is_calibration_run: bool Flag to indicate if this is a calibration run. """ - def __init__(self, label=None, corsika_config=None, is_calibration_run=False): + def __init__(self, label=None, corsika_config=None, core_config=None, is_calibration_run=False): """Initialize SimtelRunner.""" self._logger = logging.getLogger(__name__) @@ -42,7 +44,12 @@ def __init__(self, label=None, corsika_config=None, is_calibration_run=False): self.runs_per_set = 1 - self.runner_service = RunnerServices(corsika_config, "sim_telarray", label) + self.runner_service = RunnerServices( + corsika_config=corsika_config, + core_config=core_config, + run_type="sim_telarray", + label=label, + ) self.file_list = None def run(self, test=False, input_file=None, run_number=None): diff --git a/src/simtools/simtel/simulator_light_emission.py b/src/simtools/simtel/simulator_light_emission.py index 4bb4365c50..e298c501c5 100644 --- a/src/simtools/simtel/simulator_light_emission.py +++ b/src/simtools/simtel/simulator_light_emission.py @@ -35,9 +35,7 @@ def __init__(self, light_emission_config, label=None): self._logger = logging.getLogger(__name__) self.io_handler = io_handler.IOHandler() - super().__init__(label=label, corsika_config=None) - - self.output_directory = self.io_handler.get_output_directory() + super().__init__(label=label, core_config=light_emission_config) self.telescope_model, self.site_model, self.calibration_model = ( initialize_simulation_models( @@ -84,9 +82,12 @@ def simulate(self): The output simtel file path. """ run_script = self.prepare_script() - log_path = Path(self.output_directory) / "logfile.log" - job_manager.submit(run_script, out_file=log_path, err_file=log_path.with_suffix(".err")) - out = Path(self._get_simulation_output_filename()) + job_manager.submit( + run_script, + out_file=self.runner_service.get_file_name("sub_out"), + err_file=self.runner_service.get_file_name("sub_err"), + ) + out = Path(self.runner_service.get_file_name(file_type="sim_telarray_output")) if not out.exists(): self._logger.warning(f"Expected sim_telarray output not found: {out}") return out @@ -100,39 +101,28 @@ def prepare_script(self): Path Full path of the run script. """ - script_dir = self.output_directory.joinpath("scripts") - script_dir.mkdir(parents=True, exist_ok=True) - - app_name = self._get_light_emission_application_name() - script_file = script_dir / f"{app_name}-light_emission.sh" - self._logger.debug(f"Run bash script - {script_file}") - - target_out = Path(self._get_simulation_output_filename()) - if target_out.exists(): + script_file = self.runner_service.get_file_name(file_type="sub_script") + output_file = self.runner_service.get_file_name(file_type="sim_telarray_output") + iact_output = self.runner_service.get_file_name(file_type="iact_output") + if output_file.exists(): raise FileExistsError( - f"sim_telarray output file exists, cancelling simulation: {target_out}" + f"sim_telarray output file exists, cancelling simulation: {output_file}" ) lines = [ "#!/usr/bin/env bash\n", - f"{self._make_light_emission_script()}\n\n", + f"{self._make_light_emission_command(iact_output)}\n\n", ( - f"[ -s '{self.output_directory}/{app_name}.iact.gz' ] || " + f"[ -s '{iact_output}' ] || " f"{{ echo 'LightEmission did not produce IACT file' >&2; exit 1; }}\n\n" ), f"{self._make_simtel_script()}\n\n", - f"rm -f '{self.output_directory}/{app_name}.iact.gz'\n\n", + f"rm -f '{iact_output}'\n\n", ] script_file.write_text("".join(lines), encoding="utf-8") return script_file - def _get_prefix(self): - prefix = self.light_emission_config.get("output_prefix", "") - if prefix is not None: - return f"{prefix}_" - return "" - def _get_light_emission_application_name(self): """ Return the LightEmission application and mode from type. @@ -231,7 +221,9 @@ def _write_telescope_position_file(self): radius = self.telescope_model.get_parameter_value_with_unit("telescope_sphere_radius") radius = radius.to(u.cm).value # Convert radius to cm - telescope_position_file = self.output_directory.joinpath("telescope_position.dat") + telescope_position_file = ( + self.io_handler.get_output_directory("light_emission") / "telescope_position.dat" + ) telescope_position_file.write_text(f"{x_tel} {y_tel} {z_tel} {radius}\n", encoding="utf-8") return telescope_position_file @@ -259,13 +251,18 @@ def _prepare_flasher_atmosphere_files(self, config_directory, model_id=1): self._logger.warning(f"Failed to create atmosphere alias {dst.name}: {copy_err}") return model_id - def _make_light_emission_script(self): + def _make_light_emission_command(self, iact_output): """ - Create the light emission script to run the light emission package. + Create the light emission command to run the light emission package. Require the specified pre-compiled light emission package application in the sim_telarray/LightEmission/ path. + Parameters + ---------- + iact_output: str or Path + The output iact file path. + Returns ------- str @@ -274,24 +271,24 @@ def _make_light_emission_script(self): config_directory = self.io_handler.get_model_configuration_directory( model_version=self.site_model.model_version ) - app_name = self._get_light_emission_application_name() - corsika_observation_level = self.site_model.get_parameter_value_with_unit( - "corsika_observation_level" - ) + obs_level = self.site_model.get_parameter_value_with_unit("corsika_observation_level") + + app = self._get_light_emission_application_name() + cmd = [ + str(settings.config.sim_telarray_path / "LightEmission" / app), + *self._get_site_command(app, config_directory, obs_level), + *self._get_light_source_command(), + ] - parts = [str(settings.config.sim_telarray_path / "LightEmission") + f"/{app_name}"] - parts.extend(self._get_site_command(app_name, config_directory, corsika_observation_level)) - parts.extend(self._get_light_source_command()) if self.light_emission_config["light_source_type"] == "illuminator": - parts += [ + cmd += [ "-A", - ( - f"{config_directory}/" - f"{self.telescope_model.get_parameter_value('atmospheric_profile')}" - ), + f"{config_directory}/" + f"{self.telescope_model.get_parameter_value('atmospheric_profile')}", ] - parts += [f"-o {self.output_directory}/{app_name}.iact.gz", "\n"] - return " ".join(parts) + + cmd += ["-o", str(iact_output)] + return " ".join(cmd) def _get_site_command(self, app_name, config_directory, corsika_observation_level): """Return site command with altitude, atmosphere and telescope_position handling.""" @@ -350,17 +347,12 @@ def _add_flasher_command_options(self): " and exponential decay values" ) try: - base_dir = self.io_handler.get_output_directory("pulse_shapes") - - def _sanitize_name(value): - return "".join( - ch if (ch.isalnum() or ch in ("-", "_")) else "_" for ch in str(value) - ) - tel = self.light_emission_config.get("telescope") or "telescope" cal = self.light_emission_config.get("light_source") or "calibration" - fname = f"flasher_pulse_shape_{_sanitize_name(tel)}_{_sanitize_name(cal)}.dat" - table_path = base_dir / fname + fname = ( + f"flasher_pulse_shape_{self._sanitize_name(tel)}_{self._sanitize_name(cal)}.dat" + ) + table_path = self.io_handler.get_output_directory("light_emission") / fname fadc_bins = self.telescope_model.get_parameter_value("fadc_sum_bins") SimtelConfigWriter.write_light_pulse_table_gauss_exp_conv( @@ -386,6 +378,9 @@ def _sanitize_name(value): f"--angular-distribution {angular_distribution}", ] + def _sanitize_name(self, value): + return "".join(ch if (ch.isalnum() or ch in ("-", "_")) else "_" for ch in str(value)) + def _add_illuminator_command_options(self): """Get illuminator-specific command options for light emission script.""" pos = self.light_emission_config.get("light_source_position") @@ -456,26 +451,21 @@ def _make_simtel_script(self): if self.light_emission_config["light_source_type"] == "flat_fielding": options.append(("Bypass_Optics", "1")) - app_name = self._get_light_emission_application_name() - pref = self._get_prefix() + input_file = self.runner_service.get_file_name(file_type="iact_output") + output_file = self.runner_service.get_file_name(file_type="sim_telarray_output") + histo_file = self.runner_service.get_file_name(file_type="sim_telarray_histogram") options += [ ("power_law", "2.68"), - ("input_file", f"{self.output_directory}/{app_name}.iact.gz"), - ("output_file", f"{self.output_directory}/{pref}{app_name}.simtel.zst"), - ("histogram_file", f"{self.output_directory}/{pref}{app_name}.ctsim.hdata"), + ("input_file", f"{input_file}"), + ("output_file", f"{output_file}"), + ("histogram_file", f"{histo_file}"), ] parts += [f"-C {key}={value}" for key, value in options] return sim_telarray_env_as_string() + " ".join(parts) - def _get_simulation_output_filename(self): - """Get the filename of the simulation output.""" - app_name = self._get_light_emission_application_name() - pref = self._get_prefix() - return f"{self.output_directory}/{pref}{app_name}.simtel.zst" - def calculate_distance_focal_plane_calibration_device(self): """ Calculate distance between focal plane and calibration device. @@ -499,21 +489,14 @@ def _generate_lambertian_angular_distribution_table(self): Uses a pure cosine profile normalized to 1 at 0 deg and spans 0..90 deg by default. """ - base_dir = self.io_handler.get_output_directory("angular_distributions") - - def _sanitize_name(value): - return "".join(ch if (ch.isalnum() or ch in ("-", "_")) else "_" for ch in str(value)) - - tel = self.light_emission_config.get("telescope") or "telescope" - cal = self.light_emission_config.get("light_source") or "calibration" - fname = f"flasher_angular_distribution_{_sanitize_name(tel)}_{_sanitize_name(cal)}.dat" - table_path = base_dir / fname - SimtelConfigWriter.write_angular_distribution_table_lambertian( - file_path=table_path, + tel = self._sanitize_name(self.light_emission_config.get("telescope") or "telescope") + cal = self._sanitize_name(self.light_emission_config.get("light_source") or "calibration") + fname = f"flasher_angular_distribution_{tel}_{cal}.dat" + return SimtelConfigWriter.write_angular_distribution_table_lambertian( + file_path=self.io_handler.get_output_directory("light_emission") / fname, max_angle_deg=90.0, n_samples=100, ) - return str(table_path) def _get_angular_distribution_string_for_sim_telarray(self): """ diff --git a/tests/unit_tests/simtel/test_simulator_light_emission.py b/tests/unit_tests/simtel/test_simulator_light_emission.py index 3b463e56a3..0137e42a14 100644 --- a/tests/unit_tests/simtel/test_simulator_light_emission.py +++ b/tests/unit_tests/simtel/test_simulator_light_emission.py @@ -25,7 +25,7 @@ def simulator_instance(): def test_get_prefix_non_none_returns_with_underscore(simulator_instance): - simulator_instance.light_emission_config = {"output_prefix": "pre", "number_of_events": 1} + simulator_instance.light_emission_config = {"number_of_events": 1} assert simulator_instance._get_prefix() == "pre_" @@ -83,7 +83,6 @@ def test__get_simulation_output_filename(simulator_instance): # Test with flat_fielding and prefix simulator_instance.light_emission_config = { "light_source_type": "flat_fielding", - "output_prefix": "pre", } result = simulator_instance._get_simulation_output_filename() assert result == "/test/output/pre_ff-1m.simtel.zst" @@ -91,7 +90,6 @@ def test__get_simulation_output_filename(simulator_instance): # Test with no prefix (should return empty string for prefix) simulator_instance.light_emission_config = { "light_source_type": "flat_fielding", - "output_prefix": None, } result = simulator_instance._get_simulation_output_filename() assert result == "/test/output/ff-1m.simtel.zst" From adcf36c1621a458a7abd7175d057af6176814aec Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Mon, 12 Jan 2026 15:10:57 +0100 Subject: [PATCH 02/10] tests --- .../simtel/simulator_light_emission.py | 21 +++++++++---------- .../simtel/test_simulator_light_emission.py | 16 +++++++------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/simtools/simtel/simulator_light_emission.py b/src/simtools/simtel/simulator_light_emission.py index e298c501c5..e2b93ab4dc 100644 --- a/src/simtools/simtel/simulator_light_emission.py +++ b/src/simtools/simtel/simulator_light_emission.py @@ -81,7 +81,7 @@ def simulate(self): Path The output simtel file path. """ - run_script = self.prepare_script() + run_script = self.prepare_run() job_manager.submit( run_script, out_file=self.runner_service.get_file_name("sub_out"), @@ -92,9 +92,9 @@ def simulate(self): self._logger.warning(f"Expected sim_telarray output not found: {out}") return out - def prepare_script(self): + def prepare_run(self): """ - Build and return bash run script containing the light-emission command. + Prepare the bash run script containing the light-emission command. Returns ------- @@ -103,13 +103,18 @@ def prepare_script(self): """ script_file = self.runner_service.get_file_name(file_type="sub_script") output_file = self.runner_service.get_file_name(file_type="sim_telarray_output") - iact_output = self.runner_service.get_file_name(file_type="iact_output") if output_file.exists(): raise FileExistsError( f"sim_telarray output file exists, cancelling simulation: {output_file}" ) + lines = self.make_run_command() + script_file.write_text("".join(lines), encoding="utf-8") + return script_file - lines = [ + def make_run_command(self, run_number=None, input_file=None): # pylint: disable=unused-argument + """Light emission and sim_telarray run command.""" + iact_output = self.runner_service.get_file_name(file_type="iact_output") + return [ "#!/usr/bin/env bash\n", f"{self._make_light_emission_command(iact_output)}\n\n", ( @@ -120,9 +125,6 @@ def prepare_script(self): f"rm -f '{iact_output}'\n\n", ] - script_file.write_text("".join(lines), encoding="utf-8") - return script_file - def _get_light_emission_application_name(self): """ Return the LightEmission application and mode from type. @@ -553,6 +555,3 @@ def _get_pulse_shape_string_for_sim_telarray(self): if shape_out == "exponential" and expv is not None: return f"{shape_out}:{float(expv)}" return shape_out - - def make_run_command(self, run_number=None, input_file=None): - """Temporary stub to avoid errors.""" diff --git a/tests/unit_tests/simtel/test_simulator_light_emission.py b/tests/unit_tests/simtel/test_simulator_light_emission.py index 0137e42a14..122c5416f1 100644 --- a/tests/unit_tests/simtel/test_simulator_light_emission.py +++ b/tests/unit_tests/simtel/test_simulator_light_emission.py @@ -1101,8 +1101,8 @@ def test__get_light_emission_application_name(simulator_instance): assert result == "xyzls" -def test_prepare_script(simulator_instance, tmp_test_directory): - """Test prepare_script method.""" +def test_prepare_run(simulator_instance, tmp_test_directory): + """Test prepare_run method.""" # Setup mocks simulator_instance.output_directory = Path(tmp_test_directory) / "output" simulator_instance.light_emission_config = {"light_source_type": "illuminator"} @@ -1122,7 +1122,7 @@ def test_prepare_script(simulator_instance, tmp_test_directory): ), patch.object(simulator_instance, "_make_simtel_script", return_value="simtel_cmd"), ): - result = simulator_instance.prepare_script() + result = simulator_instance.prepare_run() # Verify return value is the script path expected_path = Path(tmp_test_directory) / "output" / "scripts" / "xyzls-light_emission.sh" @@ -1136,8 +1136,8 @@ def test_prepare_script(simulator_instance, tmp_test_directory): assert "simtel_cmd" in content -def test_prepare_script_output_file_exists(simulator_instance, tmp_test_directory): - """Test prepare_script method when output file already exists.""" +def test_prepare_run_output_file_exists(simulator_instance, tmp_test_directory): + """Test prepare_run method when output file already exists.""" simulator_instance.output_directory = Path(tmp_test_directory) / "output" simulator_instance.light_emission_config = {"light_source_type": "illuminator"} @@ -1151,7 +1151,7 @@ def test_prepare_script_output_file_exists(simulator_instance, tmp_test_director ): # Should raise FileExistsError with pytest.raises(FileExistsError, match="sim_telarray output file exists"): - simulator_instance.prepare_script() + simulator_instance.prepare_run() def test_simulate(simulator_instance, tmp_test_directory): @@ -1165,7 +1165,7 @@ def test_simulate(simulator_instance, tmp_test_directory): mock_output_file = Path(tmp_test_directory) / "output" / "test_output.simtel.gz" with ( - patch.object(simulator_instance, "prepare_script", return_value=mock_script_path), + patch.object(simulator_instance, "prepare_run", return_value=mock_script_path), patch.object( simulator_instance, "_get_simulation_output_filename", @@ -1205,7 +1205,7 @@ def test_simulate_output_file_missing(simulator_instance, tmp_test_directory): mock_output_file = Path(tmp_test_directory) / "output" / "missing_output.simtel.gz" with ( - patch.object(simulator_instance, "prepare_script", return_value=mock_script_path), + patch.object(simulator_instance, "prepare_run", return_value=mock_script_path), patch.object( simulator_instance, "_get_simulation_output_filename", From d2ba793239937490951ecb02ac1699c314b4cdd8 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Mon, 12 Jan 2026 17:13:54 +0100 Subject: [PATCH 03/10] unit tests --- .../runners/test_runner_services.py | 11 ++ .../simtel/test_simulator_light_emission.py | 187 ++++++++---------- 2 files changed, 90 insertions(+), 108 deletions(-) diff --git a/tests/unit_tests/runners/test_runner_services.py b/tests/unit_tests/runners/test_runner_services.py index 8c1929f857..ae0221d498 100644 --- a/tests/unit_tests/runners/test_runner_services.py +++ b/tests/unit_tests/runners/test_runner_services.py @@ -18,6 +18,7 @@ def runner_service(corsika_runner_mock_array_model): corsika_config=corsika_runner_mock_array_model.corsika_config, label="test-corsika-runner", run_type="corsika", + core_config=None, ) _runner_service.load_data_directory() return _runner_service @@ -30,6 +31,7 @@ def runner_service_mock_array_model(corsika_runner_mock_array_model): corsika_config=corsika_runner_mock_array_model.corsika_config, label="test-corsika-runner", run_type="corsika", + core_config=None, ) _runner_service.load_data_directory() return _runner_service @@ -42,6 +44,7 @@ def runner_service_config_only(corsika_config_mock_array_model): corsika_config=corsika_config_mock_array_model, label="test-corsika-runner", run_type="corsika", + core_config=None, ) @@ -54,6 +57,7 @@ def runner_service_pedestals(corsika_config_mock_array_model): corsika_config=corsika_config_pedestals, label="test-pedestals-runner", run_type="pedestals", + core_config=None, ) @@ -197,3 +201,10 @@ def test_get_sub_directory(runner_service_config_only, tmp_path): run_number=1, dir_path=tmp_path / "base" / "dir" ) assert dir_path == tmp_path / "base" / "dir" / "run000001" + + +def test__get_file_basename(runner_service_config_only): + runner_service_config_only.corsika_config = None + runner_service_config_only.core_config = None + with pytest.raises(ValueError, match=r"Either corsika_config or core_config must be provided."): + runner_service_config_only._get_file_basename(run_number=1) diff --git a/tests/unit_tests/simtel/test_simulator_light_emission.py b/tests/unit_tests/simtel/test_simulator_light_emission.py index 122c5416f1..6580de756b 100644 --- a/tests/unit_tests/simtel/test_simulator_light_emission.py +++ b/tests/unit_tests/simtel/test_simulator_light_emission.py @@ -13,7 +13,7 @@ @pytest.fixture def simulator_instance(): """Create a fresh mock SimulatorLightEmission instance for each test.""" - inst = object.__new__(SimulatorLightEmission) + inst = SimulatorLightEmission.__new__(SimulatorLightEmission) # Create fresh mocks for each test to avoid cross-test contamination inst.calibration_model = Mock() inst.telescope_model = Mock() @@ -21,18 +21,14 @@ def simulator_instance(): inst.light_emission_config = {} inst.output_directory = "/test/output" inst._logger = Mock() + inst.runner_service = Mock() + inst.io_handler = Mock() return inst -def test_get_prefix_non_none_returns_with_underscore(simulator_instance): - simulator_instance.light_emission_config = {"number_of_events": 1} - assert simulator_instance._get_prefix() == "pre_" - - @patch.object(SimulatorLightEmission, "_get_telescope_pointing") @patch.object(SimulatorLightEmission, "_get_light_emission_application_name") -@patch.object(SimulatorLightEmission, "_get_prefix") -def test__make_simtel_script(mock_prefix, mock_app_name, mock_pointing, simulator_instance): +def test__make_simtel_script(mock_app_name, mock_pointing, simulator_instance): """Test _make_simtel_script method with different conditions.""" simulator_instance.telescope_model.config_file_directory = "/mock/config" simulator_instance.telescope_model.config_file_path = "/mock/config/telescope.cfg" @@ -44,7 +40,6 @@ def test__make_simtel_script(mock_prefix, mock_app_name, mock_pointing, simulato mock_pointing.return_value = [10.5, 20.0] mock_app_name.return_value = "test-app" - mock_prefix.return_value = "test_" def test__make_simtel_script_bypass_optics_condition(simulator_instance): @@ -64,7 +59,6 @@ def test__make_simtel_script_bypass_optics_condition(simulator_instance): patch.object( simulator_instance, "_get_light_emission_application_name", return_value="ff-1m" ), - patch.object(simulator_instance, "_get_prefix", return_value=""), ): # Test flat_fielding - should include Bypass_Optics simulator_instance.light_emission_config = {"light_source_type": "flat_fielding"} @@ -79,22 +73,6 @@ def test__make_simtel_script_bypass_optics_condition(simulator_instance): assert "Bypass_Optics=1" not in options -def test__get_simulation_output_filename(simulator_instance): - # Test with flat_fielding and prefix - simulator_instance.light_emission_config = { - "light_source_type": "flat_fielding", - } - result = simulator_instance._get_simulation_output_filename() - assert result == "/test/output/pre_ff-1m.simtel.zst" - - # Test with no prefix (should return empty string for prefix) - simulator_instance.light_emission_config = { - "light_source_type": "flat_fielding", - } - result = simulator_instance._get_simulation_output_filename() - assert result == "/test/output/ff-1m.simtel.zst" - - def test_calculate_distance_focal_plane_calibration_device(simulator_instance): simulator_instance.telescope_model.get_parameter_value_with_unit.return_value = 10 * u.m simulator_instance.calibration_model.get_parameter_value_with_unit.return_value = [ @@ -157,8 +135,8 @@ def test__get_angular_distribution_string_for_sim_telarray_lambertian( result = simulator_instance._get_angular_distribution_string_for_sim_telarray() # Result should be a path to the generated table - assert result.endswith(".dat") table_path = Path(result) + assert str(table_path).endswith(".dat") assert table_path.exists() content = table_path.read_text().splitlines() assert content[0].startswith("# angle[deg] relative_intensity") @@ -908,7 +886,7 @@ def test__get_site_command(simulator_instance, tmp_test_directory): def test__make_light_emission_script(simulator_instance): - """Test _make_light_emission_script method.""" + """Test _make_light_emission_command method.""" simulator_instance.output_directory = "/output" simulator_instance.label = "test_label" @@ -940,11 +918,11 @@ def test__make_light_emission_script(simulator_instance): simulator_instance.light_emission_config = {"light_source_type": "flat_fielding"} mock_settings.config.sim_telarray_path = Path("/mock/simtel/sim_telarray") - result = simulator_instance._make_light_emission_script() + result = simulator_instance._make_light_emission_command("/output/ff-1m.iact.gz") expected = ( "/mock/simtel/sim_telarray/LightEmission/ff-1m -I. --altitude 2200 " - "--photons 1000000 -o /output/ff-1m.iact.gz \n" + "--photons 1000000 -o /output/ff-1m.iact.gz" ) assert result == expected @@ -971,12 +949,12 @@ def test__make_light_emission_script(simulator_instance): simulator_instance.light_emission_config = {"light_source_type": "illuminator"} mock_settings.config.sim_telarray_path = Path("/mock/simtel/sim_telarray") - result = simulator_instance._make_light_emission_script() + result = simulator_instance._make_light_emission_command("/output/illuminator-app.iact.gz") expected = ( "/mock/simtel/sim_telarray/LightEmission/illuminator-app -h 2200 " "-x 100 -y 200 -A /config/dir/atm_profile.dat " - "-o /output/illuminator-app.iact.gz \n" + "-o /output/illuminator-app.iact.gz" ) assert result == expected @@ -1107,18 +1085,26 @@ def test_prepare_run(simulator_instance, tmp_test_directory): simulator_instance.output_directory = Path(tmp_test_directory) / "output" simulator_instance.light_emission_config = {"light_source_type": "illuminator"} + script_dir = Path(tmp_test_directory) / "output" / "scripts" + script_dir.mkdir(parents=True, exist_ok=True) + script_path = script_dir / "xyzls-light_emission.sh" + + # Mock runner_service.get_file_name to return paths + def get_file_name_side_effect(file_type): + if file_type == "sub_script": + return script_path + if file_type == "sim_telarray_output": + return Path(tmp_test_directory) / "output" / "test_output.simtel.gz" + if file_type == "iact_output": + return Path(tmp_test_directory) / "output" / "iact.dat" + return Path(tmp_test_directory) / "output" / f"{file_type}.tmp" + + simulator_instance.runner_service.get_file_name.side_effect = get_file_name_side_effect + # Mock the internal methods with ( patch.object( - simulator_instance, "_get_light_emission_application_name", return_value="xyzls" - ), - patch.object( - simulator_instance, - "_get_simulation_output_filename", - return_value="test_output.simtel.gz", - ), - patch.object( - simulator_instance, "_make_light_emission_script", return_value="light_emission_cmd" + simulator_instance, "_make_light_emission_command", return_value="light_emission_cmd" ), patch.object(simulator_instance, "_make_simtel_script", return_value="simtel_cmd"), ): @@ -1146,12 +1132,21 @@ def test_prepare_run_output_file_exists(simulator_instance, tmp_test_directory): output_file_path.parent.mkdir(parents=True, exist_ok=True) output_file_path.touch() # Create the file - with patch.object( - simulator_instance, "_get_simulation_output_filename", return_value=str(output_file_path) - ): - # Should raise FileExistsError - with pytest.raises(FileExistsError, match="sim_telarray output file exists"): - simulator_instance.prepare_run() + # Setup mock to return the output file that already exists + def get_file_name_side_effect(file_type): + if file_type == "sim_telarray_output": + return output_file_path + if file_type == "sub_script": + return Path(tmp_test_directory) / "output" / "script.sh" + if file_type == "iact_output": + return Path(tmp_test_directory) / "output" / "iact.dat" + return Path(tmp_test_directory) / "output" / f"{file_type}.tmp" + + simulator_instance.runner_service.get_file_name.side_effect = get_file_name_side_effect + + # Should raise FileExistsError + with pytest.raises(FileExistsError, match="sim_telarray output file exists"): + simulator_instance.prepare_run() def test_simulate(simulator_instance, tmp_test_directory): @@ -1162,66 +1157,42 @@ def test_simulate(simulator_instance, tmp_test_directory): # Mock the methods called by simulate mock_script_path = Path(tmp_test_directory) / "output" / "scripts" / "test_script.sh" + mock_script_path.parent.mkdir(parents=True, exist_ok=True) mock_output_file = Path(tmp_test_directory) / "output" / "test_output.simtel.gz" - with ( - patch.object(simulator_instance, "prepare_run", return_value=mock_script_path), - patch.object( - simulator_instance, - "_get_simulation_output_filename", - return_value=str(mock_output_file), - ), - patch("simtools.job_execution.job_manager.submit") as mock_job_submit, - ): - # Create the output file to simulate successful run + # Setup mock to return the script and output file + def get_file_name_side_effect(file_type): + if file_type == "sub_script": + return mock_script_path + if file_type == "sim_telarray_output": + return mock_output_file + if file_type == "sub_out": + return Path(tmp_test_directory) / "output" / "logfile.log" + if file_type == "sub_err": + return Path(tmp_test_directory) / "output" / "logfile.err" + if file_type == "iact_output": + return Path(tmp_test_directory) / "output" / "iact.dat" + return Path(tmp_test_directory) / "output" / f"{file_type}.tmp" + + simulator_instance.runner_service.get_file_name.side_effect = get_file_name_side_effect + + with patch("simtools.job_execution.job_manager.submit") as mock_job_submit: + # Mock make_run_command to return a simple script + with patch.object( + simulator_instance, "make_run_command", return_value=["#!/bin/bash\n", "echo test\n"] + ): + result = simulator_instance.simulate() + + # Create the output file to simulate successful run (this happens during simulate()) mock_output_file.parent.mkdir(parents=True, exist_ok=True) mock_output_file.touch() - result = simulator_instance.simulate() - # Verify job_manager.submit was called correctly mock_job_submit.assert_called_once() call_args = mock_job_submit.call_args assert call_args[0][0] == mock_script_path # First positional arg is the script - # Check the out_file and err_file arguments - expected_log_path = Path(tmp_test_directory) / "output" / "logfile.log" - expected_err_path = expected_log_path.with_suffix(".err") - assert call_args[1]["out_file"] == expected_log_path - assert call_args[1]["err_file"] == expected_err_path - - # Verify return value - assert result == mock_output_file - - -def test_simulate_output_file_missing(simulator_instance, tmp_test_directory): - """Test simulate method when output file is missing (logs warning).""" - # Setup - simulator_instance.output_directory = Path(tmp_test_directory) / "output" - simulator_instance.output_directory.mkdir(parents=True, exist_ok=True) - - # Mock the methods called by simulate - mock_script_path = Path(tmp_test_directory) / "output" / "scripts" / "test_script.sh" - mock_output_file = Path(tmp_test_directory) / "output" / "missing_output.simtel.gz" - - with ( - patch.object(simulator_instance, "prepare_run", return_value=mock_script_path), - patch.object( - simulator_instance, - "_get_simulation_output_filename", - return_value=str(mock_output_file), - ), - patch("simtools.job_execution.job_manager.submit"), - ): - # Don't create the output file to simulate missing output - result = simulator_instance.simulate() - - # Verify warning was logged - simulator_instance._logger.warning.assert_called_once_with( - f"Expected sim_telarray output not found: {mock_output_file}" - ) - - # Should still return the expected path + # Verify return value - result will be the mock_output_file if it exists assert result == mock_output_file @@ -1412,19 +1383,17 @@ def test__write_telescope_position_file(simulator_instance): mock_radius, # telescope_sphere_radius ] + # Mock io_handler to return a proper Path object + mock_output_dir = Path("/tmp/test_output") + simulator_instance.io_handler.get_output_directory.return_value = mock_output_dir + expected_file = mock_output_dir / "telescope_position.dat" + # Mock file writing - mock_file = Mock() - with ( - patch.object(Path, "joinpath", return_value=mock_file) as mock_joinpath, - patch.object(mock_file, "write_text") as mock_write, - ): + with patch.object(Path, "write_text") as mock_write: result = simulator_instance._write_telescope_position_file() # Should return the telescope position file path - assert result == mock_file - - # Should create file in output directory - mock_joinpath.assert_called_once_with("telescope_position.dat") + assert result == expected_file # Should write coordinates and radius in correct format expected_content = "100.0 200.0 300.0 1500.0\n" @@ -1500,17 +1469,19 @@ def test__calibration_pointing_direction_with_custom_params(simulator_instance): custom_y = 0 * u.m custom_z = 5 * u.m - pointing_vector, angles = simulator_instance._calibration_pointing_direction( + result = simulator_instance._calibration_pointing_direction( x_cal=custom_x, y_cal=custom_y, z_cal=custom_z ) + # The method returns a tuple: (pointing_vector, [theta, phi, source_theta, source_phi]) + pointing_vector, _ = result + # Verify calculations - direction vector is [5, 5, -5] expected_direction = np.array([5.0, 5.0, -5.0]) expected_norm = np.linalg.norm(expected_direction) expected_pointing = np.round(expected_direction / expected_norm, 6).tolist() assert pointing_vector == expected_pointing - assert len(angles) == 4 # Verify calibration model was NOT called (custom params provided) simulator_instance.calibration_model.get_parameter_value_with_unit.assert_not_called() From c52b82f155920202e0dd72a722c174b38939f5fb Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Tue, 13 Jan 2026 09:44:29 +0100 Subject: [PATCH 04/10] paths --- src/simtools/runners/corsika_runner.py | 12 +++-- src/simtools/runners/corsika_simtel_runner.py | 2 +- src/simtools/runners/runner_services.py | 50 ++++++++++--------- src/simtools/runners/simtel_runner.py | 15 ++---- src/simtools/simtel/simulator_array.py | 6 +-- .../simtel/simulator_light_emission.py | 2 +- .../runners/test_runner_services.py | 4 -- .../unit_tests/runners/test_simtel_runner.py | 2 +- 8 files changed, 41 insertions(+), 52 deletions(-) diff --git a/src/simtools/runners/corsika_runner.py b/src/simtools/runners/corsika_runner.py index f6253cb89a..f1fafccd30 100644 --- a/src/simtools/runners/corsika_runner.py +++ b/src/simtools/runners/corsika_runner.py @@ -45,7 +45,7 @@ def __init__( self._use_multipipe = use_multipipe self.curved_atmosphere_min_zenith_angle = curved_atmosphere_min_zenith_angle - self.runner_service = RunnerServices(corsika_config, "corsika", label) + self.runner_service = RunnerServices(corsika_config, run_type="corsika", label=label) self.file_list = None def prepare_run(self, run_number, sub_script, extra_commands=None, corsika_file=None): @@ -82,7 +82,7 @@ def prepare_run(self, run_number, sub_script, extra_commands=None, corsika_file= "corsika_output", run_number=run_number ).parent - self._export_run_script(sub_script, corsika_run_dir, extra_commands) + self._export_run_script(run_number, sub_script, corsika_run_dir, extra_commands) def _corsika_executable(self): """Get the CORSIKA executable path.""" @@ -92,10 +92,12 @@ def _corsika_executable(self): self._logger.debug("Using flat-atmosphere CORSIKA binary.") return Path(settings.config.corsika_exe) - def _export_run_script(self, sub_script, corsika_run_dir, extra_commands): + def _export_run_script(self, run_number, sub_script, corsika_run_dir, extra_commands): """Export CORSIKA run script.""" - corsika_log_file = self.runner_service.get_file_name("corsika_log").with_suffix("") - corsika_input = self.runner_service.get_file_name("corsika_input") + corsika_log_file = self.runner_service.get_file_name( + "corsika_log", run_number=run_number + ).with_suffix("") # remove .gz from log file + corsika_input = self.runner_service.get_file_name("corsika_input", run_number=run_number) sub_script = Path(sub_script) with open(sub_script, "w", encoding="utf-8") as file: file.write("#!/usr/bin/env bash\n") diff --git a/src/simtools/runners/corsika_simtel_runner.py b/src/simtools/runners/corsika_simtel_runner.py index 2473561aad..b739547e2b 100644 --- a/src/simtools/runners/corsika_simtel_runner.py +++ b/src/simtools/runners/corsika_simtel_runner.py @@ -53,7 +53,7 @@ def __init__( self.sequential = "--sequential" if sequential else "" self.runner_service = runner_services.RunnerServices( - self.base_corsika_config, "multi_pipe", label + self.base_corsika_config, run_type="multi_pipe", label=label ) self.file_list = None diff --git a/src/simtools/runners/runner_services.py b/src/simtools/runners/runner_services.py index 2509e33ae5..dae089887d 100644 --- a/src/simtools/runners/runner_services.py +++ b/src/simtools/runners/runner_services.py @@ -2,6 +2,7 @@ import logging +from simtools.corsika.corsika_config import CorsikaConfig from simtools.io import io_handler _logger = logging.getLogger(__name__) @@ -106,22 +107,19 @@ class RunnerServices: Parameters ---------- - corsika_config : CorsikaConfig, list of CorsikaConfig - Configuration parameters for CORSIKA. - core_config : dict - Core configuration parameters (less specific than CorsikaConfig). + config: CorsikaConfig, dict + Configuration parameters. run_type : str Type of simulation runner. label : str Label. """ - def __init__(self, corsika_config, core_config, run_type, label=None): + def __init__(self, config, run_type, label=None): """Initialize RunnerServices.""" self._logger = logging.getLogger(__name__) self.label = label - self.corsika_config = corsika_config - self.core_config = core_config + self.config = config self.run_type = run_type self.directory = self.load_data_directory() @@ -137,6 +135,7 @@ def load_data_directory(self): ioh = io_handler.IOHandler() directory = ioh.get_output_directory(self.run_type) self._logger.debug(f"Data directories for {self.run_type}: {directory}") + return directory def load_files(self, run_number=None): @@ -179,15 +178,15 @@ def _get_file_basename(self, run_number, is_multi_pipe=False): str Base name for the simulation files. """ - if self.corsika_config: + if isinstance(self.config, CorsikaConfig): return self._get_file_base_name_from_corsika_config(run_number, is_multi_pipe) - if self.core_config: + if isinstance(self.config, dict): return self._get_file_base_name_from_core_config() - raise ValueError("Either corsika_config or core_config must be provided.") + raise ValueError(f"Invalid configuration type: {type(self.config)}") def _get_file_base_name_from_core_config(self): """Get file base name from core configuration.""" - cfg = self.core_config + cfg = self.config parts = [ f"{cfg.get('run_mode', '')}_", f"{self._get_run_number_string(cfg.get('run_number'))}_" if "run_number" in cfg else "", @@ -202,13 +201,13 @@ def _get_file_base_name_from_core_config(self): def _get_file_base_name_from_corsika_config(self, run_number, is_multi_pipe=False): """Get file base name from CORSIKA configuration.""" - zenith = self.corsika_config.get_config_parameter("THETAP")[0] - vc_high = self.corsika_config.get_config_parameter("VIEWCONE")[1] + zenith = self.config.get_config_parameter("THETAP")[0] + vc_high = self.config.get_config_parameter("VIEWCONE")[1] - if self.corsika_config.run_mode is not None and self.corsika_config.run_mode != "": - primary_name = self.corsika_config.run_mode + if self.config.run_mode is not None and self.config.run_mode != "": + primary_name = self.config.run_mode else: - primary_name = self.corsika_config.primary_particle.name + primary_name = self.config.primary_particle.name if primary_name == "gamma" and vc_high > 0: primary_name = "gamma_diffuse" @@ -219,10 +218,10 @@ def _get_file_base_name_from_corsika_config(self, run_number, is_multi_pipe=Fals return ( prefix + f"za{round(zenith):02}deg_" - + f"azm{self.corsika_config.azimuth_angle:03}deg_" - + f"{self.corsika_config.array_model.site}_" - + f"{self.corsika_config.array_model.layout_name}_" - + (self.corsika_config.array_model.model_version if not is_multi_pipe else "") + + f"azm{self.config.azimuth_angle:03}deg_" + + f"{self.config.array_model.site}_" + + f"{self.config.array_model.layout_name}_" + + (self.config.array_model.model_version if not is_multi_pipe else "") + file_label ) @@ -325,7 +324,10 @@ def get_resources(self, sub_out_file): if runtime is None: _logger.debug("RUNTIME was not found in run log file") - return { - "runtime": runtime, - "n_events": int(self.corsika_config.get_config_parameter("NSHOW")), - } + if isinstance(self.config, CorsikaConfig): + return { + "runtime": runtime, + "n_events": int(self.config.get_config_parameter("NSHOW")), + } + self._logger.warning("Number of events cannot be determined from non-CORSIKA config.") + return {"runtime": runtime, "n_events": None} diff --git a/src/simtools/runners/simtel_runner.py b/src/simtools/runners/simtel_runner.py index c988e9a867..c106750b48 100644 --- a/src/simtools/runners/simtel_runner.py +++ b/src/simtools/runners/simtel_runner.py @@ -26,15 +26,13 @@ class SimtelRunner: ---------- label: str Instance label. Important for output file naming. - corsika_config: CorsikaConfig - CORSIKA configuration. - core_config: dict - Core configuration parameters (less specific than CorsikaConfig). + config: CorsikaConfig or dict + Configuration parameters. is_calibration_run: bool Flag to indicate if this is a calibration run. """ - def __init__(self, label=None, corsika_config=None, core_config=None, is_calibration_run=False): + def __init__(self, label=None, config=None, is_calibration_run=False): """Initialize SimtelRunner.""" self._logger = logging.getLogger(__name__) @@ -44,12 +42,7 @@ def __init__(self, label=None, corsika_config=None, core_config=None, is_calibra self.runs_per_set = 1 - self.runner_service = RunnerServices( - corsika_config=corsika_config, - core_config=core_config, - run_type="sim_telarray", - label=label, - ) + self.runner_service = RunnerServices(config, run_type="sim_telarray", label=label) self.file_list = None def run(self, test=False, input_file=None, run_number=None): diff --git a/src/simtools/simtel/simulator_array.py b/src/simtools/simtel/simulator_array.py index 25c9863000..e2dcf6e201 100644 --- a/src/simtools/simtel/simulator_array.py +++ b/src/simtools/simtel/simulator_array.py @@ -33,11 +33,7 @@ def __init__( is_calibration_run=False, ): """Initialize SimulatorArray.""" - super().__init__( - label=label, - corsika_config=corsika_config, - is_calibration_run=is_calibration_run, - ) + super().__init__(label=label, config=corsika_config, is_calibration_run=is_calibration_run) self.sim_telarray_seeds = sim_telarray_seeds self.corsika_config = corsika_config diff --git a/src/simtools/simtel/simulator_light_emission.py b/src/simtools/simtel/simulator_light_emission.py index e2b93ab4dc..d27bce59f2 100644 --- a/src/simtools/simtel/simulator_light_emission.py +++ b/src/simtools/simtel/simulator_light_emission.py @@ -35,7 +35,7 @@ def __init__(self, light_emission_config, label=None): self._logger = logging.getLogger(__name__) self.io_handler = io_handler.IOHandler() - super().__init__(label=label, core_config=light_emission_config) + super().__init__(label=label, config=light_emission_config) self.telescope_model, self.site_model, self.calibration_model = ( initialize_simulation_models( diff --git a/tests/unit_tests/runners/test_runner_services.py b/tests/unit_tests/runners/test_runner_services.py index ae0221d498..374825790c 100644 --- a/tests/unit_tests/runners/test_runner_services.py +++ b/tests/unit_tests/runners/test_runner_services.py @@ -18,7 +18,6 @@ def runner_service(corsika_runner_mock_array_model): corsika_config=corsika_runner_mock_array_model.corsika_config, label="test-corsika-runner", run_type="corsika", - core_config=None, ) _runner_service.load_data_directory() return _runner_service @@ -31,7 +30,6 @@ def runner_service_mock_array_model(corsika_runner_mock_array_model): corsika_config=corsika_runner_mock_array_model.corsika_config, label="test-corsika-runner", run_type="corsika", - core_config=None, ) _runner_service.load_data_directory() return _runner_service @@ -44,7 +42,6 @@ def runner_service_config_only(corsika_config_mock_array_model): corsika_config=corsika_config_mock_array_model, label="test-corsika-runner", run_type="corsika", - core_config=None, ) @@ -57,7 +54,6 @@ def runner_service_pedestals(corsika_config_mock_array_model): corsika_config=corsika_config_pedestals, label="test-pedestals-runner", run_type="pedestals", - core_config=None, ) diff --git a/tests/unit_tests/runners/test_simtel_runner.py b/tests/unit_tests/runners/test_simtel_runner.py index 6983687938..e2bcd57272 100644 --- a/tests/unit_tests/runners/test_simtel_runner.py +++ b/tests/unit_tests/runners/test_simtel_runner.py @@ -11,7 +11,7 @@ @pytest.fixture def simtel_runner(corsika_config_mock_array_model): - return SimtelRunner(label="test", corsika_config=corsika_config_mock_array_model) + return SimtelRunner(label="test", config=corsika_config_mock_array_model) def test_run(simtel_runner, caplog, mocker): From b8d4661c012c063c5cbace9ec19f16de3272ffe9 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Tue, 13 Jan 2026 12:19:45 +0100 Subject: [PATCH 05/10] tests --- src/simtools/applications/simulate_flasher.py | 2 + .../simtel/simulator_light_emission.py | 39 ++++++++++++------- ...late_flasher_full_simulation_LST_North.yml | 4 +- ...late_flasher_full_simulation_LST_South.yml | 4 +- ...e_flasher_full_simulation_mst_flashcam.yml | 6 +-- ..._flasher_full_simulation_mst_nectarcam.yml | 6 +-- ...late_illuminator_configurable_position.yml | 6 +-- .../config/simulate_illuminator_layout.yml | 6 +-- .../runners/test_runner_services.py | 23 ++++++----- .../simtel/test_simulator_light_emission.py | 26 +++++++++---- tests/unit_tests/test_simulator.py | 5 ++- 11 files changed, 75 insertions(+), 52 deletions(-) diff --git a/src/simtools/applications/simulate_flasher.py b/src/simtools/applications/simulate_flasher.py index 8e294e745a..6110d18d30 100644 --- a/src/simtools/applications/simulate_flasher.py +++ b/src/simtools/applications/simulate_flasher.py @@ -119,6 +119,8 @@ def main(): raise ValueError(f"Unsupported run_mode: {app_context.args['run_mode']}") light_source.simulate() + light_source.verify_simulations() + app_context.logger.info("Flasher simulation completed.") diff --git a/src/simtools/simtel/simulator_light_emission.py b/src/simtools/simtel/simulator_light_emission.py index d27bce59f2..04c0f2559a 100644 --- a/src/simtools/simtel/simulator_light_emission.py +++ b/src/simtools/simtel/simulator_light_emission.py @@ -11,6 +11,7 @@ from simtools.io import io_handler from simtools.job_execution import job_manager from simtools.model.model_utils import initialize_simulation_models +from simtools.runners import runner_services from simtools.runners.simtel_runner import SimtelRunner, sim_telarray_env_as_string from simtools.simtel.simtel_config_writer import SimtelConfigWriter from simtools.utils.geometry import fiducial_radius_from_shape @@ -36,6 +37,9 @@ def __init__(self, light_emission_config, label=None): self.io_handler = io_handler.IOHandler() super().__init__(label=label, config=light_emission_config) + self.job_files = runner_services.RunnerServices( + light_emission_config, run_type="sub", label=label + ) self.telescope_model, self.site_model, self.calibration_model = ( initialize_simulation_models( @@ -73,24 +77,13 @@ def _initialize_light_emission_configuration(self, config): return config def simulate(self): - """ - Simulate light emission. - - Returns - ------- - Path - The output simtel file path. - """ + """Simulate light emission.""" run_script = self.prepare_run() job_manager.submit( run_script, - out_file=self.runner_service.get_file_name("sub_out"), - err_file=self.runner_service.get_file_name("sub_err"), + out_file=self.job_files.get_file_name("sub_out"), + err_file=self.job_files.get_file_name("sub_err"), ) - out = Path(self.runner_service.get_file_name(file_type="sim_telarray_output")) - if not out.exists(): - self._logger.warning(f"Expected sim_telarray output not found: {out}") - return out def prepare_run(self): """ @@ -101,7 +94,7 @@ def prepare_run(self): Path Full path of the run script. """ - script_file = self.runner_service.get_file_name(file_type="sub_script") + script_file = self.job_files.get_file_name(file_type="sub_script") output_file = self.runner_service.get_file_name(file_type="sim_telarray_output") if output_file.exists(): raise FileExistsError( @@ -555,3 +548,19 @@ def _get_pulse_shape_string_for_sim_telarray(self): if shape_out == "exponential" and expv is not None: return f"{shape_out}:{float(expv)}" return shape_out + + def verify_simulations(self): + """ + Verify that the simulations were successful. + + Returns + ------- + bool + True if simulations were successful, False otherwise. + """ + out = Path(self.runner_service.get_file_name(file_type="sim_telarray_output")) + if not out.exists(): + self._logger.error(f"Expected sim_telarray output not found: {out}") + return False + self._logger.info(f"sim_telarray output found: {out}") + return True diff --git a/tests/integration_tests/config/simulate_flasher_full_simulation_LST_North.yml b/tests/integration_tests/config/simulate_flasher_full_simulation_LST_North.yml index deabe0ecd2..3e7437974b 100644 --- a/tests/integration_tests/config/simulate_flasher_full_simulation_LST_North.yml +++ b/tests/integration_tests/config/simulate_flasher_full_simulation_LST_North.yml @@ -12,8 +12,8 @@ applications: test: true log_level: DEBUG integration_tests: - - output_file: logfile.log - - output_file: ff-1m.simtel.zst + - output_file: sub/full_simulation_run000010_North_7.0.0_simulate_flasher.out + - output_file: sim_telarray/full_simulation_run000010_North_7.0.0_simulate_flasher.simtel.zst model_version_use_current: true test_name: full-simulation_lsfn_design schema_name: application_workflow.metaschema diff --git a/tests/integration_tests/config/simulate_flasher_full_simulation_LST_South.yml b/tests/integration_tests/config/simulate_flasher_full_simulation_LST_South.yml index 54b0247726..ce3ec7e420 100644 --- a/tests/integration_tests/config/simulate_flasher_full_simulation_LST_South.yml +++ b/tests/integration_tests/config/simulate_flasher_full_simulation_LST_South.yml @@ -12,8 +12,8 @@ applications: test: true log_level: DEBUG integration_tests: - - output_file: logfile.log - - output_file: ff-1m.simtel.zst + - output_file: sub/full_simulation_run000010_South_7.0.0_simulate_flasher.out + - output_file: sim_telarray/full_simulation_run000010_South_7.0.0_simulate_flasher.simtel.zst model_version_use_current: true test_name: full-simulation_lsfs_design schema_name: application_workflow.metaschema diff --git a/tests/integration_tests/config/simulate_flasher_full_simulation_mst_flashcam.yml b/tests/integration_tests/config/simulate_flasher_full_simulation_mst_flashcam.yml index 4a289bc318..3e47728d28 100644 --- a/tests/integration_tests/config/simulate_flasher_full_simulation_mst_flashcam.yml +++ b/tests/integration_tests/config/simulate_flasher_full_simulation_mst_flashcam.yml @@ -13,9 +13,9 @@ applications: log_level: DEBUG integration_tests: - test_output_files: - - file: logfile.log + - file: sub/full_simulation_run000010_South_7.0.0_simulate_flasher.out path_descriptor: output_path - - file: ff-1m.simtel.zst + - file: sim_telarray/full_simulation_run000010_South_7.0.0_simulate_flasher.simtel.zst path_descriptor: output_path - expected_log_output: pattern: @@ -25,7 +25,7 @@ applications: forbidden_pattern: - "Error" - "had less photons than required and were skipped" - file: logfile.log + file: sub/full_simulation_run000010_South_7.0.0_simulate_flasher.out path_descriptor: output_path model_version_use_current: true test_name: full-simulation-msfx-flashcam-7.0.0 diff --git a/tests/integration_tests/config/simulate_flasher_full_simulation_mst_nectarcam.yml b/tests/integration_tests/config/simulate_flasher_full_simulation_mst_nectarcam.yml index 796cf25d88..ec1e317166 100644 --- a/tests/integration_tests/config/simulate_flasher_full_simulation_mst_nectarcam.yml +++ b/tests/integration_tests/config/simulate_flasher_full_simulation_mst_nectarcam.yml @@ -13,9 +13,9 @@ applications: log_level: DEBUG integration_tests: - test_output_files: - - file: logfile.log + - file: sub/full_simulation_run000010_North_7.0.0_simulate_flasher.out path_descriptor: output_path - - file: ff-1m.simtel.zst + - file: sim_telarray/full_simulation_run000010_North_7.0.0_simulate_flasher.simtel.zst path_descriptor: output_path - expected_log_output: pattern: @@ -25,7 +25,7 @@ applications: forbidden_pattern: - "Error" - "had less photons than required and were skipped" - file: logfile.log + file: sub/full_simulation_run000010_North_7.0.0_simulate_flasher.out path_descriptor: output_path model_version_use_current: true test_name: full-simulation_msfx_nectarcam-7.0.0 diff --git a/tests/integration_tests/config/simulate_illuminator_configurable_position.yml b/tests/integration_tests/config/simulate_illuminator_configurable_position.yml index da12cd182d..905b8419cc 100644 --- a/tests/integration_tests/config/simulate_illuminator_configurable_position.yml +++ b/tests/integration_tests/config/simulate_illuminator_configurable_position.yml @@ -19,9 +19,9 @@ applications: log_level: DEBUG integration_tests: - test_output_files: - - file: logfile.log + - file: sub/illuminator_North_7.0.0_simulate_illuminator.out path_descriptor: output_path - - file: xyzls.simtel.zst + - file: sim_telarray/illuminator_North_7.0.0_simulate_illuminator.simtel.zst path_descriptor: output_path - expected_log_output: pattern: @@ -33,7 +33,7 @@ applications: - "Finished." forbidden_pattern: - "Error" - file: logfile.log + file: sub/illuminator_North_7.0.0_simulate_illuminator.out path_descriptor: output_path model_version_use_current: true test_name: run-configurable-position diff --git a/tests/integration_tests/config/simulate_illuminator_layout.yml b/tests/integration_tests/config/simulate_illuminator_layout.yml index 05f84a359a..ebc02cbdca 100644 --- a/tests/integration_tests/config/simulate_illuminator_layout.yml +++ b/tests/integration_tests/config/simulate_illuminator_layout.yml @@ -11,9 +11,9 @@ applications: log_level: DEBUG integration_tests: - test_output_files: - - file: logfile.log + - file: sub/illuminator_North_7.0.0_simulate_illuminator.out path_descriptor: output_path - - file: xyzls.simtel.zst + - file: sim_telarray/illuminator_North_7.0.0_simulate_illuminator.simtel.zst path_descriptor: output_path - expected_log_output: pattern: @@ -25,7 +25,7 @@ applications: - "Finished." forbidden_pattern: - "Error" - file: logfile.log + file: sub/illuminator_North_7.0.0_simulate_illuminator.out path_descriptor: output_path model_version_use_current: true test_name: run-layout diff --git a/tests/unit_tests/runners/test_runner_services.py b/tests/unit_tests/runners/test_runner_services.py index 374825790c..2b9b0782ab 100644 --- a/tests/unit_tests/runners/test_runner_services.py +++ b/tests/unit_tests/runners/test_runner_services.py @@ -15,7 +15,7 @@ def runner_service(corsika_runner_mock_array_model): """Runner services object for corsika.""" _runner_service = runner_services.RunnerServices( - corsika_config=corsika_runner_mock_array_model.corsika_config, + config=corsika_runner_mock_array_model.corsika_config, label="test-corsika-runner", run_type="corsika", ) @@ -27,7 +27,7 @@ def runner_service(corsika_runner_mock_array_model): def runner_service_mock_array_model(corsika_runner_mock_array_model): """Runner services object for corsika.""" _runner_service = runner_services.RunnerServices( - corsika_config=corsika_runner_mock_array_model.corsika_config, + config=corsika_runner_mock_array_model.corsika_config, label="test-corsika-runner", run_type="corsika", ) @@ -39,7 +39,7 @@ def runner_service_mock_array_model(corsika_runner_mock_array_model): def runner_service_config_only(corsika_config_mock_array_model): """Runner services object with simplified config.""" return runner_services.RunnerServices( - corsika_config=corsika_config_mock_array_model, + config=corsika_config_mock_array_model, label="test-corsika-runner", run_type="corsika", ) @@ -51,7 +51,7 @@ def runner_service_pedestals(corsika_config_mock_array_model): corsika_config_pedestals = copy.deepcopy(corsika_config_mock_array_model) corsika_config_pedestals.run_mode = "pedestals" return runner_services.RunnerServices( - corsika_config=corsika_config_pedestals, + config=corsika_config_pedestals, label="test-pedestals-runner", run_type="pedestals", ) @@ -66,7 +66,7 @@ def runner_service_config_only_diffuse_gamma(corsika_config_mock_array_model): } return runner_services.RunnerServices( - corsika_config=corsika_config_mock_array_model, + config=corsika_config_mock_array_model, label="test-corsika-runner", run_type="corsika", ) @@ -82,7 +82,7 @@ def file_base_name(model_version): def test_init_runner_services(runner_service_config_only): assert runner_service_config_only.label == "test-corsika-runner" - assert runner_service_config_only.corsika_config.primary_particle.name == "proton" + assert runner_service_config_only.config.primary_particle.name == "proton" assert str(runner_service_config_only.directory).endswith("corsika") @@ -94,16 +94,16 @@ def test_get_file_basename(runner_service, file_base_name, model_version): f"proton_run000001_za20deg_azm000deg_South_test_layout_{model_version}" ) - _runner_service_copy.corsika_config.primary_particle = None + _runner_service_copy.config.primary_particle = None assert _runner_service_copy._get_file_basename(1) == ( f"run000001_za20deg_azm000deg_South_test_layout_{model_version}" ) - _runner_service_copy.corsika_config.primary_particle = { + _runner_service_copy.config.primary_particle = { "primary_id_type": "common_name", "primary": "gamma", } - _runner_service_copy.corsika_config.config["USER_INPUT"]["VIEWCONE"] = [0, 5] + _runner_service_copy.config.config["USER_INPUT"]["VIEWCONE"] = [0, 5] assert _runner_service_copy._get_file_basename(1) == ( f"gamma_diffuse_run000001_za20deg_azm000deg_South_test_layout_{model_version}" ) @@ -200,7 +200,6 @@ def test_get_sub_directory(runner_service_config_only, tmp_path): def test__get_file_basename(runner_service_config_only): - runner_service_config_only.corsika_config = None - runner_service_config_only.core_config = None - with pytest.raises(ValueError, match=r"Either corsika_config or core_config must be provided."): + runner_service_config_only.config = None + with pytest.raises(ValueError, match=r"Invalid configuration type: "): runner_service_config_only._get_file_basename(run_number=1) diff --git a/tests/unit_tests/simtel/test_simulator_light_emission.py b/tests/unit_tests/simtel/test_simulator_light_emission.py index 6580de756b..1604ad22c2 100644 --- a/tests/unit_tests/simtel/test_simulator_light_emission.py +++ b/tests/unit_tests/simtel/test_simulator_light_emission.py @@ -19,6 +19,7 @@ def simulator_instance(): inst.telescope_model = Mock() inst.site_model = Mock() inst.light_emission_config = {} + inst.job_files = Mock() inst.output_directory = "/test/output" inst._logger = Mock() inst.runner_service = Mock() @@ -1089,10 +1090,16 @@ def test_prepare_run(simulator_instance, tmp_test_directory): script_dir.mkdir(parents=True, exist_ok=True) script_path = script_dir / "xyzls-light_emission.sh" - # Mock runner_service.get_file_name to return paths - def get_file_name_side_effect(file_type): + # Mock job_files.get_file_name to return the script path + def job_files_get_file_name_side_effect(file_type): if file_type == "sub_script": return script_path + return Path(tmp_test_directory) / "output" / f"{file_type}.tmp" + + simulator_instance.job_files.get_file_name.side_effect = job_files_get_file_name_side_effect + + # Mock runner_service.get_file_name to return paths + def get_file_name_side_effect(file_type): if file_type == "sim_telarray_output": return Path(tmp_test_directory) / "output" / "test_output.simtel.gz" if file_type == "iact_output": @@ -1160,10 +1167,16 @@ def test_simulate(simulator_instance, tmp_test_directory): mock_script_path.parent.mkdir(parents=True, exist_ok=True) mock_output_file = Path(tmp_test_directory) / "output" / "test_output.simtel.gz" - # Setup mock to return the script and output file - def get_file_name_side_effect(file_type): + # Setup job_files mock to return the script path + def job_files_get_file_name_side_effect(file_type): if file_type == "sub_script": return mock_script_path + return Path(tmp_test_directory) / "output" / f"{file_type}.tmp" + + simulator_instance.job_files.get_file_name.side_effect = job_files_get_file_name_side_effect + + # Setup runner_service mock to return the output file and other paths + def get_file_name_side_effect(file_type): if file_type == "sim_telarray_output": return mock_output_file if file_type == "sub_out": @@ -1181,7 +1194,7 @@ def get_file_name_side_effect(file_type): with patch.object( simulator_instance, "make_run_command", return_value=["#!/bin/bash\n", "echo test\n"] ): - result = simulator_instance.simulate() + simulator_instance.simulate() # Create the output file to simulate successful run (this happens during simulate()) mock_output_file.parent.mkdir(parents=True, exist_ok=True) @@ -1192,9 +1205,6 @@ def get_file_name_side_effect(file_type): call_args = mock_job_submit.call_args assert call_args[0][0] == mock_script_path # First positional arg is the script - # Verify return value - result will be the mock_output_file if it exists - assert result == mock_output_file - def test__initialize_light_emission_configuration(simulator_instance): """Test _initialize_light_emission_configuration method.""" diff --git a/tests/unit_tests/test_simulator.py b/tests/unit_tests/test_simulator.py index 8d85c23816..1e2166b31c 100644 --- a/tests/unit_tests/test_simulator.py +++ b/tests/unit_tests/test_simulator.py @@ -10,6 +10,7 @@ import pytest +from simtools.corsika.corsika_config import CorsikaConfig from simtools.sim_events import file_info from simtools.simulator import Simulator @@ -315,12 +316,14 @@ def test_pack_for_register_with_multiple_versions( mocker.patch("simtools.simulator.ArrayModel", side_effect=mock_array_models) - mock_corsika_config = mocker.MagicMock() + mock_corsika_config = mocker.MagicMock(CorsikaConfig, instance=True) + mock_corsika_config.array_model = mocker.MagicMock() mock_corsika_config.get_config_parameter.side_effect = lambda param: { "VIEWCONE": [0, 10], "THETAP": [20, 20], }.get(param, [0, 0]) mock_corsika_config.azimuth_angle = 0 # from args + mock_corsika_config.zenith_angle = 20 # from args mock_corsika_config.array_model.site = "North" # from args mock_corsika_config.array_model.layout_name = "test_layout" # from args mock_corsika_config.array_model.model_version = model_versions[0] From 20a9fb23271f1ad46256797bf0ac2010d047cb1f Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Tue, 13 Jan 2026 13:12:36 +0100 Subject: [PATCH 06/10] Update src/simtools/runners/runner_services.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/simtools/runners/runner_services.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/simtools/runners/runner_services.py b/src/simtools/runners/runner_services.py index dae089887d..c72305d05c 100644 --- a/src/simtools/runners/runner_services.py +++ b/src/simtools/runners/runner_services.py @@ -187,11 +187,25 @@ def _get_file_basename(self, run_number, is_multi_pipe=False): def _get_file_base_name_from_core_config(self): """Get file base name from core configuration.""" cfg = self.config + zenith_angle = cfg.get("zenith_angle") + azimuth_angle = cfg.get("azimuth_angle") + + za_part = ( + f"za{round(zenith_angle):02}deg_" + if isinstance(zenith_angle, (int, float)) + else "" + ) + az_part = ( + f"azm{azimuth_angle:03}deg_" + if isinstance(azimuth_angle, (int, float)) + else "" + ) + parts = [ f"{cfg.get('run_mode', '')}_", f"{self._get_run_number_string(cfg.get('run_number'))}_" if "run_number" in cfg else "", - f"za{round(cfg['zenith_angle']):02}deg_" if "zenith_angle" in cfg else "", - f"azm{cfg['azimuth_angle']:03}deg_" if "azimuth_angle" in cfg else "", + za_part, + az_part, f"{cfg['site']}_" if "site" in cfg else "", f"{cfg['layout']}_" if "layout" in cfg else "", cfg.get("model_version", ""), From a8207e46284d7219690717e99f27ddf7fae459b6 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Tue, 13 Jan 2026 12:28:08 +0100 Subject: [PATCH 07/10] changelog --- docs/changes/1981.maintenance.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/changes/1981.maintenance.md diff --git a/docs/changes/1981.maintenance.md b/docs/changes/1981.maintenance.md new file mode 100644 index 0000000000..2b5ec983a6 --- /dev/null +++ b/docs/changes/1981.maintenance.md @@ -0,0 +1 @@ +Improve file management for light emission simulations (flasher/illuminators). Change of output file naming and directory tree. From 5d4ab666fd62e04312fd0db35a971c9602258caf Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Tue, 13 Jan 2026 13:18:00 +0100 Subject: [PATCH 08/10] tests --- .../applications/simulate_illuminator.py | 1 + src/simtools/runners/runner_services.py | 10 ++---- .../simtel/test_simulator_light_emission.py | 34 ++++++++++--------- 3 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/simtools/applications/simulate_illuminator.py b/src/simtools/applications/simulate_illuminator.py index 753e702bca..6247230eeb 100644 --- a/src/simtools/applications/simulate_illuminator.py +++ b/src/simtools/applications/simulate_illuminator.py @@ -114,6 +114,7 @@ def main(): label=app_context.args.get("label"), ) light_source.simulate() + light_source.verify_simulations() if __name__ == "__main__": diff --git a/src/simtools/runners/runner_services.py b/src/simtools/runners/runner_services.py index c72305d05c..083b4baaac 100644 --- a/src/simtools/runners/runner_services.py +++ b/src/simtools/runners/runner_services.py @@ -191,15 +191,9 @@ def _get_file_base_name_from_core_config(self): azimuth_angle = cfg.get("azimuth_angle") za_part = ( - f"za{round(zenith_angle):02}deg_" - if isinstance(zenith_angle, (int, float)) - else "" - ) - az_part = ( - f"azm{azimuth_angle:03}deg_" - if isinstance(azimuth_angle, (int, float)) - else "" + f"za{round(zenith_angle):02}deg_" if isinstance(zenith_angle, (int, float)) else "" ) + az_part = f"azm{azimuth_angle:03}deg_" if isinstance(azimuth_angle, (int, float)) else "" parts = [ f"{cfg.get('run_mode', '')}_", diff --git a/tests/unit_tests/simtel/test_simulator_light_emission.py b/tests/unit_tests/simtel/test_simulator_light_emission.py index 1604ad22c2..1574f7af95 100644 --- a/tests/unit_tests/simtel/test_simulator_light_emission.py +++ b/tests/unit_tests/simtel/test_simulator_light_emission.py @@ -1373,7 +1373,7 @@ def test__get_telescope_pointing(simulator_instance): simulator_instance._logger.info.assert_not_called() -def test__write_telescope_position_file(simulator_instance): +def test__write_telescope_position_file(simulator_instance, tmp_test_directory): """Test _write_telescope_position_file method.""" simulator_instance.output_directory = Path("/output") @@ -1393,27 +1393,29 @@ def test__write_telescope_position_file(simulator_instance): mock_radius, # telescope_sphere_radius ] - # Mock io_handler to return a proper Path object - mock_output_dir = Path("/tmp/test_output") + # Use real temporary directory for file writing + mock_output_dir = Path(tmp_test_directory) + mock_output_dir.mkdir(parents=True, exist_ok=True) simulator_instance.io_handler.get_output_directory.return_value = mock_output_dir expected_file = mock_output_dir / "telescope_position.dat" - # Mock file writing - with patch.object(Path, "write_text") as mock_write: - result = simulator_instance._write_telescope_position_file() + # Call the method + result = simulator_instance._write_telescope_position_file() - # Should return the telescope position file path - assert result == expected_file + # Should return the telescope position file path + assert result == expected_file - # Should write coordinates and radius in correct format - expected_content = "100.0 200.0 300.0 1500.0\n" - mock_write.assert_called_once_with(expected_content, encoding="utf-8") + # Verify file was created with correct content + assert result.exists() + content = result.read_text(encoding="utf-8") + expected_content = "100.0 200.0 300.0 1500.0\n" + assert content == expected_content - # Verify unit conversions were called - mock_x.to.assert_called_once_with(u.cm) - mock_y.to.assert_called_once_with(u.cm) - mock_z.to.assert_called_once_with(u.cm) - mock_radius.to.assert_called_once_with(u.cm) + # Verify unit conversions were called + mock_x.to.assert_called_once_with(u.cm) + mock_y.to.assert_called_once_with(u.cm) + mock_z.to.assert_called_once_with(u.cm) + mock_radius.to.assert_called_once_with(u.cm) def test__calibration_pointing_direction(simulator_instance): From b1b4836c6204e365fd34844cd846e03d338e45b4 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Tue, 13 Jan 2026 13:20:47 +0100 Subject: [PATCH 09/10] integration test --- .../simulate_flasher_MSFx-FlashCam-full_simulation.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration_tests/config/simulate_flasher_MSFx-FlashCam-full_simulation.yml b/tests/integration_tests/config/simulate_flasher_MSFx-FlashCam-full_simulation.yml index 3f2f2555d7..065147bfdf 100644 --- a/tests/integration_tests/config/simulate_flasher_MSFx-FlashCam-full_simulation.yml +++ b/tests/integration_tests/config/simulate_flasher_MSFx-FlashCam-full_simulation.yml @@ -12,9 +12,9 @@ applications: log_level: DEBUG integration_tests: - test_output_files: - - file: logfile.log + - file: sub/full_simulation_run000001_South_6.0.2_simulate_flasher.out path_descriptor: output_path - - file: ff-1m.simtel.zst + - file: sim_telarray/full_simulation_run000001_South_6.0.2_simulate_flasher.simtel.zst path_descriptor: output_path - expected_log_output: pattern: @@ -24,7 +24,7 @@ applications: forbidden_pattern: - "Error" - "had less photons than required and were skipped" - file: logfile.log + file: sub/full_simulation_run000001_South_6.0.2_simulate_flasher.out path_descriptor: output_path model_version_use_current: true test_name: full-simulation-msfx-flashcam-6.0.2 From 54a3b6ff5e5fe5733d42d00cfc4c0714406af981 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Tue, 13 Jan 2026 13:45:05 +0100 Subject: [PATCH 10/10] coverage --- src/simtools/runners/runner_services.py | 15 +-- .../runners/test_runner_services.py | 93 +++++++++++++++++++ .../simtel/test_simulator_light_emission.py | 29 ++++++ 3 files changed, 127 insertions(+), 10 deletions(-) diff --git a/src/simtools/runners/runner_services.py b/src/simtools/runners/runner_services.py index 083b4baaac..92875246b8 100644 --- a/src/simtools/runners/runner_services.py +++ b/src/simtools/runners/runner_services.py @@ -190,18 +190,13 @@ def _get_file_base_name_from_core_config(self): zenith_angle = cfg.get("zenith_angle") azimuth_angle = cfg.get("azimuth_angle") - za_part = ( - f"za{round(zenith_angle):02}deg_" if isinstance(zenith_angle, (int, float)) else "" - ) - az_part = f"azm{azimuth_angle:03}deg_" if isinstance(azimuth_angle, (int, float)) else "" - parts = [ - f"{cfg.get('run_mode', '')}_", + f"{cfg['run_mode']}_" if cfg.get("run_mode") else "", f"{self._get_run_number_string(cfg.get('run_number'))}_" if "run_number" in cfg else "", - za_part, - az_part, - f"{cfg['site']}_" if "site" in cfg else "", - f"{cfg['layout']}_" if "layout" in cfg else "", + f"za{round(zenith_angle):02}deg_" if isinstance(zenith_angle, (int, float)) else "", + f"azm{round(azimuth_angle):03}deg_" if isinstance(azimuth_angle, (int, float)) else "", + f"{cfg['site']}_" if cfg.get("site") else "", + f"{cfg['layout']}_" if cfg.get("layout") else "", cfg.get("model_version", ""), f"_{self.label}" if self.label else "", ] diff --git a/tests/unit_tests/runners/test_runner_services.py b/tests/unit_tests/runners/test_runner_services.py index 2b9b0782ab..ed369b9225 100644 --- a/tests/unit_tests/runners/test_runner_services.py +++ b/tests/unit_tests/runners/test_runner_services.py @@ -145,6 +145,8 @@ def test_get_run_number_string(runner_service_config_only): ): runner_service_config_only._get_run_number_string(1234567) + assert runner_service_config_only._get_run_number_string(None) == "" + def test_get_resources(runner_service_mock_array_model, caplog): sub_log_file = runner_service_mock_array_model.get_file_name(file_type="sub_log", run_number=5) @@ -167,6 +169,10 @@ def test_get_resources(runner_service_mock_array_model, caplog): assert resources["runtime"] is None assert "RUNTIME was not found in run log file" in caplog.text + runner_service_mock_array_model.config = None + resources = runner_service_mock_array_model.get_resources(sub_log_file) + assert resources["runtime"] is None + def test_validate_corsika_run_number(): assert runner_services.validate_corsika_run_number(1) @@ -203,3 +209,90 @@ def test__get_file_basename(runner_service_config_only): runner_service_config_only.config = None with pytest.raises(ValueError, match=r"Invalid configuration type: "): runner_service_config_only._get_file_basename(run_number=1) + + +def test_get_file_base_name_from_core_config_full(): + """Test file base name generation from core config with all parameters.""" + config = { + "run_mode": "test_mode", + "run_number": 5, + "zenith_angle": 20.3, + "azimuth_angle": 45.7, + "site": "South", + "layout": "test_layout", + "model_version": "1.0.0", + } + runner_service = runner_services.RunnerServices( + config=config, + label="test-label", + run_type="corsika", + ) + basename = runner_service._get_file_base_name_from_core_config() + assert basename == "test_mode_run000005_za20deg_azm046deg_South_test_layout_1.0.0_test-label" + + +def test_get_file_base_name_from_core_config_minimal(): + """Test file base name generation from core config with minimal parameters.""" + config = { + "run_mode": "test_mode", + "model_version": "1.0.0", + } + runner_service = runner_services.RunnerServices( + config=config, + label=None, + run_type="corsika", + ) + basename = runner_service._get_file_base_name_from_core_config() + assert basename == "test_mode_1.0.0" + + +def test_get_file_base_name_from_core_config_no_run_mode(): + """Test file base name generation without run_mode.""" + config = { + "zenith_angle": 20, + "azimuth_angle": 45, + "site": "North", + "layout": "test_layout", + "model_version": "1.0.0", + } + runner_service = runner_services.RunnerServices( + config=config, + label="test-label", + run_type="corsika", + ) + basename = runner_service._get_file_base_name_from_core_config() + assert basename == "za20deg_azm045deg_North_test_layout_1.0.0_test-label" + + +def test_get_file_base_name_from_core_config_invalid_angles(): + """Test file base name generation with invalid angle types.""" + config = { + "run_mode": "test_mode", + "zenith_angle": "invalid", + "azimuth_angle": "invalid", + "model_version": "1.0.0", + } + runner_service = runner_services.RunnerServices( + config=config, + label=None, + run_type="corsika", + ) + basename = runner_service._get_file_base_name_from_core_config() + assert basename == "test_mode_1.0.0" + + +def test_get_file_base_name_from_core_config_none_values(): + """Test file base name generation with None values in optional fields.""" + config = { + "run_mode": "test_mode", + "site": None, + "layout": None, + "model_version": "1.0.0", + } + runner_service = runner_services.RunnerServices( + config=config, + label=None, + run_type="corsika", + ) + basename = runner_service._get_file_base_name_from_core_config() + assert basename == "test_mode_1.0.0" diff --git a/tests/unit_tests/simtel/test_simulator_light_emission.py b/tests/unit_tests/simtel/test_simulator_light_emission.py index 1574f7af95..8dccef1b5e 100644 --- a/tests/unit_tests/simtel/test_simulator_light_emission.py +++ b/tests/unit_tests/simtel/test_simulator_light_emission.py @@ -1518,3 +1518,32 @@ def test__get_angular_distribution_string_for_sim_telarray_isotropic(simulator_i # Verify width was NOT requested: the implementation returns early for isotropic distributions # before attempting to fetch the width via get_parameter_value_with_unit. simulator_instance.calibration_model.get_parameter_value_with_unit.assert_not_called() + + +def test_verify_simulations_success(simulator_instance, tmp_test_directory): + """Test verify_simulations returns True when output file exists.""" + output_file = Path(tmp_test_directory) / "output.iact" + output_file.write_text("test data", encoding="utf-8") + + simulator_instance.runner_service.get_file_name.return_value = output_file + + result = simulator_instance.verify_simulations() + + assert result is True + simulator_instance._logger.info.assert_called_once() + assert "sim_telarray output found" in str(simulator_instance._logger.info.call_args) + + +def test_verify_simulations_missing_output(simulator_instance, tmp_test_directory): + """Test verify_simulations returns False when output file does not exist.""" + output_file = Path(tmp_test_directory) / "nonexistent_output.iact" + + simulator_instance.runner_service.get_file_name.return_value = output_file + + result = simulator_instance.verify_simulations() + + assert result is False + simulator_instance._logger.error.assert_called_once() + assert "Expected sim_telarray output not found" in str( + simulator_instance._logger.error.call_args + )