From 42c52c9d3d4d8f524b0e8cc928b1090277a20c18 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Mon, 17 Feb 2025 10:55:09 +0100 Subject: [PATCH 01/14] File names with semantic versioning --- .../submit_model_parameter_from_external.py | 4 ++-- src/simtools/data_model/model_data_writer.py | 4 +++- src/simtools/utils/names.py | 24 +++++++++++++++++++ tests/unit_tests/utils/test_names.py | 10 ++++++++ 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/simtools/applications/submit_model_parameter_from_external.py b/src/simtools/applications/submit_model_parameter_from_external.py index 0c89c8e636..9131e4e093 100644 --- a/src/simtools/applications/submit_model_parameter_from_external.py +++ b/src/simtools/applications/submit_model_parameter_from_external.py @@ -104,7 +104,7 @@ def main(): # noqa: D103 logger.setLevel(gen.get_log_level_from_user(args_dict["log_level"])) output_path = ( - Path(args_dict["output_path"]) / args_dict["parameter_version"] / args_dict["instrument"] + Path(args_dict["output_path"]) / args_dict["instrument"] / args_dict["parameter"] if args_dict.get("output_path") else None ) @@ -113,7 +113,7 @@ def main(): # noqa: D103 value=args_dict["value"], instrument=args_dict["instrument"], parameter_version=args_dict["parameter_version"], - output_file=Path(args_dict["parameter"]).with_suffix(".json"), + output_file=Path(args_dict["parameter"] + "-" + args_dict["parameter_version"] + ".json"), output_path=output_path, use_plain_output_path=args_dict.get("use_plain_output_path"), metadata_input_dict=args_dict, diff --git a/src/simtools/data_model/model_data_writer.py b/src/simtools/data_model/model_data_writer.py index 1651275b00..6186c1cd35 100644 --- a/src/simtools/data_model/model_data_writer.py +++ b/src/simtools/data_model/model_data_writer.py @@ -425,7 +425,9 @@ def write_metadata_to_yml(self, metadata, yml_file=None, keys_lower_case=False): If yml_file is not defined. """ try: - yml_file = Path(yml_file or self.product_data_file).with_suffix(".metadata.yml") + yml_file = names.file_name_with_version( + yml_file or self.product_data_file, ".metadata.yml" + ) with open(yml_file, "w", encoding="UTF-8") as file: yaml.safe_dump( gen.change_dict_keys_case(metadata, keys_lower_case), diff --git a/src/simtools/utils/names.py b/src/simtools/utils/names.py index e0777c66ac..31625262aa 100644 --- a/src/simtools/utils/names.py +++ b/src/simtools/utils/names.py @@ -592,3 +592,27 @@ def sanitize_name(name): _logger.error(msg) raise ValueError(msg) return sanitized + + +def file_name_with_version(file_name, suffix): + """ + Return a file name including a semantic version with the correct suffix. + + Avoids having 'Path.suffix()' to remove trailing numbers. + + Parameters + ---------- + file_name: str + File name. + suffix: str + File suffix. + + Returns + ------- + Path + File name with version number. + """ + file_name = str(file_name) + if bool(re.search(r"\d+\.\d+\.\d+$", file_name)): + return Path(file_name + suffix) + return Path(file_name).with_suffix(suffix) diff --git a/tests/unit_tests/utils/test_names.py b/tests/unit_tests/utils/test_names.py index dcf42c8a10..aeb2a1c256 100644 --- a/tests/unit_tests/utils/test_names.py +++ b/tests/unit_tests/utils/test_names.py @@ -1,6 +1,7 @@ #!/usr/bin/python3 import logging +from pathlib import Path import pytest @@ -504,3 +505,12 @@ def test_get_simulation_software_name_from_parameter_name(): def test_get_parameter_name_from_simtel_name(): assert names.get_parameter_name_from_simtel_name("focal_length") == "focal_length" assert names.get_parameter_name_from_simtel_name("altitude") == "corsika_observation_level" + + +def test_file_name_with_version(): + + assert names.file_name_with_version("file", ".yml") == Path("file.yml") + assert names.file_name_with_version("file.json", ".yml") == Path("file.yml") + + assert names.file_name_with_version("file-5.22.0", ".yml") == Path("file-5.22.0.yml") + assert names.file_name_with_version("file-5.0.0.json", ".yml") == Path("file-5.0.0.yml") From 199a71e5b357cf4da3fce1a808c90083c2dabefc Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Mon, 17 Feb 2025 13:08:02 +0100 Subject: [PATCH 02/14] add unique ID --- src/simtools/data_model/model_data_writer.py | 25 +++++++++++++------- src/simtools/utils/names.py | 2 ++ tests/unit_tests/utils/test_names.py | 4 ++++ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/simtools/data_model/model_data_writer.py b/src/simtools/data_model/model_data_writer.py index 6186c1cd35..a67b0665cc 100644 --- a/src/simtools/data_model/model_data_writer.py +++ b/src/simtools/data_model/model_data_writer.py @@ -161,21 +161,30 @@ def dump_model_parameter( output_path=output_path, use_plain_output_path=use_plain_output_path, ) - _json_dict = writer.get_validated_parameter_dict( - parameter_name, value, instrument, parameter_version - ) - writer.write_dict_to_model_parameter_json(output_file, _json_dict) + unique_id = None if metadata_input_dict is not None: metadata_input_dict["output_file"] = output_file metadata_input_dict["output_file_format"] = Path(output_file).suffix.lstrip(".") + metadata = MetadataCollector(args_dict=metadata_input_dict).get_top_level_metadata() writer.write_metadata_to_yml( - metadata=MetadataCollector(args_dict=metadata_input_dict).get_top_level_metadata(), - yml_file=output_path / f"{Path(output_file).stem}", + metadata=metadata, yml_file=output_path / f"{Path(output_file).stem}" ) + unique_id = metadata.get("cta", {}).get("product", {}).get("id") + + _json_dict = writer.get_validated_parameter_dict( + parameter_name, value, instrument, parameter_version, unique_id + ) + writer.write_dict_to_model_parameter_json(output_file, _json_dict) return _json_dict def get_validated_parameter_dict( - self, parameter_name, value, instrument, parameter_version, schema_version=None + self, + parameter_name, + value, + instrument, + parameter_version, + unique_id=None, + schema_version=None, ): """ Get validated parameter dictionary. @@ -215,7 +224,7 @@ def get_validated_parameter_dict( "instrument": instrument, "site": site, "parameter_version": parameter_version, - "unique_id": None, + "unique_id": unique_id, "value": value, "unit": unit, "type": self._get_parameter_type(), diff --git a/src/simtools/utils/names.py b/src/simtools/utils/names.py index 31625262aa..aa199d5084 100644 --- a/src/simtools/utils/names.py +++ b/src/simtools/utils/names.py @@ -612,6 +612,8 @@ def file_name_with_version(file_name, suffix): Path File name with version number. """ + if file_name is None or suffix is None: + return None file_name = str(file_name) if bool(re.search(r"\d+\.\d+\.\d+$", file_name)): return Path(file_name + suffix) diff --git a/tests/unit_tests/utils/test_names.py b/tests/unit_tests/utils/test_names.py index aeb2a1c256..272079108b 100644 --- a/tests/unit_tests/utils/test_names.py +++ b/tests/unit_tests/utils/test_names.py @@ -509,6 +509,10 @@ def test_get_parameter_name_from_simtel_name(): def test_file_name_with_version(): + assert names.file_name_with_version(None, None) is None + assert names.file_name_with_version("file", None) is None + assert names.file_name_with_version(None, ".yml") is None + assert names.file_name_with_version("file", ".yml") == Path("file.yml") assert names.file_name_with_version("file.json", ".yml") == Path("file.yml") From aaad3b9c488a44e285f7d973103162e2fad1d900 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Mon, 17 Feb 2025 15:06:40 +0100 Subject: [PATCH 03/14] Add applications to run simtools --- src/simtools/applications/run_application.py | 60 +++++++++++++++++ src/simtools/dependencies.py | 71 ++++++++++++++++++++ tests/unit_tests/test_dependencies.py | 71 ++++++++++++++++++++ 3 files changed, 202 insertions(+) create mode 100644 src/simtools/applications/run_application.py create mode 100644 src/simtools/dependencies.py create mode 100644 tests/unit_tests/test_dependencies.py diff --git a/src/simtools/applications/run_application.py b/src/simtools/applications/run_application.py new file mode 100644 index 0000000000..4eeafbf01a --- /dev/null +++ b/src/simtools/applications/run_application.py @@ -0,0 +1,60 @@ +#!/usr/bin/python3 + +""" +Run simtools applications from configuration files. + +Allows to run several simtools applications with a single configuration file, which includes +both the name of the simtools application and the configuration for the application. + +""" + +import logging +from pathlib import Path + +import simtools.utils.general as gen +from simtools.configuration import configurator + + +def _parse(label, description, usage): + """ + Parse command line configuration. + + Parameters + ---------- + label : str + Label describing the application. + description : str + Description of the application. + usage : str + Example on how to use the application. + + Returns + ------- + CommandLineParser + Command line parser object. + """ + config = configurator.Configurator(label=label, description=description, usage=usage) + + config.parser.add_argument( + "--configuration_file", + help="Application configuration.", + type=str, + required=True, + default=None, + ) + return config.initialize(db_config=False) + + +def main(): # noqa: D103 + + args_dict, _ = _parse( + Path(__file__).stem, + description="Run simtools applications from configuration file.", + usage="simtools-run-application --config_file config_file_name", + ) + logger = logging.getLogger() + logger.setLevel(gen.get_log_level_from_user(args_dict["log_level"])) + + +if __name__ == "__main__": + main() diff --git a/src/simtools/dependencies.py b/src/simtools/dependencies.py new file mode 100644 index 0000000000..d6e0dd9cb2 --- /dev/null +++ b/src/simtools/dependencies.py @@ -0,0 +1,71 @@ +"""Simtools dependencies version management.""" + +import logging +import os +import re +from pathlib import Path + +from simtools.db.db_handler import DatabaseHandler + +_logger = logging.getLogger(__name__) + + +def get_database_version(db_config): + """ + Get the version of the simulation model data base used. + + Parameters + ---------- + db_config : dict + Dictionary containing the database configuration. + + Returns + ------- + str + Version of the simulation model data base used. + + """ + if db_config is None: + return None + db = DatabaseHandler(db_config) + return db.mongo_db_config.get("db_simulation_model") + + +def get_sim_telarray_version(): + """ + Get the version of the sim_telarray package. + + Returns + ------- + str + Version of the sim_telarray package. + """ + sim_telarray_path = os.getenv("SIMTOOLS_SIMTEL_PATH") + if sim_telarray_path is None: + _logger.warning("Environment variable SIMTOOLS_SIMTEL_PATH is not set.") + return None + version_file = Path(sim_telarray_path) / "sim_telarray" / "version.h" + try: + with open(version_file, encoding="utf-8") as file: + content = file.read() + except FileNotFoundError as exc: + raise FileNotFoundError("sim_telarray version file not found.") from exc + + match = re.search(r'#define BASE_RELEASE\s+"([^"]+)"', content) + + if match: + return match.group(1) + raise ValueError("sim_telarray BASE_RELEASE not found in the file.") + + +def get_corsika_version(): + """ + Get the version of the corsika package. + + Returns + ------- + str + Version of the corsika package. + """ + _logger.warning("CORSIKA version not implemented yet.") + return "7.7" diff --git a/tests/unit_tests/test_dependencies.py b/tests/unit_tests/test_dependencies.py new file mode 100644 index 0000000000..7b3391014a --- /dev/null +++ b/tests/unit_tests/test_dependencies.py @@ -0,0 +1,71 @@ +#!/usr/bin/python3 + +import logging +import os +from unittest import mock + +import pytest + +from simtools.db.db_handler import DatabaseHandler +from simtools.dependencies import ( + get_corsika_version, + get_database_version, + get_sim_telarray_version, +) + + +def test_get_sim_telarray_version_success(): + with mock.patch.dict(os.environ, {"SIMTOOLS_SIMTEL_PATH": "/fake/path"}): + version_content = '#define BASE_RELEASE "1.2.3"' + + with mock.patch("builtins.open", mock.mock_open(read_data=version_content)): + assert get_sim_telarray_version() == "1.2.3" + + +def test_get_sim_telarray_version_env_not_set(): + with mock.patch.dict(os.environ, {}, clear=True): + assert get_sim_telarray_version() is None + + +def test_get_sim_telarray_version_file_not_found(): + with mock.patch.dict(os.environ, {"SIMTOOLS_SIMTEL_PATH": "/fake/path"}): + with mock.patch("pathlib.Path.open", side_effect=FileNotFoundError): + with pytest.raises(FileNotFoundError, match="sim_telarray version file not found."): + get_sim_telarray_version() + + +def test_get_sim_telarray_version_base_release_not_found(): + with mock.patch.dict(os.environ, {"SIMTOOLS_SIMTEL_PATH": "/fake/path"}): + version_content = '#define SOME_OTHER_DEFINE "1.2.3"' + + with mock.patch("builtins.open", mock.mock_open(read_data=version_content)): + with pytest.raises( + ValueError, match="sim_telarray BASE_RELEASE not found in the file." + ): + get_sim_telarray_version() + + +def test_get_database_version_success(): + db_config = {"host": "localhost", "port": 27017} + mock_db_handler = mock.MagicMock(spec=DatabaseHandler) + mock_db_handler.mongo_db_config = {"db_simulation_model": "v1.0.0"} + + with mock.patch("simtools.dependencies.DatabaseHandler", return_value=mock_db_handler): + assert get_database_version(db_config) == "v1.0.0" + + +def test_get_database_version_no_version(): + db_config = {"host": "localhost", "port": 27017} + mock_db_handler = mock.MagicMock(spec=DatabaseHandler) + mock_db_handler.mongo_db_config = {} + + with mock.patch("simtools.dependencies.DatabaseHandler", return_value=mock_db_handler): + assert get_database_version(db_config) is None + + +def test_get_corsika_version(caplog): + + with caplog.at_level(logging.WARNING): + assert get_corsika_version() == "7.7" + + assert "CORSIKA version not implemented yet." in caplog.text From d4cb6bb5b59d14f47724ae816855b291d4def23a Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Tue, 18 Feb 2025 12:26:41 +0100 Subject: [PATCH 04/14] get simtools from version --- src/simtools/applications/run_application.py | 34 +++++++++++ src/simtools/dependencies.py | 23 +++---- tests/unit_tests/test_dependencies.py | 63 ++++++++++---------- 3 files changed, 79 insertions(+), 41 deletions(-) diff --git a/src/simtools/applications/run_application.py b/src/simtools/applications/run_application.py index 4eeafbf01a..c9d600e014 100644 --- a/src/simtools/applications/run_application.py +++ b/src/simtools/applications/run_application.py @@ -9,8 +9,12 @@ """ import logging +import subprocess +import tempfile from pathlib import Path +import yaml + import simtools.utils.general as gen from simtools.configuration import configurator @@ -45,6 +49,21 @@ def _parse(label, description, usage): return config.initialize(db_config=False) +def run_application(application, configuration): + """Run a simtools application and return stdout and stderr.""" + with tempfile.NamedTemporaryFile(mode="w", delete=True, suffix=".yml") as temp_config: + yaml.dump(configuration, temp_config, default_flow_style=False) + temp_config.flush() + configuration_file = Path(temp_config.name) + result = subprocess.run( + [application, "--config", configuration_file], + check=False, + capture_output=True, + text=True, + ) + return result.stdout, result.stderr + + def main(): # noqa: D103 args_dict, _ = _parse( @@ -55,6 +74,21 @@ def main(): # noqa: D103 logger = logging.getLogger() logger.setLevel(gen.get_log_level_from_user(args_dict["log_level"])) + # TODO still not working with the configuration files with and without lists + configurations = gen.collect_data_from_file(args_dict["configuration_file"]).get("CTA_SIMPIPE") + log_file = Path(configurations.get("LOG_PATH", "./")) / "simtools.log" + log_file.parent.mkdir(parents=True, exist_ok=True) + if isinstance(configurations, dict): + configurations = [configurations] + with log_file.open("w", encoding="utf-8") as file: + for config in configurations: + config = gen.change_dict_keys_case(config, False) + stdout, stderr = run_application(config.get("APPLICATION"), config.get("CONFIGURATION")) + file.write("=" * 80 + "\n") + file.write(f"Application: {config.get('APPLICATION')}\n") + file.write("STDOUT:\n" + stdout) + file.write("STDERR:\n" + stderr) + if __name__ == "__main__": main() diff --git a/src/simtools/dependencies.py b/src/simtools/dependencies.py index d6e0dd9cb2..016cd66687 100644 --- a/src/simtools/dependencies.py +++ b/src/simtools/dependencies.py @@ -3,6 +3,7 @@ import logging import os import re +import subprocess from pathlib import Path from simtools.db.db_handler import DatabaseHandler @@ -33,7 +34,7 @@ def get_database_version(db_config): def get_sim_telarray_version(): """ - Get the version of the sim_telarray package. + Get the version of the sim_telarray package using 'sim_telarray --version'. Returns ------- @@ -44,18 +45,20 @@ def get_sim_telarray_version(): if sim_telarray_path is None: _logger.warning("Environment variable SIMTOOLS_SIMTEL_PATH is not set.") return None - version_file = Path(sim_telarray_path) / "sim_telarray" / "version.h" - try: - with open(version_file, encoding="utf-8") as file: - content = file.read() - except FileNotFoundError as exc: - raise FileNotFoundError("sim_telarray version file not found.") from exc + sim_telarray_path = Path(sim_telarray_path) / "bin" / "sim_telarray" - match = re.search(r'#define BASE_RELEASE\s+"([^"]+)"', content) + # expect stdout with e.g. a line 'Release: 2024.271.0 from 2024-09-27' + result = subprocess.run( + [sim_telarray_path, "--version"], + capture_output=True, + text=True, + check=False, + ) + match = re.search(r"^Release:\s+(.+)", result.stdout, re.MULTILINE) if match: - return match.group(1) - raise ValueError("sim_telarray BASE_RELEASE not found in the file.") + return match.group(1).split()[0] + raise ValueError(f"sim_telarray release not found in {result.stdout}") def get_corsika_version(): diff --git a/tests/unit_tests/test_dependencies.py b/tests/unit_tests/test_dependencies.py index 7b3391014a..acde50a51b 100644 --- a/tests/unit_tests/test_dependencies.py +++ b/tests/unit_tests/test_dependencies.py @@ -14,37 +14,6 @@ ) -def test_get_sim_telarray_version_success(): - with mock.patch.dict(os.environ, {"SIMTOOLS_SIMTEL_PATH": "/fake/path"}): - version_content = '#define BASE_RELEASE "1.2.3"' - - with mock.patch("builtins.open", mock.mock_open(read_data=version_content)): - assert get_sim_telarray_version() == "1.2.3" - - -def test_get_sim_telarray_version_env_not_set(): - with mock.patch.dict(os.environ, {}, clear=True): - assert get_sim_telarray_version() is None - - -def test_get_sim_telarray_version_file_not_found(): - with mock.patch.dict(os.environ, {"SIMTOOLS_SIMTEL_PATH": "/fake/path"}): - with mock.patch("pathlib.Path.open", side_effect=FileNotFoundError): - with pytest.raises(FileNotFoundError, match="sim_telarray version file not found."): - get_sim_telarray_version() - - -def test_get_sim_telarray_version_base_release_not_found(): - with mock.patch.dict(os.environ, {"SIMTOOLS_SIMTEL_PATH": "/fake/path"}): - version_content = '#define SOME_OTHER_DEFINE "1.2.3"' - - with mock.patch("builtins.open", mock.mock_open(read_data=version_content)): - with pytest.raises( - ValueError, match="sim_telarray BASE_RELEASE not found in the file." - ): - get_sim_telarray_version() - - def test_get_database_version_success(): db_config = {"host": "localhost", "port": 27017} mock_db_handler = mock.MagicMock(spec=DatabaseHandler) @@ -69,3 +38,35 @@ def test_get_corsika_version(caplog): assert get_corsika_version() == "7.7" assert "CORSIKA version not implemented yet." in caplog.text + + +def test_get_sim_telarray_version_success(): + os.environ["SIMTOOLS_SIMTEL_PATH"] = "/fake/path" + expected_version = "2024.271.0" + mock_result = mock.Mock() + mock_result.stdout = "Release: 2024.271.0 from 2024-09-27" + mock_result.stderr = "" + + with mock.patch("subprocess.run", return_value=mock_result): + assert get_sim_telarray_version() == expected_version + + +def test_get_sim_telarray_version_no_env_var(caplog): + if "SIMTOOLS_SIMTEL_PATH" in os.environ: + del os.environ["SIMTOOLS_SIMTEL_PATH"] + + with caplog.at_level(logging.WARNING): + assert get_sim_telarray_version() is None + + assert "Environment variable SIMTOOLS_SIMTEL_PATH is not set." in caplog.text + + +def test_get_sim_telarray_version_no_release(): + os.environ["SIMTOOLS_SIMTEL_PATH"] = "/fake/path" + mock_result = mock.Mock() + mock_result.stdout = "Some other output" + mock_result.stderr = "" + + with mock.patch("subprocess.run", return_value=mock_result): + with pytest.raises(ValueError, match="sim_telarray release not found in Some other output"): + get_sim_telarray_version() From 706db32b15683339377d9def0efe164959378fa3 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Tue, 18 Feb 2025 15:58:14 +0100 Subject: [PATCH 05/14] check if parameter exists in DB --- pyproject.toml | 1 + src/simtools/applications/run_application.py | 12 +++-- .../submit_model_parameter_from_external.py | 14 +++-- src/simtools/data_model/model_data_writer.py | 51 ++++++++++++++++--- src/simtools/dependencies.py | 9 ++++ src/simtools/utils/names.py | 5 +- tests/unit_tests/utils/test_names.py | 1 + 7 files changed, 77 insertions(+), 16 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 1247bdcd21..da5fc92755 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -93,6 +93,7 @@ scripts.simtools-plot-array-layout = "simtools.applications.plot_array_layout:ma scripts.simtools-plot-tabular-data = "simtools.applications.plot_tabular_data:main" scripts.simtools-production-generate-simulation-config = "simtools.applications.production_generate_simulation_config:main" scripts.simtools-production-scale-events = "simtools.applications.production_scale_events:main" +scripts.simtools-run-application = "simtools.applications.run_application:main" scripts.simtools-simulate-light-emission = "simtools.applications.simulate_light_emission:main" scripts.simtools-simulate-prod = "simtools.applications.simulate_prod:main" scripts.simtools-simulate-prod-htcondor-generator = "simtools.applications.simulate_prod_htcondor_generator:main" diff --git a/src/simtools/applications/run_application.py b/src/simtools/applications/run_application.py index c9d600e014..694e954b8c 100644 --- a/src/simtools/applications/run_application.py +++ b/src/simtools/applications/run_application.py @@ -15,6 +15,7 @@ import yaml +from simtools import dependencies import simtools.utils.general as gen from simtools.configuration import configurator @@ -74,14 +75,15 @@ def main(): # noqa: D103 logger = logging.getLogger() logger.setLevel(gen.get_log_level_from_user(args_dict["log_level"])) - # TODO still not working with the configuration files with and without lists - configurations = gen.collect_data_from_file(args_dict["configuration_file"]).get("CTA_SIMPIPE") - log_file = Path(configurations.get("LOG_PATH", "./")) / "simtools.log" + application_config = gen.collect_data_from_file(args_dict["configuration_file"]).get("CTA_SIMPIPE") + log_file = Path(application_config.get("LOG_PATH", "./")) / "simtools.log" log_file.parent.mkdir(parents=True, exist_ok=True) - if isinstance(configurations, dict): - configurations = [configurations] + configurations = application_config.get("APPLICATIONS") with log_file.open("w", encoding="utf-8") as file: + file.write("Running simtools applications\n") + file.write(dependencies.get_version_string()) for config in configurations: + logger.info(f"Running application: {config.get('APPLICATION')}") config = gen.change_dict_keys_case(config, False) stdout, stderr = run_application(config.get("APPLICATION"), config.get("CONFIGURATION")) file.write("=" * 80 + "\n") diff --git a/src/simtools/applications/submit_model_parameter_from_external.py b/src/simtools/applications/submit_model_parameter_from_external.py index 9131e4e093..bc888f829e 100644 --- a/src/simtools/applications/submit_model_parameter_from_external.py +++ b/src/simtools/applications/submit_model_parameter_from_external.py @@ -44,6 +44,7 @@ import simtools.data_model.model_data_writer as writer import simtools.utils.general as gen from simtools.configuration import configurator +from simtools.db import db_handler def _parse(label, description): @@ -73,7 +74,6 @@ def _parse(label, description): config.parser.add_argument( "--parameter_version", type=str, required=True, help="Parameter version" ) - config.parser.add_argument( "--value", type=str, @@ -84,18 +84,22 @@ def _parse(label, description): 'Examples: "--value=5", "--value=\'5 km\'", "--value=\'5 cm, 0.5 deg\'"' ), ) - config.parser.add_argument( "--input_meta", help="meta data file associated to input data", type=str, required=False, ) - return config.initialize(output=True) + config.parser.add_argument( + "--check_parameter_version", + help="Check if the parameter version exists in the database", + action="store_true", + ) + return config.initialize(output=True, db_config=True) def main(): # noqa: D103 - args_dict, _ = _parse( + args_dict, db_config = _parse( label=Path(__file__).stem, description="Submit and validate a model parameters).", ) @@ -108,6 +112,7 @@ def main(): # noqa: D103 if args_dict.get("output_path") else None ) + writer.ModelDataWriter.dump_model_parameter( parameter_name=args_dict["parameter"], value=args_dict["value"], @@ -117,6 +122,7 @@ def main(): # noqa: D103 output_path=output_path, use_plain_output_path=args_dict.get("use_plain_output_path"), metadata_input_dict=args_dict, + db_config=db_config if args_dict.get("check_parameter_version") else None, ) diff --git a/src/simtools/data_model/model_data_writer.py b/src/simtools/data_model/model_data_writer.py index a67b0665cc..9b30d9544e 100644 --- a/src/simtools/data_model/model_data_writer.py +++ b/src/simtools/data_model/model_data_writer.py @@ -12,6 +12,7 @@ import simtools.utils.general as gen from simtools.data_model import schema, validate_data from simtools.data_model.metadata_collector import MetadataCollector +from simtools.db import db_handler from simtools.io_operations import io_handler from simtools.utils import names, value_conversion @@ -126,6 +127,7 @@ def dump_model_parameter( output_path=None, use_plain_output_path=False, metadata_input_dict=None, + db_config=None, ): """ Generate DB-style model parameter dict and write it to json file. @@ -148,6 +150,8 @@ def dump_model_parameter( Use plain output path. metadata_input_dict: dict Input to metadata collector. + db_config: dict + Database configuration. If not None, check if parameter with the same version exists. Returns ------- @@ -161,6 +165,11 @@ def dump_model_parameter( output_path=output_path, use_plain_output_path=use_plain_output_path, ) + if db_config is not None: + writer.check_db_for_existing_parameter( + parameter_name, instrument, parameter_version, db_config + ) + unique_id = None if metadata_input_dict is not None: metadata_input_dict["output_file"] = output_file @@ -177,6 +186,41 @@ def dump_model_parameter( writer.write_dict_to_model_parameter_json(output_file, _json_dict) return _json_dict + def check_db_for_existing_parameter(self, parameter_name, instrument, parameter_version, db_config): + """ + Check if a parameter with the same version exists in the simulation model database. + + Parameters + ---------- + parameter_name: str + Name of the parameter. + instrument: str + Name of the instrument. + parameter_version: str + Version of the parameter. + db_config: dict + Database configuration. + + Raises + ------ + ValueError + If parameter with the same version exists in the database. + """ + db = db_handler.DatabaseHandler(mongo_db_config=db_config) + try: + db.get_model_parameter( + parameter=parameter_name, + parameter_version=parameter_version, + site=names.get_site_from_array_element_name(instrument), + array_element_name=instrument, + collection="telescopes", # TODO - generalize collection + ) + except ValueError: + pass # parameter does not exist - expected behavior + else: + raise ValueError( + f"Parameter {parameter_name} with version {parameter_version} already exists.") + def get_validated_parameter_dict( self, parameter_name, @@ -211,18 +255,13 @@ def get_validated_parameter_dict( schema_file = schema.get_model_parameter_schema_file(parameter_name) self.schema_dict = gen.collect_data_from_file(schema_file) - try: # e.g. instrument is 'North" - site = names.validate_site_name(instrument) - except ValueError: # e.g. instrument is 'LSTN-01' - site = names.get_site_from_array_element_name(instrument) - value, unit = value_conversion.split_value_and_unit(value) data_dict = { "schema_version": schema.get_model_parameter_schema_version(schema_version), "parameter": parameter_name, "instrument": instrument, - "site": site, + "site": names.get_site_from_array_element_name(instrument), "parameter_version": parameter_version, "unique_id": unique_id, "value": value, diff --git a/src/simtools/dependencies.py b/src/simtools/dependencies.py index 016cd66687..830568eb3d 100644 --- a/src/simtools/dependencies.py +++ b/src/simtools/dependencies.py @@ -11,6 +11,15 @@ _logger = logging.getLogger(__name__) +def get_version_string(db_config=None): + """Print the versions of the dependencies.""" + return ( + f"Database version: {get_database_version(db_config)}\n" + f"sim_telarray version: {get_sim_telarray_version()}\n" + f"CORSIKA version: {get_corsika_version()}\n" + ) + + def get_database_version(db_config): """ Get the version of the simulation model data base used. diff --git a/src/simtools/utils/names.py b/src/simtools/utils/names.py index aa199d5084..627ff0d3e2 100644 --- a/src/simtools/utils/names.py +++ b/src/simtools/utils/names.py @@ -291,7 +291,10 @@ def get_site_from_array_element_name(name): str Site name (South or North). """ - return array_elements()[get_array_element_type_from_name(name)]["site"] + try: # e.g. instrument is 'North' as given for the site parameters + return validate_site_name(name) + except ValueError: # e.g. instrument is 'LSTN' as given for the array element types + return array_elements()[get_array_element_type_from_name(name)]["site"] def get_collection_name_from_array_element_name(name, array_elements_only=True): diff --git a/tests/unit_tests/utils/test_names.py b/tests/unit_tests/utils/test_names.py index 272079108b..33e7cb2f30 100644 --- a/tests/unit_tests/utils/test_names.py +++ b/tests/unit_tests/utils/test_names.py @@ -142,6 +142,7 @@ def test_get_site_from_array_element_name(invalid_name): assert "South" == names.get_site_from_array_element_name("MSTS-05") with pytest.raises(ValueError, match=rf"^{invalid_name}"): names.get_site_from_array_element_name("LSTW") + assert "North" == names.get_site_from_array_element_name("North") def test_get_class_from_telescope_name(invalid_name): From 1ef03fe9ea505be08e947c84307cc233b221933a Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Tue, 18 Feb 2025 15:59:50 +0100 Subject: [PATCH 06/14] Check if parameter exists in DB --- src/simtools/applications/run_application.py | 6 ++++-- .../applications/submit_model_parameter_from_external.py | 1 - src/simtools/data_model/model_data_writer.py | 9 ++++++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/simtools/applications/run_application.py b/src/simtools/applications/run_application.py index 694e954b8c..e2d88540d0 100644 --- a/src/simtools/applications/run_application.py +++ b/src/simtools/applications/run_application.py @@ -15,8 +15,8 @@ import yaml -from simtools import dependencies import simtools.utils.general as gen +from simtools import dependencies from simtools.configuration import configurator @@ -75,7 +75,9 @@ def main(): # noqa: D103 logger = logging.getLogger() logger.setLevel(gen.get_log_level_from_user(args_dict["log_level"])) - application_config = gen.collect_data_from_file(args_dict["configuration_file"]).get("CTA_SIMPIPE") + application_config = gen.collect_data_from_file(args_dict["configuration_file"]).get( + "CTA_SIMPIPE" + ) log_file = Path(application_config.get("LOG_PATH", "./")) / "simtools.log" log_file.parent.mkdir(parents=True, exist_ok=True) configurations = application_config.get("APPLICATIONS") diff --git a/src/simtools/applications/submit_model_parameter_from_external.py b/src/simtools/applications/submit_model_parameter_from_external.py index bc888f829e..14b355be7f 100644 --- a/src/simtools/applications/submit_model_parameter_from_external.py +++ b/src/simtools/applications/submit_model_parameter_from_external.py @@ -44,7 +44,6 @@ import simtools.data_model.model_data_writer as writer import simtools.utils.general as gen from simtools.configuration import configurator -from simtools.db import db_handler def _parse(label, description): diff --git a/src/simtools/data_model/model_data_writer.py b/src/simtools/data_model/model_data_writer.py index 9b30d9544e..926511ab15 100644 --- a/src/simtools/data_model/model_data_writer.py +++ b/src/simtools/data_model/model_data_writer.py @@ -186,7 +186,9 @@ def dump_model_parameter( writer.write_dict_to_model_parameter_json(output_file, _json_dict) return _json_dict - def check_db_for_existing_parameter(self, parameter_name, instrument, parameter_version, db_config): + def check_db_for_existing_parameter( + self, parameter_name, instrument, parameter_version, db_config + ): """ Check if a parameter with the same version exists in the simulation model database. @@ -195,7 +197,7 @@ def check_db_for_existing_parameter(self, parameter_name, instrument, parameter_ parameter_name: str Name of the parameter. instrument: str - Name of the instrument. + Name of the instrument. parameter_version: str Version of the parameter. db_config: dict @@ -219,7 +221,8 @@ def check_db_for_existing_parameter(self, parameter_name, instrument, parameter_ pass # parameter does not exist - expected behavior else: raise ValueError( - f"Parameter {parameter_name} with version {parameter_version} already exists.") + f"Parameter {parameter_name} with version {parameter_version} already exists." + ) def get_validated_parameter_dict( self, From 07fd2dc3b7f9ea358f7e3b7c5da5b8c96c7adaaf Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Wed, 19 Feb 2025 20:37:14 +0100 Subject: [PATCH 07/14] docs --- docs/changes/1379.feature.md | 1 + docs/source/user-guide/applications.md | 3 ++- .../user-guide/applications/simtools-run-application.rst | 6 ++++++ 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 docs/changes/1379.feature.md create mode 100644 docs/source/user-guide/applications/simtools-run-application.rst diff --git a/docs/changes/1379.feature.md b/docs/changes/1379.feature.md new file mode 100644 index 0000000000..929e6bdd76 --- /dev/null +++ b/docs/changes/1379.feature.md @@ -0,0 +1 @@ +Add generic `simtools-run-application` to run one or several simtools using a single configuration file. diff --git a/docs/source/user-guide/applications.md b/docs/source/user-guide/applications.md index bd81ed8701..d553470ead 100644 --- a/docs/source/user-guide/applications.md +++ b/docs/source/user-guide/applications.md @@ -44,10 +44,11 @@ simtools-generate-default-metadata simtools-generate-simtel-array-histograms simtools-plot-array-layout +simtools-plot-tabular-data simtools-production-generate-simulation-config simtools-production-scale-events +simtools-run-application simtools-simulate-light-emission -simtools-plot-tabular-data simtools-simulate-prod simtools-simulate-prod-htcondor-generator simtools-submit-data-from-external diff --git a/docs/source/user-guide/applications/simtools-run-application.rst b/docs/source/user-guide/applications/simtools-run-application.rst new file mode 100644 index 0000000000..7ab0b71123 --- /dev/null +++ b/docs/source/user-guide/applications/simtools-run-application.rst @@ -0,0 +1,6 @@ + +simtools-run-application +======================== + +.. automodule:: run_application + :members: From f1494362bbb8abe959c45b2264ef8836110dc3a9 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Wed, 19 Feb 2025 20:50:01 +0100 Subject: [PATCH 08/14] unit tests --- src/simtools/data_model/model_data_writer.py | 1 - .../data_model/test_model_data_writer.py | 43 +++++++++++++++++++ tests/unit_tests/test_dependencies.py | 6 +++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/simtools/data_model/model_data_writer.py b/src/simtools/data_model/model_data_writer.py index 926511ab15..48ead05a06 100644 --- a/src/simtools/data_model/model_data_writer.py +++ b/src/simtools/data_model/model_data_writer.py @@ -215,7 +215,6 @@ def check_db_for_existing_parameter( parameter_version=parameter_version, site=names.get_site_from_array_element_name(instrument), array_element_name=instrument, - collection="telescopes", # TODO - generalize collection ) except ValueError: pass # parameter does not exist - expected behavior diff --git a/tests/unit_tests/data_model/test_model_data_writer.py b/tests/unit_tests/data_model/test_model_data_writer.py index d2ec19a8b6..aa869c6120 100644 --- a/tests/unit_tests/data_model/test_model_data_writer.py +++ b/tests/unit_tests/data_model/test_model_data_writer.py @@ -2,6 +2,7 @@ import logging from pathlib import Path +from unittest.mock import patch import astropy.units as u import numpy as np @@ -14,6 +15,7 @@ from simtools.constants import SCHEMA_PATH from simtools.data_model import schema from simtools.data_model.model_data_writer import JsonNumpyEncoder +from simtools.utils import names logger = logging.getLogger() @@ -393,3 +395,44 @@ def test_parameter_is_a_file(num_gains_schema): w1.schema_dict["data"] = [] assert not w1._parameter_is_a_file() + + +def test_check_db_for_existing_parameter(): + db_config = {"host": "localhost", "port": 27017} + parameter_name = "test_parameter" + instrument = "LSTN-01" + parameter_version = "1.0.0" + + w1 = writer.ModelDataWriter() + + with patch("simtools.data_model.model_data_writer.db_handler.DatabaseHandler") as mockdbhandler: + mock_db_instance = mockdbhandler.return_value + mock_db_instance.get_model_parameter.side_effect = ValueError("Parameter not found") + + # Test case where parameter does not exist + w1.check_db_for_existing_parameter(parameter_name, instrument, parameter_version, db_config) + mock_db_instance.get_model_parameter.assert_called_once_with( + parameter=parameter_name, + parameter_version=parameter_version, + site=names.get_site_from_array_element_name(instrument), + array_element_name=instrument, + ) + + # Reset mock for next test + mock_db_instance.get_model_parameter.reset_mock() + + # Test case where parameter exists + mock_db_instance.get_model_parameter.side_effect = None + with pytest.raises( + ValueError, + match=f"Parameter {parameter_name} with version {parameter_version} already exists.", + ): + w1.check_db_for_existing_parameter( + parameter_name, instrument, parameter_version, db_config + ) + mock_db_instance.get_model_parameter.assert_called_once_with( + parameter=parameter_name, + parameter_version=parameter_version, + site=names.get_site_from_array_element_name(instrument), + array_element_name=instrument, + ) diff --git a/tests/unit_tests/test_dependencies.py b/tests/unit_tests/test_dependencies.py index acde50a51b..b291d49ff2 100644 --- a/tests/unit_tests/test_dependencies.py +++ b/tests/unit_tests/test_dependencies.py @@ -6,6 +6,7 @@ import pytest +from simtools import dependencies from simtools.db.db_handler import DatabaseHandler from simtools.dependencies import ( get_corsika_version, @@ -50,6 +51,11 @@ def test_get_sim_telarray_version_success(): with mock.patch("subprocess.run", return_value=mock_result): assert get_sim_telarray_version() == expected_version + with mock.patch("subprocess.run", return_value=mock_result): + version_string = dependencies.get_version_string() + assert "Database version: None" in version_string + assert "sim_telarray version:" in version_string + def test_get_sim_telarray_version_no_env_var(caplog): if "SIMTOOLS_SIMTEL_PATH" in os.environ: From df94bf5cccd8999ea5aea0d83f03c6378638b9be Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Thu, 20 Feb 2025 09:14:42 +0100 Subject: [PATCH 09/14] code smell --- tests/unit_tests/test_dependencies.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/test_dependencies.py b/tests/unit_tests/test_dependencies.py index b291d49ff2..c28705c895 100644 --- a/tests/unit_tests/test_dependencies.py +++ b/tests/unit_tests/test_dependencies.py @@ -48,10 +48,11 @@ def test_get_sim_telarray_version_success(): mock_result.stdout = "Release: 2024.271.0 from 2024-09-27" mock_result.stderr = "" - with mock.patch("subprocess.run", return_value=mock_result): + subprocess_mock = "subprocess.run" + with mock.patch(subprocess_mock, return_value=mock_result): assert get_sim_telarray_version() == expected_version - with mock.patch("subprocess.run", return_value=mock_result): + with mock.patch(subprocess_mock, return_value=mock_result): version_string = dependencies.get_version_string() assert "Database version: None" in version_string assert "sim_telarray version:" in version_string From a49a74109fae621ad96e5b45547f099f8b3ff8b9 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Thu, 20 Feb 2025 15:52:22 +0100 Subject: [PATCH 10/14] metadata schema file --- src/simtools/data_model/metadata_collector.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/simtools/data_model/metadata_collector.py b/src/simtools/data_model/metadata_collector.py index 62d8c3e4f2..3eb4a5a423 100644 --- a/src/simtools/data_model/metadata_collector.py +++ b/src/simtools/data_model/metadata_collector.py @@ -15,6 +15,7 @@ import simtools.constants import simtools.utils.general as gen import simtools.version +from simtools.constants import METADATA_JSON_SCHEMA from simtools.data_model import metadata_model, schema from simtools.io_operations import io_handler from simtools.utils import names @@ -266,7 +267,7 @@ def _read_input_metadata_from_file(self, metadata_file_name=None): self._logger.error("Unknown metadata file format: %s", metadata_file_name) raise gen.InvalidConfigDataError - schema.validate_dict_using_schema(_input_metadata, None) + schema.validate_dict_using_schema(_input_metadata, schema_file=METADATA_JSON_SCHEMA) return gen.change_dict_keys_case( self._process_metadata_from_file(_input_metadata), From b6a47c7e1dc5972b88f4c108c755a59d4e1b623c Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Thu, 20 Feb 2025 16:21:47 +0100 Subject: [PATCH 11/14] fix sim_telarray path --- src/simtools/dependencies.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/simtools/dependencies.py b/src/simtools/dependencies.py index 6c83d70212..528b03c3e2 100644 --- a/src/simtools/dependencies.py +++ b/src/simtools/dependencies.py @@ -63,7 +63,7 @@ def get_sim_telarray_version(): if sim_telarray_path is None: _logger.warning("Environment variable SIMTOOLS_SIMTEL_PATH is not set.") return None - sim_telarray_path = Path(sim_telarray_path) / "bin" / "sim_telarray" + sim_telarray_path = Path(sim_telarray_path) / "sim_telarray" / "bin" / "sim_telarray" # expect stdout with e.g. a line 'Release: 2024.271.0 from 2024-09-27' result = subprocess.run( From 787201df5467f38c2d73c6f114ec4574b2fd31d8 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Mon, 24 Feb 2025 10:06:35 +0100 Subject: [PATCH 12/14] improve docsstring --- src/simtools/utils/names.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/simtools/utils/names.py b/src/simtools/utils/names.py index e820b2a14d..7f146ce5e3 100644 --- a/src/simtools/utils/names.py +++ b/src/simtools/utils/names.py @@ -658,7 +658,7 @@ def file_name_with_version(file_name, suffix): """ Return a file name including a semantic version with the correct suffix. - Avoids having 'Path.suffix()' to remove trailing numbers. + Replaces 'Path.suffix()', which removes trailing numbers (and therefore version numbers). Parameters ---------- From f371dad8815212a3af247bc4842daf3be9b963a4 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Mon, 24 Feb 2025 18:16:33 +0100 Subject: [PATCH 13/14] Fix bug in config reading from APPLICATIONS dict --- src/simtools/configuration/configurator.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/simtools/configuration/configurator.py b/src/simtools/configuration/configurator.py index 3381f36682..f4da44814c 100644 --- a/src/simtools/configuration/configurator.py +++ b/src/simtools/configuration/configurator.py @@ -287,16 +287,14 @@ def _fill_from_config_file(self, config_file): ) # yaml parser adds \n in multiline strings, remove them _config_dict = gen.remove_substring_recursively_from_dict(_config_dict, substring="\n") - if "CTA_SIMPIPE" in _config_dict: - try: - self._fill_from_config_dict( - input_dict=gen.change_dict_keys_case( - _config_dict["CTA_SIMPIPE"]["CONFIGURATION"], - ), - overwrite=True, - ) - except KeyError: - self._logger.info(f"No CTA_SIMPIPE:CONFIGURATION dict found in {config_file}.") + # read configuration for first application + if "CONFIGURATION" in _config_dict.get("CTA_SIMPIPE", {}).get("APPLICATIONS", [{}])[0]: + self._fill_from_config_dict( + input_dict=gen.change_dict_keys_case( + _config_dict["CTA_SIMPIPE"]["APPLICATIONS"][0]["CONFIGURATION"], + ), + overwrite=True, + ) else: self._fill_from_config_dict( input_dict=gen.change_dict_keys_case(_config_dict), overwrite=True From 2df171b75f37347d902db4d134d7a6ec57d29471 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Tue, 25 Feb 2025 09:25:20 +0100 Subject: [PATCH 14/14] unit test fix --- .../unit_tests/configuration/test_configurator.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/tests/unit_tests/configuration/test_configurator.py b/tests/unit_tests/configuration/test_configurator.py index 12bac9f90e..f3848d3fde 100644 --- a/tests/unit_tests/configuration/test_configurator.py +++ b/tests/unit_tests/configuration/test_configurator.py @@ -108,7 +108,9 @@ def test_fill_from_workflow_config_file(configurator, args_dict, tmp_test_direct "output_path": "./abc/", "test": True, } - _tmp_dict_workflow = {"CTA_SIMPIPE": {"CONFIGURATION": _tmp_dict}} + _tmp_dict_workflow = { + "CTA_SIMPIPE": {"APPLICATIONS": [{"APPLICATION": "test", "CONFIGURATION": _tmp_dict}]} + } _workflow_file = tmp_test_directory / "configuration-test.yml" with open(_workflow_file, "w") as output: yaml.safe_dump(_tmp_dict_workflow, output, sort_keys=False) @@ -125,17 +127,6 @@ def test_fill_from_workflow_config_file(configurator, args_dict, tmp_test_direct _tmp_config[key] = value assert _tmp_config == configurator.config - # test that no KeyError is raised for "CTA_SIMPIPE:NO_CONFIGURATION" - _tmp_dict_workflow = {"CTA_SIMPIPE": {"NO_CONFIGURATION": _tmp_dict}} - _workflow_file = tmp_test_directory / "configuration-test-2.yml" - with open(_workflow_file, "w") as output: - yaml.safe_dump(_tmp_dict_workflow, output, sort_keys=False) - configurator.config["config"] = str(_workflow_file) - _tmp_config["config"] = str(_workflow_file) - configurator.config["output_path"] = None - # no KeyError - configurator._fill_from_config_file(_workflow_file) - def test_initialize_io_handler(configurator, tmp_test_directory): # io_handler is a Singleton, so configurator changes should