diff --git a/docs/changes/2088.feature.md b/docs/changes/2088.feature.md new file mode 100644 index 0000000000..efe4908b4e --- /dev/null +++ b/docs/changes/2088.feature.md @@ -0,0 +1,2 @@ +Added a unified parameter export flow for both file-backed and dict-backed table parameters, with stricter validation and normalization of table content including required `column_units` for `fadc_pulse_shape`. +Plot table generation now selects the schema document by `model_parameter_schema_version` (with fallback), and sim_telarray/model/database handling was aligned with expanded unit test coverage. diff --git a/docs/source/api-reference/data_model.md b/docs/source/api-reference/data_model.md index ac852a32cc..16de69828f 100644 --- a/docs/source/api-reference/data_model.md +++ b/docs/source/api-reference/data_model.md @@ -67,3 +67,12 @@ Data products ingested or produced by simtools generally follows the CTAO data m .. automodule:: data_model.validate_data :members: ``` + +## row_table_utils + +(row-table-utils-1)= + +```{eval-rst} +.. automodule:: data_model.row_table_utils + :members: +``` diff --git a/docs/source/api-reference/db_handler.md b/docs/source/api-reference/db_handler.md index 47eddf6d76..8942a809e6 100644 --- a/docs/source/api-reference/db_handler.md +++ b/docs/source/api-reference/db_handler.md @@ -30,3 +30,12 @@ Modules for database access. See the databases sections for details. .. automodule:: db.db_model_upload :members: ``` + +## parameter_exporter + +(db-parameter-exporter)= + +```{eval-rst} +.. automodule:: db.parameter_exporter + :members: +``` diff --git a/docs/source/api-reference/sim_telarray.md b/docs/source/api-reference/sim_telarray.md index 51eed1d660..c155e50127 100644 --- a/docs/source/api-reference/sim_telarray.md +++ b/docs/source/api-reference/sim_telarray.md @@ -40,6 +40,15 @@ Support modules for running sim_telarray. :members: ``` +## simtel_table_writer + +(simtel-table-writer-1)= + +```{eval-rst} +.. automodule:: simtel.simtel_table_writer + :members: +``` + ## simtel_io_metadata (simtel-io-metadata-1)= diff --git a/src/simtools/applications/db_get_file_from_db.py b/src/simtools/applications/db_get_file_from_db.py index 1fd2376255..32eb587437 100644 --- a/src/simtools/applications/db_get_file_from_db.py +++ b/src/simtools/applications/db_get_file_from_db.py @@ -61,15 +61,16 @@ def main(): app_context = startup_application(_parse) db = db_handler.DatabaseHandler() - file_id = db.export_model_files( - dest=app_context.io_handler.get_output_directory(), - file_names=app_context.args["file_name"], - ) - if file_id is None: - app_context.logger.error( - f"The file {app_context.args['file_name']} was not found in {db.db_name}." + try: + db.export_model_files( + dest=app_context.io_handler.get_output_directory(), + file_names=app_context.args["file_name"], ) - raise FileNotFoundError + except FileNotFoundError as exc: + raise FileNotFoundError( + f"The file {app_context.args['file_name']} was not found in {db.db_name}." + ) from exc + app_context.logger.info( f"Got file {app_context.args['file_name']} from DB {db.db_name} " f"and saved into {app_context.io_handler.get_output_directory()}" diff --git a/src/simtools/applications/db_get_parameter_from_db.py b/src/simtools/applications/db_get_parameter_from_db.py index c3fe4a9133..592582e6fc 100644 --- a/src/simtools/applications/db_get_parameter_from_db.py +++ b/src/simtools/applications/db_get_parameter_from_db.py @@ -3,10 +3,19 @@ r""" Get a parameter entry from DB for a specific telescope or a site. - The application receives a parameter name, a site, a telescope (if applicable) and - a version. Allow to print the parameter entry to screen or save it to a file. - Parameter describing a table file can be written to disk or exported as an astropy table - (if available). + The application supports three output modes: + + 1. Print the database entry to stdout. + 2. Write the database entry to a JSON or YAML file using output_file. + 3. Export table-type model parameters using export_model_file. + + The export_model_file mode is type-dependent: + + - File-backed parameters are exported with their original file name from the database. + - Dict-backed table parameters are exported as ECSV, using output_file as the base name. + + For file-backed parameters, export_model_file_as_table can be added to also write an + ECSV representation next to the exported file. Command line arguments ---------------------- @@ -26,13 +35,16 @@ Telescope model name (e.g. LST-1, SST-D, ...) output_file (str, optional) - Output file name. If not given, print to stdout. + Output file name for writing the database entry, or base file name for + exporting dict-backed tables as ECSV. export_model_file (bool, optional) - Export model file (if parameter describes a file). + Export parameter data. File-backed parameters are written as model files. + Embedded dict-typed table parameters are written as ECSV using output_file. export_model_file_as_table (bool, optional) - Export model file as astropy table (if parameter describes a file). + Export file-backed parameters as astropy tables in addition to the + original file export. Use together with export_model_file. Raises ------ @@ -40,7 +52,7 @@ Example ------- - Get the mirror_list parameter used for a given model_version from the DB. + Print the mirror_list parameter entry used for a given model_version. .. code-block:: console @@ -48,8 +60,16 @@ --site North --telescope LSTN-01 \\ --model_version 5.0.0 - Get the mirror_list parameter using the parameter_version from the DB. - Write the mirror list to disk. + Write the database entry for a parameter to a JSON file. + + .. code-block:: console + + simtools-db-get-parameter-from-db --parameter array_element_position_ground \\ + --site North --telescope LSTN-01 \\ + --parameter_version 6.0.0 \\ + --output_file array_element_position_ground.json + + Export a file-backed parameter using the original file name stored in the database. .. code-block:: console @@ -58,6 +78,24 @@ --parameter_version 1.0.0 \\ --export_model_file + Export a file-backed parameter and also write an ECSV table representation. + + .. code-block:: console + + simtools-db-get-parameter-from-db --parameter mirror_reflectivity \\ + --site North --telescope LSTN-01 \\ + --model_version 6.0.2 \\ + --export_model_file --export_model_file_as_table + + Export a dict-backed table parameter as ECSV. The .ecsv suffix is added automatically. + + .. code-block:: console + + simtools-db-get-parameter-from-db --parameter fadc_pulse_shape \\ + --site North --telescope LSTN-01 \\ + --parameter_version 2.0.0 \\ + --export_model_file --output_file fadc_pulse_shape + """ from pprint import pprint @@ -72,33 +110,39 @@ def _parse(): """Parse command line configuration.""" config = configurator.Configurator( label=get_application_label(__file__), - description=( - "Get a parameter entry from DB for a specific telescope or a site. " - "The application receives a parameter name, a site, a telescope (if applicable), " - "and a version. It then prints out the parameter entry. " - ), + description=("Export a parameter entry from model parameter database."), ) config.parser.add_argument("--parameter", help="Parameter name", type=str, required=True) config.parser.add_argument( "--output_file", - help="output file name (if not given: print to stdout)", + help=( + "Output file name for writing the DB entry, or base name for ECSV export of " + "dict-backed tables." + ), type=str, required=False, ) config.parser.add_argument( "--export_model_file", - help="Export model file (if parameter describes a file)", + help=( + "Export parameter data. File-backed parameters are written as files; " + "embedded dict-typed table parameters are written as ECSV using --output_file." + ), action="store_true", required=False, ) config.parser.add_argument( "--export_model_file_as_table", - help="Export model file as astropy table (if parameter describes a file)", + help=( + "Also export file-backed parameters as ECSV. Use together with " + "--export_model_file. " + "(legacy option; as file-backed parameters will be replaced by table-backed ones, " + "this option will be removed in the future)" + ), action="store_true", required=False, ) - return config.initialize( db_config=True, simulation_model=["telescope", "parameter_version", "model_version"] ) @@ -110,6 +154,21 @@ def main(): db = db_handler.DatabaseHandler() + if app_context.args["export_model_file"] or app_context.args["export_model_file_as_table"]: + output_files = db.export_parameter_data( + parameter=app_context.args["parameter"], + site=app_context.args["site"], + array_element_name=app_context.args.get("telescope"), + parameter_version=app_context.args.get("parameter_version"), + model_version=app_context.args.get("model_version"), + output_file=app_context.args.get("output_file"), + export_model_file=app_context.args["export_model_file"], + export_model_file_as_table=app_context.args["export_model_file_as_table"], + ) + for output_file in output_files: + app_context.logger.info(f"Exported parameter output to {output_file}") + return + pars = db.get_model_parameter( parameter=app_context.args["parameter"], site=app_context.args["site"], @@ -117,23 +176,6 @@ def main(): parameter_version=app_context.args.get("parameter_version"), model_version=app_context.args.get("model_version"), ) - if app_context.args["export_model_file"] or app_context.args["export_model_file_as_table"]: - table = db.export_model_file( - parameter=app_context.args["parameter"], - site=app_context.args["site"], - array_element_name=app_context.args["telescope"], - parameter_version=app_context.args.get("parameter_version"), - model_version=app_context.args.get("model_version"), - export_file_as_table=app_context.args["export_model_file_as_table"], - ) - param_value = pars[app_context.args["parameter"]]["value"] - table_file = app_context.io_handler.get_output_file(param_value) - app_context.logger.info(f"Exported model file {param_value} to {table_file}") - if table and table_file.suffix != ".ecsv": - table.write(table_file.with_suffix(".ecsv"), format="ascii.ecsv", overwrite=True) - app_context.logger.info( - f"Exported model file {param_value} to {table_file.with_suffix('.ecsv')}" - ) if app_context.args["output_file"] is not None: pars[app_context.args["parameter"]].pop("_id") diff --git a/src/simtools/applications/submit_model_parameter_from_external.py b/src/simtools/applications/submit_model_parameter_from_external.py index dce276a0f7..0e4579d3ee 100644 --- a/src/simtools/applications/submit_model_parameter_from_external.py +++ b/src/simtools/applications/submit_model_parameter_from_external.py @@ -18,6 +18,8 @@ site location. parameter_version (str) Parameter version. + model_parameter_schema_version (str, optional) + Version of the model-parameter schema to use for validation and value interpretation. input_meta (str, optional) input meta data file (yml format) @@ -43,6 +45,7 @@ import simtools.data_model.model_data_writer as writer from simtools.application_control import get_application_label, startup_application from simtools.configuration import configurator +from simtools.simtel import simtel_table_reader def _parse(): @@ -60,6 +63,12 @@ def _parse(): config.parser.add_argument( "--parameter_version", type=str, required=True, help="Parameter version" ) + config.parser.add_argument( + "--model_parameter_schema_version", + type=str, + required=False, + help="Model-parameter schema version to use for validation and value interpretation", + ) config.parser.add_argument( "--value", type=str, @@ -88,6 +97,23 @@ def _parse(): def main(): """Submit and validate a model parameter value and metadata.""" app_context = startup_application(_parse) + model_parameter_schema_version = app_context.args.get("model_parameter_schema_version") + value = app_context.args["value"] + data_writer = writer.ModelDataWriter() + parameter_type = data_writer.get_parameter_type_for_schema( + app_context.args["parameter"], + model_parameter_schema_version, + ) + + if parameter_type == "dict" and data_writer.parameter_uses_row_table_schema( + app_context.args["parameter"], + model_parameter_schema_version, + ): + value = simtel_table_reader.resolve_dict_parameter_value( + value, + app_context.args["parameter"], + app_context.args.get("data_path"), + ) if app_context.args.get("output_path"): output_path = app_context.io_handler.get_output_directory( @@ -98,7 +124,7 @@ def main(): writer.ModelDataWriter.dump_model_parameter( parameter_name=app_context.args["parameter"], - value=app_context.args["value"], + value=value, instrument=app_context.args["instrument"], parameter_version=app_context.args["parameter_version"], output_file=Path( @@ -107,6 +133,7 @@ def main(): output_path=output_path, metadata_input_dict=app_context.args, check_db_for_existing_parameter=app_context.args.get("check_parameter_version", False), + model_parameter_schema_version=model_parameter_schema_version, ) diff --git a/src/simtools/data_model/model_data_writer.py b/src/simtools/data_model/model_data_writer.py index 48b30a6e63..40b94125e7 100644 --- a/src/simtools/data_model/model_data_writer.py +++ b/src/simtools/data_model/model_data_writer.py @@ -8,7 +8,7 @@ import simtools.utils.general as gen from simtools import settings -from simtools.data_model import schema, validate_data +from simtools.data_model import row_table_utils, schema, validate_data from simtools.data_model.metadata_collector import MetadataCollector from simtools.db import db_handler from simtools.io import ascii_handler, io_handler @@ -139,11 +139,11 @@ def dump_model_parameter( writer.check_db_for_existing_parameter(parameter_name, instrument, parameter_version) unique_id = None + metadata = 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) - metadata.write(output_path / Path(output_file)) unique_id = ( metadata.get_top_level_metadata().get("cta", {}).get("product", {}).get("id") ) @@ -159,6 +159,8 @@ def dump_model_parameter( meta_parameter=meta_parameter, ) writer.write_dict_to_model_parameter_json(output_file, _json_dict) + if metadata is not None: + metadata.write(output_path / Path(output_file)) return _json_dict def check_db_for_existing_parameter(self, parameter_name, instrument, parameter_version): @@ -317,16 +319,79 @@ def _find_highest_schema_version(self, schema_list): raise TypeError("No valid schema versions found in the list.") from exc return max(valid_entries, key=lambda e: packaging.version.Version(e["schema_version"])) + def get_parameter_type_for_schema(self, parameter_name, model_parameter_schema_version=None): + """ + Return parameter type(s) from a selected model-parameter schema. + + This method loads the schema for ``parameter_name`` and an optional + ``model_parameter_schema_version``, then extracts the ``type`` field from + each data entry. If all entries share the same type, a single string is + returned; otherwise a list of types is returned. + + Parameters + ---------- + parameter_name: str + Name of the model parameter. + model_parameter_schema_version: str or None + Explicit model-parameter schema version to use. If None, the newest + available schema version is selected. + + Returns + ------- + str or list[str] + Parameter type, reduced to a single value when all schema entries + have the same type. + """ + schema_dict, _ = self._read_schema_dict(parameter_name, model_parameter_schema_version) + parameter_types = [data["type"] for data in schema_dict["data"]] + return ( + parameter_types[0] + if all(data_type == parameter_types[0] for data_type in parameter_types) + else parameter_types + ) + + def parameter_uses_row_table_schema(self, parameter_name, model_parameter_schema_version=None): + """Return True if selected schema defines row-oriented table dict payload. + + Parameters + ---------- + parameter_name: str + Name of the model parameter. + model_parameter_schema_version: str or None + Explicit model-parameter schema version to use. If None, the newest + available schema version is selected. + + Returns + ------- + bool + True when a dict-typed schema entry requires the row-table keys + ``columns``, ``rows`` and ``column_units``. + """ + schema_dict, _ = self._read_schema_dict(parameter_name, model_parameter_schema_version) + + for data_entry in schema_dict.get("data", []): + if data_entry.get("type") != "dict": + continue + + json_schema = data_entry.get("json_schema", {}) + if row_table_utils.is_row_table_schema(json_schema): + return True + + return False + def _get_parameter_type(self): """ - Return parameter type from schema. + Return parameter type(s) from the currently loaded schema. - Reduce list of types to single type if all types are the same. + This helper reads ``self.schema_dict`` (expected to be populated by + ``_read_schema_dict`` beforehand), extracts ``type`` values from its + data entries, and reduces the result to a single string when all types + are identical. Returns ------- str or list[str] - Parameter type + Parameter type derived from ``self.schema_dict``. """ _parameter_type = [data["type"] for data in self.schema_dict["data"]] return ( diff --git a/src/simtools/data_model/row_table_utils.py b/src/simtools/data_model/row_table_utils.py new file mode 100644 index 0000000000..d88a0a8d5e --- /dev/null +++ b/src/simtools/data_model/row_table_utils.py @@ -0,0 +1,178 @@ +"""Utilities for row-oriented table data (columns, column_units, rows).""" + +import logging +from typing import Any + +import astropy.units as u +import numpy as np + +logger = logging.getLogger(__name__) + +# Canonical set of row-table dict keys +ROW_TABLE_KEYS = {"columns", "rows", "column_units"} + + +def is_row_table_dict(value: Any) -> bool: + """ + Check if a dict has the row-table structure (columns, rows, column_units). + + Parameters + ---------- + value : Any + Value to check. + + Returns + ------- + bool + True if value is a dict with all three required keys. + """ + if not isinstance(value, dict): + return False + return all(key in value for key in ROW_TABLE_KEYS) + + +def is_row_table_schema(json_schema: dict[str, Any]) -> bool: + """ + Check if a JSON schema defines row-table shape. + + Requires ``columns``, ``rows``, and ``column_units`` keys. + + Parameters + ---------- + json_schema : dict + JSON schema properties dict. + + Returns + ------- + bool + True if schema specifies row-table structure. + """ + required = set(json_schema.get("required", [])) + properties = set(json_schema.get("properties", {}).keys()) + return ROW_TABLE_KEYS.issubset(required) and ROW_TABLE_KEYS.issubset(properties) + + +def validate_row_table_structure( + parameter_name: str, value: dict[str, Any], require_column_units: bool = True +) -> None: + """ + Validate row-table dict structure and value consistency. + + Checks: + - Dict contains 'columns' and 'rows' keys + - 'columns' and 'rows' are sequences (list or tuple) + - All column names are strings + - Each row is a sequence with correct length and numeric values + - If require_column_units is True (default), validates 'column_units' presence and length + + Parameters + ---------- + parameter_name : str + Parameter name (for error messages). + value : dict + Row-table dict to validate. + require_column_units : bool, optional + If True (default), requires 'column_units' and validates its length. + + Raises + ------ + ValueError + If structure is invalid or values are non-numeric. + """ + if not isinstance(value, dict) or "columns" not in value or "rows" not in value: + raise ValueError( + f"Row-table value for '{parameter_name}' must be a dict with 'columns' and 'rows' keys." + ) + + columns = value["columns"] + rows = value["rows"] + + if not isinstance(columns, (list, tuple)): + raise ValueError( + f"Row-table for '{parameter_name}': 'columns' must be a list or tuple, " + f"got {type(columns).__name__}." + ) + + if not isinstance(rows, (list, tuple)): + raise ValueError( + f"Row-table for '{parameter_name}': 'rows' must be a list or tuple, " + f"got {type(rows).__name__}." + ) + + if not all(isinstance(column_name, str) for column_name in columns): + raise ValueError( + f"Row-table for '{parameter_name}': all column names in 'columns' must be strings." + ) + + if require_column_units: + if "column_units" not in value: + raise ValueError(f"Row-table value for '{parameter_name}' must include 'column_units'.") + column_units = value["column_units"] + if len(column_units) != len(columns): + raise ValueError( + f"Row-table for '{parameter_name}': column_units length ({len(column_units)}) " + f"must match columns length ({len(columns)})." + ) + + _validate_row_values(parameter_name, columns, rows) + + +def _validate_row_values(parameter_name: str, columns: list[str], rows: list) -> None: + """Validate numeric scalars and row length consistency.""" + n_columns = len(columns) + for row_index, row in enumerate(rows): + if not isinstance(row, (list, tuple, np.ndarray)): + raise ValueError( + f"Row-table for '{parameter_name}' has invalid row at index {row_index}: " + "each row must be a sequence with one numeric value per column." + ) + + if len(row) != n_columns: + raise ValueError( + f"Row-table for '{parameter_name}' has invalid row length at index {row_index}: " + f"expected {n_columns} values, got {len(row)}." + ) + + for col_index, value in enumerate(row): + if not np.isscalar(value): + raise ValueError( + f"Row-table for '{parameter_name}' has non-numeric value at " + f"row {row_index}, column {col_index} ('{columns[col_index]}'): {value!r}." + ) + + value_dtype = np.asarray(value).dtype + is_numeric = np.issubdtype(value_dtype, np.number) + is_real = not np.issubdtype(value_dtype, np.complexfloating) + + if not (is_numeric and is_real): + raise ValueError( + f"Row-table for '{parameter_name}' has non-real-numeric value at " + f"row {row_index}, column {col_index} ('{columns[col_index]}'): {value!r}." + ) + + +def normalize_column_unit(unit_value: Any) -> str: + """ + Convert astropy unit or string to schema-compatible unit string. + + Parameters + ---------- + unit_value : Any + Unit value (astropy.units.Unit, str, or None). + + Returns + ------- + str + Normalized unit string compatible with model parameter schemas. + """ + if unit_value is None: + return "dimensionless" + + if isinstance(unit_value, str): + return unit_value if unit_value else "dimensionless" + + if unit_value == u.dimensionless_unscaled: + return "dimensionless" + + unit_str = str(unit_value) + return unit_str if unit_str else "dimensionless" diff --git a/src/simtools/db/db_handler.py b/src/simtools/db/db_handler.py index c9995ae3ce..2299bfe525 100644 --- a/src/simtools/db/db_handler.py +++ b/src/simtools/db/db_handler.py @@ -6,9 +6,9 @@ from simtools import settings from simtools.data_model import validate_data +from simtools.db import parameter_exporter from simtools.db.mongo_db import MongoDBHandler from simtools.io import io_handler -from simtools.simtel import simtel_table_reader from simtools.utils import names, value_conversion from simtools.version import resolve_version_to_latest_patch @@ -326,7 +326,9 @@ def export_model_file( Export single model file from the DB identified by the parameter name. The parameter can be identified by model or parameter version. - Files can be exported as astropy tables (ecsv format). + File-backed parameters can be exported as astropy tables (ecsv format). + Embedded dict-typed parameters are converted to an astropy table directly + from the stored row data. Parameters ---------- @@ -341,27 +343,23 @@ def export_model_file( model_version: str Version of the model. export_file_as_table: bool - If True, export the file as an astropy table (ecsv format). + If True, export the parameter value as an astropy table. Returns ------- astropy.table.Table or None - If export_file_as_table is True - """ - parameters = self.get_model_parameter( - parameter, - site, - array_element_name, + Astropy table when export_file_as_table is True and the parameter + value is a table (file-backed or embedded dict), otherwise None. + """ + return parameter_exporter.export_single_model_file( + db=self, + parameter=parameter, + site=site, + array_element_name=array_element_name, parameter_version=parameter_version, model_version=model_version, + export_file_as_table=export_file_as_table, ) - self.export_model_files(parameters=parameters, dest=self.io_handler.get_output_directory()) - if export_file_as_table: - return simtel_table_reader.read_simtel_table( - parameter, - self.io_handler.get_output_directory().joinpath(parameters[parameter]["value"]), - ) - return None def export_model_files(self, parameters=None, file_names=None, dest=None, db_name=None): """ @@ -384,26 +382,37 @@ def export_model_files(self, parameters=None, file_names=None, dest=None, db_nam file_id: dict of GridOut._id Dict of database IDs of files. """ - db_name = db_name or self.db_name - - if file_names: - file_names = [file_names] if not isinstance(file_names, list) else file_names - elif parameters: - file_names = [ - info["value"] - for info in parameters.values() - if info and info.get("file") and info["value"] is not None - ] + return parameter_exporter.export_model_files( + db=self, + parameters=parameters, + file_names=file_names, + dest=dest, + db_name=db_name, + ) - instance_ids = {} - for file_name in file_names: - if Path(dest).joinpath(file_name).exists(): - instance_ids[file_name] = "file exists" - else: - file_path_instance = self.mongo_db_handler.get_file_from_db(db_name, file_name) - self._write_file_from_db_to_disk(db_name, dest, file_path_instance) - instance_ids[file_name] = file_path_instance._id # pylint: disable=protected-access - return instance_ids + def export_parameter_data( + self, + parameter, + site, + array_element_name, + parameter_version=None, + model_version=None, + output_file=None, + export_model_file=False, + export_model_file_as_table=False, + ): + """Export parameter payload based on parameter type and export options.""" + return parameter_exporter.export_parameter_data( + db=self, + parameter=parameter, + site=site, + array_element_name=array_element_name, + parameter_version=parameter_version, + model_version=model_version, + output_file=output_file, + export_model_file=export_model_file, + export_model_file_as_table=export_model_file_as_table, + ) def _get_query_from_parameter_version_table( self, parameter_version_table, array_element_name, site @@ -644,7 +653,7 @@ def get_simulation_configuration_parameters( ) raise ValueError(f"Unknown simulation software: {simulation_software}") - def _write_file_from_db_to_disk(self, db_name, path, file): + def write_file_from_db_to_disk(self, db_name, path, file): """ Extract a file from MongoDB and write it to disk. @@ -657,7 +666,11 @@ def _write_file_from_db_to_disk(self, db_name, path, file): file: GridOut A file instance returned by GridFS find_one """ - self.mongo_db_handler.write_file_from_db_to_disk(db_name, path, file) + parameter_exporter.write_file_from_db_to_disk(self, db_name, path, file) + + def _write_file_from_db_to_disk(self, db_name, path, file): + """Backward-compatible alias for write_file_from_db_to_disk.""" + self.write_file_from_db_to_disk(db_name, path, file) def get_ecsv_file_as_astropy_table(self, file_name, db_name=None): """ diff --git a/src/simtools/db/parameter_exporter.py b/src/simtools/db/parameter_exporter.py new file mode 100644 index 0000000000..9c24378546 --- /dev/null +++ b/src/simtools/db/parameter_exporter.py @@ -0,0 +1,345 @@ +"""Utilities for exporting model parameter values / files from the database.""" + +from pathlib import Path + +from simtools.data_model import row_table_utils +from simtools.simtel import simtel_table_reader + +ECSV_SUFFIX = ".ecsv" + + +def _is_dict_table_value(parameter_info): + """Return True if a parameter stores embedded row-oriented table data.""" + return parameter_info.get("type") == "dict" and row_table_utils.is_row_table_dict( + parameter_info.get("value") + ) + + +def _get_parameter_info( + db, + parameter, + site, + array_element_name, + parameter_version=None, + model_version=None, +): + """Fetch single-parameter metadata dict from DB.""" + parameters = db.get_model_parameter( + parameter, + site, + array_element_name, + parameter_version=parameter_version, + model_version=model_version, + ) + return parameters, parameters[parameter] + + +def _normalize_file_names(file_names=None, parameters=None): + """Normalize file_names input or derive it from parameter metadata.""" + if file_names: + return [file_names] if not isinstance(file_names, list) else file_names + if parameters: + return [ + info["value"] + for info in parameters.values() + if isinstance(info, dict) and info.get("file") and info.get("value") is not None + ] + return [] + + +def write_file_from_db_to_disk(db, db_name, path, file): + """ + Write one file object from GridFS to disk. + + Parameters + ---------- + db : DatabaseHandler + Database handler wrapper. + db_name : str + Database name. + path : str or Path + Output directory. + file : gridfs.grid_file.GridOut + File object returned by GridFS. + """ + db.mongo_db_handler.write_file_from_db_to_disk(db_name, path, file) + + +def export_model_files(db, parameters=None, file_names=None, dest=None, db_name=None): + """ + Export model files from DB to a destination directory. + + Parameters + ---------- + db : DatabaseHandler + Database handler wrapper. + parameters : dict, optional + Parameter metadata dictionary used to derive file names. + file_names : str or list[str], optional + File name or list of file names to export. + dest : str or Path + Output directory. + db_name : str, optional + Database name. Uses ``db.db_name`` when omitted. + + Returns + ------- + dict + Mapping of file name to GridFS id or ``"file exists"``. + + Raises + ------ + ValueError + If ``dest`` is not provided. + """ + if dest is None: + raise ValueError("Destination path is required to export model files.") + + db_name = db_name or db.db_name + file_names = _normalize_file_names(file_names=file_names, parameters=parameters) + destination = Path(dest) + + instance_ids = {} + for file_name in file_names: + if destination.joinpath(file_name).exists(): + instance_ids[file_name] = "file exists" + else: + file_path_instance = db.mongo_db_handler.get_file_from_db(db_name, file_name) + db.write_file_from_db_to_disk(db_name, dest, file_path_instance) + instance_ids[file_name] = file_path_instance._id # pylint: disable=protected-access + return instance_ids + + +def _export_dict_table_parameter( + db, + parameter, + site, + array_element_name, + output_file, + par_info, + parameters, + parameter_version=None, + model_version=None, +): + """ + Export dict-typed (embedded table) parameter to ECSV file. + + Returns the output file path. + """ + if output_file is None: + raise ValueError( + "Use --output_file when exporting dict-typed parameters with " + "--export_model_file or --export_model_file_as_table." + ) + + table = export_single_model_file( + db=db, + parameter=parameter, + site=site, + array_element_name=array_element_name, + parameter_version=parameter_version, + model_version=model_version, + export_file_as_table=True, + parameters=parameters, + par_info=par_info, + ) + table_file = db.io_handler.get_output_file(output_file).with_suffix(ECSV_SUFFIX) + table.write(table_file, format="ascii.ecsv", overwrite=True) + return [table_file] + + +def _export_file_backed_parameter( + db, + parameter, + site, + array_element_name, + par_info, + parameters, + export_model_file_as_table, + parameter_version=None, + model_version=None, +): + """ + Export file-backed parameter to disk. + + Exports the file and optionally also as an ECSV table. + """ + table = export_single_model_file( + db=db, + parameter=parameter, + site=site, + array_element_name=array_element_name, + parameter_version=parameter_version, + model_version=model_version, + export_file_as_table=export_model_file_as_table, + parameters=parameters, + par_info=par_info, + ) + param_value = par_info["value"] + table_file = db.io_handler.get_output_file(param_value) + output_files = [table_file] + + if table and table_file.suffix != ECSV_SUFFIX: + table_output_file = table_file.with_suffix(ECSV_SUFFIX) + table.write(table_output_file, format="ascii.ecsv", overwrite=True) + output_files.append(table_output_file) + + return output_files + + +def export_single_model_file( + db, + parameter, + site, + array_element_name, + model_version=None, + parameter_version=None, + export_file_as_table=False, + parameters=None, + par_info=None, +): + """ + Export one parameter payload and optionally return it as a table. + + Parameters + ---------- + db : DatabaseHandler + Database handler wrapper. + parameter : str + Parameter name. + site : str + Site name. + array_element_name : str + Array element name. + model_version : str, optional + Model version. + parameter_version : str, optional + Parameter version. + export_file_as_table : bool, optional + If True, return an ``astropy.table.Table`` when possible. + parameters : dict, optional + Prefetched parameter dictionary. + par_info : dict, optional + Prefetched single-parameter entry. + + Returns + ------- + astropy.table.Table or None + Exported table when requested and available, otherwise None. + """ + if parameters is None or par_info is None: + parameters, par_info = _get_parameter_info( + db=db, + parameter=parameter, + site=site, + array_element_name=array_element_name, + parameter_version=parameter_version, + model_version=model_version, + ) + + if _is_dict_table_value(par_info): + if export_file_as_table: + return simtel_table_reader.row_data_to_astropy_table(par_info["value"]) + return None + + db.export_model_files(parameters=parameters, dest=db.io_handler.get_output_directory()) + if export_file_as_table: + return simtel_table_reader.read_simtel_table( + parameter, + db.io_handler.get_output_directory().joinpath(par_info["value"]), + ) + return None + + +def export_parameter_data( + db, + parameter, + site, + array_element_name, + parameter_version=None, + model_version=None, + output_file=None, + export_model_file=False, + export_model_file_as_table=False, +): + """ + Export parameter payload based on type and export flags. + + Parameters + ---------- + db : DatabaseHandler + DatabaseHandler instance used for DB access and file output. + parameter : str + Name of the parameter. + site : str + Site name. + array_element_name : str + Name of the array element model (e.g. LSTN-01). + parameter_version : str, optional + Version of the parameter. + model_version : str, optional + Version of the model. + output_file : str, optional + Output file name for dict-backed table exports. + export_model_file : bool, optional + Export payload to files. + export_model_file_as_table : bool, optional + Also export file-backed payload as ECSV table. + + Returns + ------- + list[Path] + Output file paths. + + Raises + ------ + ValueError + If an incompatible combination of options is provided. + """ + if export_model_file_as_table and not export_model_file: + raise ValueError("Use --export_model_file together with --export_model_file_as_table.") + + if not (export_model_file or export_model_file_as_table): + return [] + + parameters, par_info = _get_parameter_info( + db=db, + parameter=parameter, + site=site, + array_element_name=array_element_name, + parameter_version=parameter_version, + model_version=model_version, + ) + + # Dispatch to appropriate export handler based on parameter type + if _is_dict_table_value(par_info): + return _export_dict_table_parameter( + db=db, + parameter=parameter, + site=site, + array_element_name=array_element_name, + output_file=output_file, + par_info=par_info, + parameters=parameters, + parameter_version=parameter_version, + model_version=model_version, + ) + + # File-backed parameter + if output_file is not None: + raise ValueError( + "Do not use --output_file when exporting file-backed parameters with " + "--export_model_file. The original database file name is used." + ) + + return _export_file_backed_parameter( + db=db, + parameter=parameter, + site=site, + array_element_name=array_element_name, + par_info=par_info, + parameters=parameters, + export_model_file_as_table=export_model_file_as_table, + parameter_version=parameter_version, + model_version=model_version, + ) diff --git a/src/simtools/io/ascii_handler.py b/src/simtools/io/ascii_handler.py index fdf2db8998..2febf5eb81 100644 --- a/src/simtools/io/ascii_handler.py +++ b/src/simtools/io/ascii_handler.py @@ -267,12 +267,13 @@ def _write_to_json(data, output_file, sort_keys, numpy_types): If True, convert numpy types to native Python types. """ with open(output_file, "w", encoding="utf-8") as file: - json.dump( - data, - file, - indent=4, - sort_keys=sort_keys, - cls=JsonNumpyEncoder if numpy_types else None, + file.write( + json.dumps( + data, + indent=4, + sort_keys=sort_keys, + cls=JsonNumpyEncoder if numpy_types else None, + ) ) file.write("\n") @@ -313,7 +314,13 @@ def _to_builtin(data): class JsonNumpyEncoder(json.JSONEncoder): - """Convert numpy to python types as accepted by json.dump.""" + """Convert numpy to python types as accepted by json.dump. + + Lists whose elements are all numbers (int or float) are serialized on a + single line regardless of the surrounding indentation level. This keeps + row-oriented table data human-readable without expanding every number onto + its own line. + """ def default(self, o): """Return default encoder.""" @@ -328,3 +335,43 @@ def default(self, o): if np.issubdtype(type(o), np.bool_): return bool(o) return super().default(o) + + def encode(self, o): + """Encode with compact inner numeric lists.""" + # Convert numpy (and other custom) types to pure-Python types first so + # that _encode_compact_rows only needs to handle builtins. + native = json.loads(super().encode(o)) + return _encode_compact_rows(native, indent=4, level=0) + + +def _is_numeric_list(obj): + """Return True if obj is a list whose elements are all int or float.""" + return isinstance(obj, list) and obj and all(isinstance(v, (int, float)) for v in obj) + + +def _encode_compact_rows(obj, indent, level): + if _is_numeric_list(obj): + return "[" + ", ".join(json.dumps(v) for v in obj) + "]" + + if isinstance(obj, (dict, list)): + if not obj: + return "{}" if isinstance(obj, dict) else "[]" + + is_dict = isinstance(obj, dict) + items = obj.items() if is_dict else obj + + pad = " " * indent * (level + 1) + close_pad = " " * indent * level + + def fmt(item): + if is_dict: + k, v = item + return f"{json.dumps(k)}: {_encode_compact_rows(v, indent, level + 1)}" + return _encode_compact_rows(item, indent, level + 1) + + body = (",\n" + pad).join(fmt(i) for i in items) + open_, close_ = ("{", "}") if is_dict else ("[", "]") + + return f"{open_}\n{pad}{body}\n{close_pad}{close_}" + + return json.dumps(obj) diff --git a/src/simtools/model/legacy_model_parameter.py b/src/simtools/model/legacy_model_parameter.py index 58ee1ecf83..d2d9ca8342 100644 --- a/src/simtools/model/legacy_model_parameter.py +++ b/src/simtools/model/legacy_model_parameter.py @@ -11,12 +11,22 @@ import logging +from simtools.data_model import row_table_utils + logger = logging.getLogger(__name__) UPDATE_HANDLERS = {} +def _log_schema_update(parameter_name, from_schema_version, to_schema_version): + """Log schema migration for a legacy model parameter.""" + logger.info( + f"Updating legacy model parameter {parameter_name} from schema version " + f"{from_schema_version} to {to_schema_version}" + ) + + def register_update(name): """Register update handler for legacy model parameter.""" @@ -49,7 +59,7 @@ def apply_legacy_updates_to_parameters(parameters, legacy_updates): parameters.pop(par_name) -def update_parameter(par_name, parameters, schema_version): +def update_parameter(par_name, parameters, schema_version, value_resolver=None): """Update legacy model parameters to recent formats. Parameters @@ -60,6 +70,11 @@ def update_parameter(par_name, parameters, schema_version): Dictionary of model parameters (all parameters). schema_version: str Target schema version. + value_resolver: callable, optional + Callback used by handlers that need to normalize a stored legacy value + before it can be embedded in the updated parameter. The callback must + accept ``(parameter_name, value)`` and return the canonical in-memory + representation for that parameter value. Returns ------- @@ -69,17 +84,47 @@ def update_parameter(par_name, parameters, schema_version): handler = UPDATE_HANDLERS.get(par_name) if handler is None: raise ValueError(_get_unsupported_update_message(parameters[par_name], schema_version)) - return handler(parameters, schema_version) + return handler(parameters, schema_version, value_resolver=value_resolver) + + +def _update_file_backed_table_parameter( + parameter_name, + parameters, + schema_version, + value_resolver=None, +): + """Update a legacy file-backed table parameter to embedded row data. + + The ``value_resolver`` callback is expected to convert the stored legacy + value, typically a file name for a GridFS-backed table, into the canonical + embedded ``{"columns", "rows"}`` representation used in memory. + """ + para_data = parameters[parameter_name] + if value_resolver is None: + raise ValueError( + f"A value_resolver is required to update legacy file-backed parameter {parameter_name}." + ) + + return { + para_data["parameter"]: { + "value": value_resolver(parameter_name, para_data["value"]), + "model_parameter_schema_version": schema_version, + "type": "dict", + "file": False, + } + } @register_update("dsum_threshold") -def _update_dsum_threshold(parameters, schema_version): +def _update_dsum_threshold(parameters, schema_version, value_resolver=None): """Update legacy dsum_threshold parameter.""" + _ = value_resolver para_data = parameters["dsum_threshold"] if para_data["model_parameter_schema_version"] == "0.1.0" and schema_version == "0.2.0": - logger.info( - "Updating legacy model parameter dsum_threshold from schema version " - f"{para_data['model_parameter_schema_version']} to {schema_version}" + _log_schema_update( + para_data["parameter"], + para_data["model_parameter_schema_version"], + schema_version, ) return { para_data["parameter"]: { @@ -91,7 +136,7 @@ def _update_dsum_threshold(parameters, schema_version): @register_update("corsika_starting_grammage") -def _update_corsika_starting_grammage(parameters, schema_version): # pylint: disable=unused-argument +def _update_corsika_starting_grammage(parameters, schema_version, value_resolver=None): # pylint: disable=unused-argument """Update legacy corsika_starting_grammage parameter (dummy function until model is updated).""" return { parameters["corsika_starting_grammage"]["parameter"]: None, @@ -99,13 +144,15 @@ def _update_corsika_starting_grammage(parameters, schema_version): # pylint: di @register_update("flasher_pulse_shape") -def _update_flasher_pulse_shape(parameters, schema_version): +def _update_flasher_pulse_shape(parameters, schema_version, value_resolver=None): """Update legacy flasher_pulse_shape parameter.""" + _ = value_resolver para_data = parameters["flasher_pulse_shape"] if para_data["model_parameter_schema_version"] == "0.1.0" and schema_version == "0.2.0": - logger.info( - f"Updating legacy model parameter flasher_pulse_shape from schema version " - f"{para_data['model_parameter_schema_version']} to {schema_version}" + _log_schema_update( + para_data["parameter"], + para_data["model_parameter_schema_version"], + schema_version, ) return { para_data["parameter"]: { @@ -125,6 +172,37 @@ def _update_flasher_pulse_shape(parameters, schema_version): raise ValueError(_get_unsupported_update_message(para_data, schema_version)) +@register_update("fadc_pulse_shape") +def _update_fadc_pulse_shape(parameters, schema_version, value_resolver=None): + """Update legacy fadc_pulse_shape parameter.""" + para_data = parameters["fadc_pulse_shape"] + current_schema_version = para_data["model_parameter_schema_version"] + value = para_data.get("value") + parameter_name = para_data["parameter"] + + # Generic migration for legacy file-backed payloads. + if para_data.get("file") and isinstance(value, str): + _log_schema_update(parameter_name, current_schema_version, schema_version) + return _update_file_backed_table_parameter( + "fadc_pulse_shape", + parameters, + schema_version, + value_resolver=value_resolver, + ) + + # Already in canonical row-oriented format {columns, rows} - pass through. + if para_data.get("type") == "dict" and row_table_utils.is_row_table_dict(value): + _log_schema_update(parameter_name, current_schema_version, schema_version) + return { + para_data["parameter"]: { + "value": value, + "model_parameter_schema_version": schema_version, + } + } + + raise ValueError(_get_unsupported_update_message(para_data, schema_version)) + + def _get_unsupported_update_message(para_data, schema_version): """Get unsupported update message.""" return ( diff --git a/src/simtools/model/model_parameter.py b/src/simtools/model/model_parameter.py index 9c6c985d83..a132b8eda3 100644 --- a/src/simtools/model/model_parameter.py +++ b/src/simtools/model/model_parameter.py @@ -5,6 +5,7 @@ import shutil from copy import copy, deepcopy from pathlib import Path +from tempfile import TemporaryDirectory import astropy.units as u @@ -14,6 +15,7 @@ from simtools.db import db_handler from simtools.io import io_handler from simtools.model import legacy_model_parameter +from simtools.simtel import simtel_table_reader from simtools.simtel.simtel_config_writer import SimtelConfigWriter from simtools.utils import names, value_conversion @@ -326,7 +328,11 @@ def _load_parameters_from_db(self): ) ) self.overwrite_parameters(self.overwrite_model_parameter_dict) - self._check_model_parameter_versions(self.parameters, self.ignore_software_version) + self._check_model_parameter_versions( + self.parameters, + self.ignore_software_version, + value_resolver=self._resolve_legacy_table_parameter_value, + ) self._load_simulation_software_parameter() for software_name, parameters in self._simulation_config_parameters.items(): @@ -334,10 +340,34 @@ def _load_parameters_from_db(self): parameters, ignore_software_version=self.ignore_software_version, software_name=software_name, + value_resolver=self._resolve_legacy_table_parameter_value, + ) + + def _resolve_legacy_table_parameter_value(self, parameter_name, value): + """Resolve a legacy stored table value to canonical row-oriented data. + + This method is passed into ``legacy_model_parameter.update_parameter`` + as ``value_resolver``. Legacy handlers use it when an old parameter + stores a table indirectly, e.g. as a GridFS-backed file name, and needs + to be normalized to the current in-memory ``{"columns", "rows"}`` + representation. + """ + with TemporaryDirectory() as temp_dir: + temp_path = Path(temp_dir) + self.db.export_model_files(file_names=[value], dest=temp_path) + return simtel_table_reader.resolve_dict_parameter_value( + value, + parameter_name, + data_path=temp_path, ) @staticmethod - def _check_model_parameter_versions(parameters, ignore_software_version, software_name=None): + def _check_model_parameter_versions( + parameters, + ignore_software_version, + software_name=None, + value_resolver=None, + ): """ Ensure parameters follow the latest schema and are compatible with installed software. @@ -355,6 +385,11 @@ def _check_model_parameter_versions(parameters, ignore_software_version, softwar If True, ignore software version checks for deprecated parameters. software_name: str Name of the software for which the parameters are checked. + value_resolver: callable + Optional callback used by legacy updates to normalize parameter + values from older storage formats to the latest in-memory format. + It must accept ``(parameter_name, value)`` and return the + normalized value. """ _legacy_updates = {} for par_name, par_data in parameters.items(): @@ -368,7 +403,10 @@ def _check_model_parameter_versions(parameters, ignore_software_version, softwar if par_data["model_parameter_schema_version"] != _latest_schema_version: _legacy_updates.update( legacy_model_parameter.update_parameter( - par_name, parameters, _latest_schema_version + par_name, + parameters, + _latest_schema_version, + value_resolver=value_resolver, ) ) diff --git a/src/simtools/schemas/model_parameters/fadc_pulse_shape.schema.yml b/src/simtools/schemas/model_parameters/fadc_pulse_shape.schema.yml index 998b20b873..daf2d3c1c5 100644 --- a/src/simtools/schemas/model_parameters/fadc_pulse_shape.schema.yml +++ b/src/simtools/schemas/model_parameters/fadc_pulse_shape.schema.yml @@ -1,6 +1,109 @@ %YAML 1.2 --- title: Schema for fadc_pulse_shape model parameter +schema_version: 0.2.0 +meta_schema: simpipe-schema +meta_schema_url: https://raw.githubusercontent.com/gammasim/simtools/main/src/simtools/schemas/model_parameter_and_data_schema.metaschema.yml +meta_schema_version: 0.1.0 +name: fadc_pulse_shape +description: |- + (F)ADC pulse shape (amplitude vs time) for low and high gain + readout chain. The pulse amplitude scale is ignored and the pulses are + re-scaled to peak values of par:fadc-amplitude times par:fadc-sensitivity. +short_description: (F)ADC pulse shape (amplitude vs time). +data: + - type: dict + json_schema: + type: object + additionalProperties: false + required: + - columns + - column_units + - rows + properties: + columns: + type: array + oneOf: + - type: array + items: + - const: time + - const: amplitude + additionalItems: false + minItems: 2 + maxItems: 2 + - type: array + items: + - const: time + - const: amplitude + - const: "amplitude (low gain)" + additionalItems: false + minItems: 3 + maxItems: 3 + column_units: + type: array + oneOf: + - type: array + items: + - const: ns + - const: dimensionless + additionalItems: false + minItems: 2 + maxItems: 2 + - type: array + items: + - const: ns + - const: dimensionless + - const: dimensionless + additionalItems: false + minItems: 3 + maxItems: 3 + rows: + type: array + items: + oneOf: + - type: array + items: + - type: number + - type: number + additionalItems: false + minItems: 2 + maxItems: 2 + - type: array + items: + - type: number + - type: number + - type: number + additionalItems: false + minItems: 3 + maxItems: 3 +instrument: + class: Camera +activity: + setting: + - SetReadoutPulseShape + - SetParameterFromExternal + validation: + - ValidateParameterByExpert + - ValidateCameraChargeResponse + - ValidateCameraTimeResponse +source: + - Calibration +simulation_software: + - name: sim_telarray +plot_configuration: + - type: fadc_pulse_shape + title: 'FADC pulse shape' + xtitle: 'Time [ns]' + ytitle: 'Amplitude [a.u.]' + xscale: 'linear' + yscale: 'linear' + no_markers: true + tables: + - parameter: fadc_pulse_shape + column_x: 'time' + column_y: 'amplitude' +--- +title: Schema for fadc_pulse_shape model parameter schema_version: 0.1.0 meta_schema: simpipe-schema meta_schema_url: https://raw.githubusercontent.com/gammasim/simtools/main/src/simtools/schemas/model_parameter_and_data_schema.metaschema.yml diff --git a/src/simtools/simtel/simtel_config_writer.py b/src/simtools/simtel/simtel_config_writer.py index 8701ace634..36c1736361 100644 --- a/src/simtools/simtel/simtel_config_writer.py +++ b/src/simtools/simtel/simtel_config_writer.py @@ -11,7 +11,7 @@ import simtools.utils.general as gen import simtools.version from simtools import dependencies, settings -from simtools.simtel.pulse_shapes import generate_pulse_from_rise_fall_times +from simtools.simtel import simtel_table_writer from simtools.utils import names logger = logging.getLogger(__name__) @@ -98,128 +98,6 @@ def write_telescope_config_file( ): file.write(f"{meta}\n") - @staticmethod - def write_light_pulse_table_gauss_exp_conv( - file_path, - width_ns, - exp_decay_ns, - fadc_sum_bins, - dt_ns=0.1, - rise_range=(0.1, 0.9), - fall_range=(0.9, 0.1), - time_margin_ns=10.0, - ): - """Write a pulse table for a Gaussian convolved with a causal exponential. - - Parameters - ---------- - file_path : str or pathlib.Path - Destination path of the ASCII pulse table to write. Parent directory must exist. - width_ns : float - Target rise time in ns between the fractional levels defined by ``rise_range``. - exp_decay_ns : float - Target fall time in ns between the fractional levels defined by ``fall_range``. - fadc_sum_bins : int - Length of the FADC integration window (treated as ns here) used to derive - the internal time sampling window of the solver as [-(margin), bins + margin]. - dt_ns : float, optional - Time sampling step in ns for the generated pulse table. - rise_range : tuple[float, float], optional - Fractional amplitude bounds (low, high) for rise-time definition. - fall_range : tuple[float, float], optional - Fractional amplitude bounds (high, low) for fall-time definition. - time_margin_ns : float, optional - Margin in ns to add to both ends of the FADC window when ``fadc_sum_bins`` is given. - - Returns - ------- - pathlib.Path - The path to the created pulse table file. - - Notes - ----- - The underlying model is a Gaussian convolved with a causal exponential. The model - parameters (sigma, tau) are solved such that the normalized pulse matches the requested - rise and fall times. The pulse is normalized to a peak amplitude of 1. - """ - if width_ns is None or exp_decay_ns is None: - raise ValueError("width_ns (rise 10-90) and exp_decay_ns (fall 90-10) are required") - logger.info( - "Generating pulse-shape table with " - f"rise{int(rise_range[0] * 100)}-{int(rise_range[1] * 100)}={width_ns} ns, " - f"fall{int(fall_range[0] * 100)}-{int(fall_range[1] * 100)}={exp_decay_ns} ns, " - f"dt={dt_ns} ns" - ) - width = float(fadc_sum_bins) - t_start_ns = -abs(time_margin_ns + width) - t_stop_ns = +abs(time_margin_ns + width) - t, y = generate_pulse_from_rise_fall_times( - width_ns, - exp_decay_ns, - dt_ns=dt_ns, - rise_range=rise_range, - fall_range=fall_range, - t_start_ns=t_start_ns, - t_stop_ns=t_stop_ns, - center_on_peak=True, - ) - - return SimtelConfigWriter._write_ascii_pulse_table(file_path, t, y) - - @staticmethod - def write_angular_distribution_table_lambertian( - file_path, - max_angle_deg, - n_samples=100, - ): - """Write a Lambertian angular distribution table (I(t) ~ cos(t)). - - Parameters - ---------- - file_path : str or pathlib.Path - Destination path of the ASCII table to write. Parent directory must exist. - max_angle_deg : float - Maximum angle (deg) for the distribution sampling range [0, max_angle_deg]. - n_samples : int, optional - Number of samples (including end point) from 0 to max_angle_deg. Default 100. - - Returns - ------- - pathlib.Path - Path to created angular distribution table. - """ - logger.info( - f"Generating Lambertian angular distribution table up to {max_angle_deg} deg " - f"with {n_samples} samples" - ) - angles = np.linspace(0.0, float(max_angle_deg), int(n_samples), dtype=float) - intensities = np.cos(np.deg2rad(angles)) - intensities[intensities < 0] = 0.0 - if intensities.max() > 0: - intensities /= intensities.max() - - return SimtelConfigWriter._write_ascii_angle_distribution_table( - file_path, angles, intensities - ) - - @staticmethod - def _write_ascii_pulse_table(file_path, t, y): - """Write two-column ASCII pulse table.""" - with open(file_path, "w", encoding="utf-8") as fh: - fh.write("# time[ns] amplitude\n") - for ti, yi in zip(t, y): - fh.write(f"{ti:.6f} {yi:.8f}\n") - return Path(file_path) - - @staticmethod - def _write_ascii_angle_distribution_table(file_path, angles, intensities): - """Write two-column ASCII angular distribution table.""" - with open(file_path, "w", encoding="utf-8") as fh: - fh.write("# angle[deg] relative_intensity\n") - for a, i in zip(angles, intensities): - fh.write(f"{a:.6f} {i:.8f}\n") - return Path(file_path) - def _get_parameters_for_sim_telarray(self, parameters, config_file_path): """ Convert parameter dictionary to sim_telarray configuration file format. @@ -639,6 +517,9 @@ def _convert_model_parameters_to_simtel_format( """ conversion_dict = { "array_triggers": self._write_array_triggers_file, + "fadc_pulse_shape": lambda v, mp, tm: self._write_table_parameter_file( + "fadc_pulse_shape", v, mp, tm + ), } try: value = conversion_dict[simtel_name](value, model_path, telescope_model) @@ -648,6 +529,34 @@ def _convert_model_parameters_to_simtel_format( return None, None return simtel_name, value + def _write_table_parameter_file(self, parameter_name, value, model_path, _telescope_model): + """ + Write a dict-valued table parameter to an ASCII file for sim_telarray. + + Parameters + ---------- + parameter_name : str + Parameter name. + value : dict or str + Table data as ``{columns, rows}`` dict, or a filename string (passed through). + model_path : Path + Path to the telescope config file being written. + _telescope_model : ignored + Unused; present to match the conversion callback signature. + + Returns + ------- + str + Basename of the written file, or the original string value unchanged. + """ + if not isinstance(value, dict): + return value + dest_dir = Path(model_path).parent + telescope_name = Path(model_path).stem + return simtel_table_writer.write_simtel_table( + parameter_name, value, dest_dir, telescope_name + ) + def _write_array_triggers_file(self, array_triggers, model_path, telescope_model): """ Write array trigger definition file in simtel format. diff --git a/src/simtools/simtel/simtel_output_validator.py b/src/simtools/simtel/simtel_output_validator.py index 12491be5a7..f181a549e9 100644 --- a/src/simtools/simtel/simtel_output_validator.py +++ b/src/simtools/simtel/simtel_output_validator.py @@ -8,6 +8,7 @@ from simtools.sim_events import file_info from simtools.sim_events.file_info import get_corsika_run_number +from simtools.simtel import simtel_table_reader from simtools.simtel.simtel_config_reader import SimtelConfigReader from simtools.simtel.simtel_io_metadata import ( get_sim_telarray_telescope_id, @@ -260,6 +261,9 @@ def _assert_model_parameters(metadata, model, allow_for_changes=None): parameter_type = model.parameters[param]["type"] value = _extract_parameter_value(metadata, sim_telarray_name, parameter_type) model_value = model.parameters[param]["value"] + value = _resolve_dict_parameter_metadata_value( + value, model_value, parameter_type, param, model + ) error = _check_parameter_validity( param, value, model_value, parameter_type, allow_for_changes @@ -270,6 +274,25 @@ def _assert_model_parameters(metadata, model, allow_for_changes=None): return invalid_parameter_list +def _resolve_dict_parameter_metadata_value(value, model_value, parameter_type, param, model): + """Resolve table-file metadata for dict parameters before comparison.""" + if parameter_type != "dict": + return value + + if not isinstance(value, str) or not isinstance(model_value, dict): + return value + + try: + return simtel_table_reader.resolve_dict_parameter_value( + value, + param, + data_path=model.config_file_directory, + ) + except (FileNotFoundError, ValueError, TypeError) as exc: + _logger.debug(f"Unable to resolve dict-valued sim_telarray metadata for {param}: {exc}") + return value + + def _assert_sim_telarray_seed(metadata, sim_telarray_seed, file=None): """ Assert that sim_telarray seed matches the values in the sim_telarray metadata. diff --git a/src/simtools/simtel/simtel_table_reader.py b/src/simtools/simtel/simtel_table_reader.py index f794ae82c8..d82ca82ac8 100644 --- a/src/simtools/simtel/simtel_table_reader.py +++ b/src/simtools/simtel/simtel_table_reader.py @@ -1,6 +1,7 @@ #!/usr/bin/python3 """Read tabular data in sim_telarray format and return as astropy table.""" +import json import logging import re from pathlib import Path @@ -9,6 +10,7 @@ import numpy as np from astropy.table import Table +from simtools.data_model import row_table_utils from simtools.io import ascii_handler logger = logging.getLogger(__name__) @@ -294,6 +296,125 @@ def read_simtel_table(parameter_name, file_path): return table +def read_simtel_table_as_row_data(parameter_name, file_path): + """Read sim_telarray table file and serialize it as row-oriented data.""" + table = read_simtel_table(parameter_name, file_path) + + columns = list(table.colnames) + column_units = [row_table_utils.normalize_column_unit(table[col].unit) for col in columns] + rows = [list(row) for row in table.as_array().tolist()] + + return { + "columns": columns, + "column_units": column_units, + "rows": rows, + } + + +def row_data_to_astropy_table(row_data): + """ + Convert a row-oriented parameter value dict to an astropy Table. + + Accepts dicts in the ``{columns, rows}`` format produced by + :func:`read_simtel_table_as_row_data` and stored as embedded ``dict``-typed + model parameter values in the database. + + Parameters + ---------- + row_data : dict + Dictionary with keys ``"columns"`` (list of str) and ``"rows"`` + (list of lists of numbers). + + Returns + ------- + astropy.table.Table + Table with one column per entry in ``row_data["columns"]``. + + Raises + ------ + ValueError + If ``row_data`` does not contain the expected ``"columns"`` and + ``"rows"`` keys. + """ + try: + columns = row_data["columns"] + rows = row_data["rows"] + except (KeyError, TypeError) as exc: + raise ValueError("row_data must be a dict with 'columns' and 'rows' keys.") from exc + + table = Table(rows=rows, names=columns) + column_units = row_data.get("column_units") + if column_units is not None: + if len(column_units) != len(columns): + raise ValueError("row_data 'column_units' length must match the number of 'columns'.") + for col_name, unit_name in zip(columns, column_units): + table[col_name].unit = ( + u.dimensionless_unscaled if unit_name == "dimensionless" else unit_name + ) + return table + + +def _resolve_input_file_path(file_name, data_path=None): + """Resolve an input file using data_path for relative paths.""" + file_name = Path(file_name) + if file_name.is_absolute(): + return file_name + + return Path(data_path) / file_name if data_path else file_name + + +def _parse_inline_json_dict(value, parameter_name): + """Parse inline JSON string value and return dict when valid.""" + stripped_value = value.strip() + if not stripped_value.startswith("{"): + return None + + try: + parsed_value = json.loads(stripped_value) + except json.JSONDecodeError: + logger.debug( + f"Value for '{parameter_name}' starts with '{{' but is not valid JSON; " + "falling back to file-path reading." + ) + return None + + return parsed_value if isinstance(parsed_value, dict) else None + + +def _resolve_dict_parameter_from_string(value, parameter_name, data_path=None): + """Resolve dict-typed value from inline JSON or from table file path string.""" + parsed_value = _parse_inline_json_dict(value, parameter_name) + if parsed_value is not None: + # Validate that row-table-like dicts (with columns and rows) also have column_units + if "columns" in parsed_value and "rows" in parsed_value: + if "column_units" not in parsed_value: + raise ValueError( + "row_data must contain 'column_units' when using 'columns' and 'rows'." + ) + return parsed_value + return read_simtel_table_as_row_data( + parameter_name, + _resolve_input_file_path(value, data_path), + ) + + +def resolve_dict_parameter_value(value, parameter_name, data_path=None): + """Resolve dict-typed value from inline JSON or from a table file path.""" + if isinstance(value, dict): + # Validate that row-table-like dicts (with columns and rows) also have column_units + if "columns" in value and "rows" in value: + if "column_units" not in value: + raise ValueError( + "row_data must contain 'column_units' when using 'columns' and 'rows'." + ) + return value + + if isinstance(value, str): + return _resolve_dict_parameter_from_string(value, parameter_name, data_path=data_path) + + return read_simtel_table_as_row_data(parameter_name, _resolve_input_file_path(value, data_path)) + + def _adjust_columns_length(rows, n_columns): """ Adjust row lengths to match the specified column count. diff --git a/src/simtools/simtel/simtel_table_writer.py b/src/simtools/simtel/simtel_table_writer.py new file mode 100644 index 0000000000..ce48ce4d78 --- /dev/null +++ b/src/simtools/simtel/simtel_table_writer.py @@ -0,0 +1,203 @@ +"""Writer for sim_telarray table data files.""" + +import logging +from pathlib import Path + +import numpy as np + +from simtools.data_model import row_table_utils +from simtools.simtel.pulse_shapes import generate_pulse_from_rise_fall_times + +logger = logging.getLogger(__name__) + + +def write_simtel_table(parameter_name, value, dest_dir, telescope_name): + """Write a table parameter to a space-separated ASCII file for sim_telarray. + + Parameters + ---------- + parameter_name : str + Parameter name, used as filename prefix. + value : dict + Table data with keys ``columns`` (list of str) and ``rows`` (list of lists). + dest_dir : str or Path + Directory to write the file into. + telescope_name : str + Telescope name, used as filename suffix. + + Returns + ------- + str + Basename of the written file (``{parameter_name}-{telescope_name}.dat``). + + Raises + ------ + ValueError + If ``value`` does not contain ``columns`` and ``rows`` keys. + """ + if not isinstance(value, dict) or "columns" not in value or "rows" not in value: + raise ValueError( + f"Table value for '{parameter_name}' must be a dict with 'columns' and 'rows' keys, " + f"got {type(value).__name__}." + ) + + row_table_utils.validate_row_table_structure(parameter_name, value, require_column_units=False) + + file_name = f"{parameter_name}-{telescope_name}.dat" + file_path = Path(dest_dir) / file_name + logger.debug(f"Writing sim_telarray table file {file_path}") + + with open(file_path, "w", encoding="utf-8") as fh: + fh.write(f"# {' '.join(value['columns'])}\n") + for row in value["rows"]: + fh.write(" ".join(str(v) for v in row) + "\n") + + return file_name + + +def write_light_pulse_table_gauss_exp_conv( + file_path, + width_ns, + exp_decay_ns, + fadc_sum_bins, + dt_ns=0.1, + rise_range=(0.1, 0.9), + fall_range=(0.9, 0.1), + time_margin_ns=10.0, +): + """Write a pulse table for a Gaussian convolved with a causal exponential. + + Parameters + ---------- + file_path : str or Path + Destination path of the ASCII pulse table. Parent directory must exist. + width_ns : float + Rise time in ns between the fractional levels defined by ``rise_range``. + exp_decay_ns : float + Fall time in ns between the fractional levels defined by ``fall_range``. + fadc_sum_bins : int + FADC integration window length in bins, used to set the time range. + dt_ns : float, optional + Time sampling step in ns. + rise_range : tuple[float, float], optional + Fractional amplitude bounds (low, high) for rise-time definition. + fall_range : tuple[float, float], optional + Fractional amplitude bounds (high, low) for fall-time definition. + time_margin_ns : float, optional + Extra margin in ns added to both ends of the time window. + + Returns + ------- + Path + Path to the created pulse table file. + + Raises + ------ + ValueError + If ``width_ns`` or ``exp_decay_ns`` is None. + """ + if width_ns is None or exp_decay_ns is None: + raise ValueError("width_ns (rise 10-90) and exp_decay_ns (fall 90-10) are required") + logger.info( + "Generating pulse-shape table with " + f"rise{int(rise_range[0] * 100)}-{int(rise_range[1] * 100)}={width_ns} ns, " + f"fall{int(fall_range[0] * 100)}-{int(fall_range[1] * 100)}={exp_decay_ns} ns, " + f"dt={dt_ns} ns" + ) + width = float(fadc_sum_bins) + t_start_ns = -abs(time_margin_ns + width) + t_stop_ns = +abs(time_margin_ns + width) + t, y = generate_pulse_from_rise_fall_times( + width_ns, + exp_decay_ns, + dt_ns=dt_ns, + rise_range=rise_range, + fall_range=fall_range, + t_start_ns=t_start_ns, + t_stop_ns=t_stop_ns, + center_on_peak=True, + ) + + return write_ascii_pulse_table(file_path, t, y) + + +def write_angular_distribution_table_lambertian( + file_path, + max_angle_deg, + n_samples=100, +): + """Write a Lambertian angular distribution table (intensity ~ cos(angle)). + + Parameters + ---------- + file_path : str or Path + Destination path of the ASCII table. Parent directory must exist. + max_angle_deg : float + Upper bound of the angular range in degrees. + n_samples : int, optional + Number of equally spaced samples from 0 to ``max_angle_deg``. + + Returns + ------- + Path + Path to the created angular distribution table. + """ + logger.info( + f"Generating Lambertian angular distribution table up to {max_angle_deg} deg " + f"with {n_samples} samples" + ) + angles = np.linspace(0.0, float(max_angle_deg), int(n_samples), dtype=float) + intensities = np.cos(np.deg2rad(angles)) + intensities[intensities < 0] = 0.0 + if intensities.max() > 0: + intensities /= intensities.max() + + return write_ascii_angle_distribution_table(file_path, angles, intensities) + + +def write_ascii_pulse_table(file_path, t, y): + """Write a two-column (time, amplitude) ASCII pulse table. + + Parameters + ---------- + file_path : str or Path + Destination path. + t : array-like + Time values in ns. + y : array-like + Amplitude values. + + Returns + ------- + Path + Path to the written file. + """ + with open(file_path, "w", encoding="utf-8") as fh: + fh.write("# time[ns] amplitude\n") + for ti, yi in zip(t, y): + fh.write(f"{ti:.6f} {yi:.8f}\n") + return Path(file_path) + + +def write_ascii_angle_distribution_table(file_path, angles, intensities): + """Write a two-column (angle, relative intensity) ASCII angular distribution table. + + Parameters + ---------- + file_path : str or Path + Destination path. + angles : array-like + Angle values in degrees. + intensities : array-like + Relative intensity values. + + Returns + ------- + Path + Path to the written file. + """ + with open(file_path, "w", encoding="utf-8") as fh: + fh.write("# angle[deg] relative_intensity\n") + for a, i in zip(angles, intensities): + fh.write(f"{a:.6f} {i:.8f}\n") + return Path(file_path) diff --git a/src/simtools/simtel/simulator_light_emission.py b/src/simtools/simtel/simulator_light_emission.py index b498501e46..1dadef76e3 100644 --- a/src/simtools/simtel/simulator_light_emission.py +++ b/src/simtools/simtel/simulator_light_emission.py @@ -13,8 +13,7 @@ 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 import simtel_output_validator -from simtools.simtel.simtel_config_writer import SimtelConfigWriter +from simtools.simtel import simtel_output_validator, simtel_table_writer from simtools.utils import general from simtools.utils.geometry import fiducial_radius_from_shape @@ -604,7 +603,7 @@ def calculate_distance_focal_plane_calibration_device(self): return focal_length - flasher_z def _generate_lambertian_angular_distribution_table(self): - """Generate Lambertian angular distribution table via config writer and return path. + """Generate Lambertian angular distribution table and return path. Uses a pure cosine profile normalized to 1 at 0 deg and spans 0..max_angle_deg. """ @@ -618,7 +617,7 @@ def _generate_lambertian_angular_distribution_table(self): .to(u.deg) .value ) - path = SimtelConfigWriter.write_angular_distribution_table_lambertian( + path = simtel_table_writer.write_angular_distribution_table_lambertian( file_path=self.io_handler.get_output_directory("light_emission") / fname, max_angle_deg=max_angle_deg, n_samples=100, @@ -687,7 +686,7 @@ def _get_pulse_shape_argument_for_sim_telarray(self): 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( + simtel_table_writer.write_light_pulse_table_gauss_exp_conv( file_path=table_path, width_ns=width_ns, exp_decay_ns=exp_ns, @@ -696,9 +695,8 @@ def _get_pulse_shape_argument_for_sim_telarray(self): ) return str(table_path) except (ValueError, OSError) as err: - raise ValueError( - f"Failed to write Gauss-Exponential pulse shape table: {err}" - ) from err + self._logger.warning(f"Failed to write pulse shape table, using token: {err}") + return self._get_pulse_shape_string_token(shape_name, width_ns, exp_ns) # For other shapes, return token string return self._get_pulse_shape_string_token(shape_name, width_ns, exp_ns) diff --git a/src/simtools/visualization/plot_tables.py b/src/simtools/visualization/plot_tables.py index 0fce72c898..894413bb9e 100644 --- a/src/simtools/visualization/plot_tables.py +++ b/src/simtools/visualization/plot_tables.py @@ -5,6 +5,7 @@ from pathlib import Path import numpy as np +import packaging.version import simtools.utils.general as gen from simtools.constants import SCHEMA_PATH @@ -139,6 +140,19 @@ def _read_table_from_model_database(table_config): ) +def _read_parameter_dict_from_model_database(table_config): + """Read a model parameter dictionary from the model parameter database.""" + db = db_handler.DatabaseHandler() + parameter_dict = db.get_model_parameter( + parameter=table_config["parameter"], + site=table_config["site"], + array_element_name=table_config.get("telescope"), + parameter_version=table_config.get("parameter_version"), + model_version=table_config.get("model_version"), + ) + return parameter_dict[table_config["parameter"]] + + def _select_values_from_table(table, column_name, value): """Return a table with only the rows where column_name == value.""" return table[np.isclose(table[column_name], value)] @@ -170,6 +184,25 @@ def _get_valid_columns(table): return [col for col in table.colnames if not all(np.isnan(table[col]))] +def _select_schema_entry(schema_data, schema_version=None): + """Return the schema dict matching schema_version or the newest available one.""" + if isinstance(schema_data, dict): + return schema_data + + if isinstance(schema_data, list) and schema_data: + if schema_version is not None: + for entry in schema_data: + if entry.get("schema_version") == schema_version: + return entry + + return max( + schema_data, + key=lambda entry: packaging.version.Version(entry.get("schema_version", "0.0.0")), + ) + + return {} + + def generate_plot_configurations( parameter, parameter_version, site, telescope, output_path, plot_type ): @@ -198,11 +231,21 @@ def generate_plot_configurations( Return None, if no plot configurations are found. """ logger = logging.getLogger(__name__) + table_config = { + "parameter": parameter, + "site": site, + "telescope": telescope, + "parameter_version": parameter_version, + } + parameter_dict = _read_parameter_dict_from_model_database(table_config) # Get schema configuration schema = gen.change_dict_keys_case( - ascii_handler.collect_data_from_file( - file_name=SCHEMA_PATH / "model_parameters" / f"{parameter}.schema.yml" + _select_schema_entry( + ascii_handler.collect_data_from_file( + file_name=SCHEMA_PATH / "model_parameters" / f"{parameter}.schema.yml" + ), + parameter_dict.get("model_parameter_schema_version"), ) ) configs = schema.get("plot_configuration") @@ -210,14 +253,7 @@ def generate_plot_configurations( return None # Get data table and determine valid columns - table = _read_table_from_model_database( - { - "parameter": parameter, - "site": site, - "telescope": telescope, - "parameter_version": parameter_version, - }, - ) + table = _read_table_from_model_database(table_config) valid_columns = _get_valid_columns(table) # Filter configs based on plot type and column validity diff --git a/tests/resources/sim_telarray_configurations/5.0.0/CTA-North-LSTN-01_test.cfg b/tests/resources/sim_telarray_configurations/5.0.0/CTA-North-LSTN-01_test.cfg index 3ea65b536f..3602ca100d 100644 --- a/tests/resources/sim_telarray_configurations/5.0.0/CTA-North-LSTN-01_test.cfg +++ b/tests/resources/sim_telarray_configurations/5.0.0/CTA-North-LSTN-01_test.cfg @@ -69,7 +69,7 @@ fadc_max_sum = 16777215 fadc_mhz = 1024.0 fadc_noise = 6.7 fadc_pedestal = 400.0 -fadc_pulse_shape = pulse_LST_8dynode_pix6_20200204.dat +fadc_pulse_shape = fadc_pulse_shape-CTA-North-LSTN-01_test.dat fadc_sensitivity = 1.0 fadc_sum_bins = 40 fadc_sum_offset = 9 @@ -135,7 +135,7 @@ save_pe_with_amplitude = 1 store_photoelectrons = 20 tailcut_scale = 2.6 stars = none -config_release = 5.0.0 with simtools v0.24.1.dev97+g5009a391a.d20251103 +config_release = 5.0.0 with simtools v0.27.2.dev544+g80ff08fab.d20260330 config_version = 5.0.0 camera_config_name = LSTN-design camera_config_variant = LSTN-01 diff --git a/tests/resources/sim_telarray_configurations/5.0.0/CTA-North-LSTN-02_test.cfg b/tests/resources/sim_telarray_configurations/5.0.0/CTA-North-LSTN-02_test.cfg index b7fdaa2622..fb7856ea27 100644 --- a/tests/resources/sim_telarray_configurations/5.0.0/CTA-North-LSTN-02_test.cfg +++ b/tests/resources/sim_telarray_configurations/5.0.0/CTA-North-LSTN-02_test.cfg @@ -69,7 +69,7 @@ fadc_max_sum = 16777215 fadc_mhz = 1024.0 fadc_noise = 6.7 fadc_pedestal = 400.0 -fadc_pulse_shape = pulse_LST_8dynode_pix6_20200204.dat +fadc_pulse_shape = fadc_pulse_shape-CTA-North-LSTN-02_test.dat fadc_sensitivity = 1.0 fadc_sum_bins = 40 fadc_sum_offset = 9 @@ -135,7 +135,7 @@ save_pe_with_amplitude = 1 store_photoelectrons = 20 tailcut_scale = 2.6 stars = none -config_release = 5.0.0 with simtools v0.24.1.dev97+g5009a391a.d20251103 +config_release = 5.0.0 with simtools v0.27.2.dev544+g80ff08fab.d20260330 config_version = 5.0.0 camera_config_name = LSTN-design camera_config_variant = LSTN-02 diff --git a/tests/resources/sim_telarray_configurations/5.0.0/CTA-North-MSTN-01_test.cfg b/tests/resources/sim_telarray_configurations/5.0.0/CTA-North-MSTN-01_test.cfg index 3354e09865..cb0dc474fa 100644 --- a/tests/resources/sim_telarray_configurations/5.0.0/CTA-North-MSTN-01_test.cfg +++ b/tests/resources/sim_telarray_configurations/5.0.0/CTA-North-MSTN-01_test.cfg @@ -82,7 +82,7 @@ fadc_max_sum = 16777215 fadc_mhz = 1000.0 fadc_noise = 3.6 fadc_pedestal = 250.0 -fadc_pulse_shape = Pulse_template_nectarCam_17042020-noshift.dat +fadc_pulse_shape = fadc_pulse_shape-CTA-North-MSTN-01_test.dat fadc_sensitivity = 1.0 fadc_sum_bins = 60 fadc_sum_offset = 18 @@ -148,7 +148,7 @@ save_pe_with_amplitude = 1 store_photoelectrons = 26 tailcut_scale = 2.3 stars = none -config_release = 5.0.0 with simtools v0.24.1.dev97+g5009a391a.d20251103 +config_release = 5.0.0 with simtools v0.27.2.dev544+g80ff08fab.d20260330 config_version = 5.0.0 camera_config_name = MSTx-NectarCam camera_config_variant = MSTN-01 diff --git a/tests/resources/sim_telarray_configurations/5.0.0/CTA-South-LSTS-01_test.cfg b/tests/resources/sim_telarray_configurations/5.0.0/CTA-South-LSTS-01_test.cfg index e6957c46f3..dfee2929b9 100644 --- a/tests/resources/sim_telarray_configurations/5.0.0/CTA-South-LSTS-01_test.cfg +++ b/tests/resources/sim_telarray_configurations/5.0.0/CTA-South-LSTS-01_test.cfg @@ -69,7 +69,7 @@ fadc_max_sum = 16777215 fadc_mhz = 1024.0 fadc_noise = 6.7 fadc_pedestal = 400.0 -fadc_pulse_shape = pulse_LST_8dynode_pix6_20200204.dat +fadc_pulse_shape = fadc_pulse_shape-CTA-South-LSTS-01_test.dat fadc_sensitivity = 1.0 fadc_sum_bins = 40 fadc_sum_offset = 9 @@ -135,7 +135,7 @@ save_pe_with_amplitude = 1 store_photoelectrons = 20 tailcut_scale = 2.6 stars = none -config_release = 5.0.0 with simtools v0.24.1.dev97+g5009a391a.d20251103 +config_release = 5.0.0 with simtools v0.27.2.dev544+g80ff08fab.d20260330 config_version = 5.0.0 camera_config_name = LSTS-design camera_config_variant = LSTS-01 diff --git a/tests/resources/sim_telarray_configurations/5.0.0/CTA-South-MSTS-01_test.cfg b/tests/resources/sim_telarray_configurations/5.0.0/CTA-South-MSTS-01_test.cfg index 18776cf32e..25af1bfa39 100644 --- a/tests/resources/sim_telarray_configurations/5.0.0/CTA-South-MSTS-01_test.cfg +++ b/tests/resources/sim_telarray_configurations/5.0.0/CTA-South-MSTS-01_test.cfg @@ -82,7 +82,7 @@ fadc_max_sum = 16777215 fadc_mhz = 250.0 fadc_noise = 1.0 fadc_pedestal = 200.0 -fadc_pulse_shape = pulse_FlashCam_7dynode_v2a.dat +fadc_pulse_shape = fadc_pulse_shape-CTA-South-MSTS-01_test.dat fadc_sensitivity = 1.0 fadc_sum_bins = 24 fadc_sum_offset = 6 @@ -148,7 +148,7 @@ save_pe_with_amplitude = 1 store_photoelectrons = 22 tailcut_scale = 2.6 stars = none -config_release = 5.0.0 with simtools v0.24.1.dev97+g5009a391a.d20251103 +config_release = 5.0.0 with simtools v0.27.2.dev544+g80ff08fab.d20260330 config_version = 5.0.0 camera_config_name = MSTx-FlashCam camera_config_variant = MSTS-01 diff --git a/tests/resources/sim_telarray_configurations/5.0.0/CTA-South-SSTS-01_test.cfg b/tests/resources/sim_telarray_configurations/5.0.0/CTA-South-SSTS-01_test.cfg index a602b2b782..b400d1dbf9 100644 --- a/tests/resources/sim_telarray_configurations/5.0.0/CTA-South-SSTS-01_test.cfg +++ b/tests/resources/sim_telarray_configurations/5.0.0/CTA-South-SSTS-01_test.cfg @@ -81,7 +81,7 @@ fadc_max_sum = 16777215 fadc_mhz = 1000.0 fadc_noise = 1.4 fadc_pedestal = 500.0 -fadc_pulse_shape = pulse_CHEC-S_FADC_27042018.dat +fadc_pulse_shape = fadc_pulse_shape-CTA-South-SSTS-01_test.dat fadc_sensitivity = 1.0 fadc_sum_bins = 128 fadc_sum_offset = 24 @@ -159,7 +159,7 @@ save_pe_with_amplitude = 1 store_photoelectrons = 10 tailcut_scale = 1 stars = none -config_release = 5.0.0 with simtools v0.24.1.dev97+g5009a391a.d20251103 +config_release = 5.0.0 with simtools v0.27.2.dev544+g80ff08fab.d20260330 config_version = 5.0.0 camera_config_name = SSTS-design camera_config_variant = SSTS-01 diff --git a/tests/resources/sim_telarray_configurations/6.0.2/CTA-North-LSTN-01_test.cfg b/tests/resources/sim_telarray_configurations/6.0.2/CTA-North-LSTN-01_test.cfg index 12bdd2e62d..141fe3a438 100644 --- a/tests/resources/sim_telarray_configurations/6.0.2/CTA-North-LSTN-01_test.cfg +++ b/tests/resources/sim_telarray_configurations/6.0.2/CTA-North-LSTN-01_test.cfg @@ -69,7 +69,7 @@ fadc_max_sum = 16777215 fadc_mhz = 1024.0 fadc_noise = 6.7 fadc_pedestal = 400.0 -fadc_pulse_shape = pulse_LST_8dynode_pix6_20200204.dat +fadc_pulse_shape = fadc_pulse_shape-CTA-North-LSTN-01_test.dat fadc_sensitivity = 1.0 fadc_sum_bins = 40 fadc_sum_offset = 9 @@ -135,7 +135,7 @@ save_pe_with_amplitude = 1 store_photoelectrons = 20 tailcut_scale = 2.6 stars = none -config_release = 6.0.2 with simtools v0.24.1.dev97+g5009a391a.d20251103 +config_release = 6.0.2 with simtools v0.27.2.dev544+g80ff08fab.d20260330 config_version = 6.0.2 camera_config_name = LSTN-design camera_config_variant = LSTN-01 diff --git a/tests/resources/sim_telarray_configurations/6.0.2/CTA-North-LSTN-02_test.cfg b/tests/resources/sim_telarray_configurations/6.0.2/CTA-North-LSTN-02_test.cfg index 8cf59df442..78ff29afa1 100644 --- a/tests/resources/sim_telarray_configurations/6.0.2/CTA-North-LSTN-02_test.cfg +++ b/tests/resources/sim_telarray_configurations/6.0.2/CTA-North-LSTN-02_test.cfg @@ -69,7 +69,7 @@ fadc_max_sum = 16777215 fadc_mhz = 1024.0 fadc_noise = 6.7 fadc_pedestal = 400.0 -fadc_pulse_shape = LST_pulse_shape_7dynode_high_intensity_pix1s.dat +fadc_pulse_shape = fadc_pulse_shape-CTA-North-LSTN-02_test.dat fadc_sensitivity = 1.0 fadc_sum_bins = 40 fadc_sum_offset = 9 @@ -135,7 +135,7 @@ save_pe_with_amplitude = 1 store_photoelectrons = 20 tailcut_scale = 2.6 stars = none -config_release = 6.0.2 with simtools v0.24.1.dev97+g5009a391a.d20251103 +config_release = 6.0.2 with simtools v0.27.2.dev544+g80ff08fab.d20260330 config_version = 6.0.2 camera_config_name = LSTN-design camera_config_variant = LSTN-02 diff --git a/tests/resources/sim_telarray_configurations/6.0.2/CTA-North-MSTN-01_test.cfg b/tests/resources/sim_telarray_configurations/6.0.2/CTA-North-MSTN-01_test.cfg index 2448a88913..a6ea9f5a81 100644 --- a/tests/resources/sim_telarray_configurations/6.0.2/CTA-North-MSTN-01_test.cfg +++ b/tests/resources/sim_telarray_configurations/6.0.2/CTA-North-MSTN-01_test.cfg @@ -82,7 +82,7 @@ fadc_max_sum = 16777215 fadc_mhz = 1000.0 fadc_noise = 3.6 fadc_pedestal = 250.0 -fadc_pulse_shape = Pulse_template_nectarCam_17042020-noshift.dat +fadc_pulse_shape = fadc_pulse_shape-CTA-North-MSTN-01_test.dat fadc_sensitivity = 1.0 fadc_sum_bins = 60 fadc_sum_offset = 18 @@ -148,7 +148,7 @@ save_pe_with_amplitude = 1 store_photoelectrons = 26 tailcut_scale = 2.3 stars = none -config_release = 6.0.2 with simtools v0.24.1.dev97+g5009a391a.d20251103 +config_release = 6.0.2 with simtools v0.27.2.dev544+g80ff08fab.d20260330 config_version = 6.0.2 camera_config_name = MSTx-NectarCam camera_config_variant = MSTN-01 diff --git a/tests/resources/sim_telarray_configurations/6.0.2/CTA-South-LSTS-01_test.cfg b/tests/resources/sim_telarray_configurations/6.0.2/CTA-South-LSTS-01_test.cfg index c618902148..849f0fb75e 100644 --- a/tests/resources/sim_telarray_configurations/6.0.2/CTA-South-LSTS-01_test.cfg +++ b/tests/resources/sim_telarray_configurations/6.0.2/CTA-South-LSTS-01_test.cfg @@ -69,7 +69,7 @@ fadc_max_sum = 16777215 fadc_mhz = 1024.0 fadc_noise = 6.7 fadc_pedestal = 400.0 -fadc_pulse_shape = LST_pulse_shape_7dynode_high_intensity_pix1s.dat +fadc_pulse_shape = fadc_pulse_shape-CTA-South-LSTS-01_test.dat fadc_sensitivity = 1.0 fadc_sum_bins = 40 fadc_sum_offset = 9 @@ -135,7 +135,7 @@ save_pe_with_amplitude = 1 store_photoelectrons = 20 tailcut_scale = 2.6 stars = none -config_release = 6.0.2 with simtools v0.24.1.dev97+g5009a391a.d20251103 +config_release = 6.0.2 with simtools v0.27.2.dev544+g80ff08fab.d20260330 config_version = 6.0.2 camera_config_name = LSTS-design camera_config_variant = LSTS-01 diff --git a/tests/resources/sim_telarray_configurations/6.0.2/CTA-South-MSTS-01_test.cfg b/tests/resources/sim_telarray_configurations/6.0.2/CTA-South-MSTS-01_test.cfg index 86f8af60ce..14ef869687 100644 --- a/tests/resources/sim_telarray_configurations/6.0.2/CTA-South-MSTS-01_test.cfg +++ b/tests/resources/sim_telarray_configurations/6.0.2/CTA-South-MSTS-01_test.cfg @@ -85,7 +85,7 @@ fadc_max_sum = 16777215 fadc_mhz = 250.0 fadc_noise = 1.0 fadc_pedestal = 200.0 -fadc_pulse_shape = pulse_FlashCam_7dynode_v2a.dat +fadc_pulse_shape = fadc_pulse_shape-CTA-South-MSTS-01_test.dat fadc_sensitivity = 1.0 fadc_sum_bins = 24 fadc_sum_offset = 6 @@ -151,7 +151,7 @@ save_pe_with_amplitude = 1 store_photoelectrons = 22 tailcut_scale = 2.6 stars = none -config_release = 6.0.2 with simtools v0.24.1.dev97+g5009a391a.d20251103 +config_release = 6.0.2 with simtools v0.27.2.dev544+g80ff08fab.d20260330 config_version = 6.0.2 camera_config_name = MSTx-FlashCam camera_config_variant = MSTS-01 diff --git a/tests/resources/sim_telarray_configurations/6.0.2/CTA-South-SSTS-01_test.cfg b/tests/resources/sim_telarray_configurations/6.0.2/CTA-South-SSTS-01_test.cfg index ebbefbdbe8..afdb312d52 100644 --- a/tests/resources/sim_telarray_configurations/6.0.2/CTA-South-SSTS-01_test.cfg +++ b/tests/resources/sim_telarray_configurations/6.0.2/CTA-South-SSTS-01_test.cfg @@ -84,7 +84,7 @@ fadc_max_sum = 16777215 fadc_mhz = 1000.0 fadc_noise = 4.7 fadc_pedestal = 1000.0 -fadc_pulse_shape = pulse_sstcam_FADC_04042022.dat +fadc_pulse_shape = fadc_pulse_shape-CTA-South-SSTS-01_test.dat fadc_sensitivity = 1.0 fadc_sum_bins = 128 fadc_sum_offset = 24 @@ -162,7 +162,7 @@ save_pe_with_amplitude = 1 store_photoelectrons = 10 tailcut_scale = 1 stars = none -config_release = 6.0.2 with simtools v0.24.1.dev97+g5009a391a.d20251103 +config_release = 6.0.2 with simtools v0.27.2.dev544+g80ff08fab.d20260330 config_version = 6.0.2 camera_config_name = SSTS-design camera_config_variant = SSTS-01 diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index fc16443fb7..976399f31b 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -40,14 +40,7 @@ def _is_db_unit_test(request): if node is None: return False - if node.get_closest_marker("db_unit_test") is not None: - return True - - test_file = "" - location = getattr(node, "location", None) - if location: - test_file = str(location[0]).replace("\\", "/") - return test_file.startswith("tests/unit_tests/db/") + return node.get_closest_marker("db_unit_test") is not None @functools.lru_cache @@ -824,3 +817,61 @@ def _get_test_data_file(file_type, variant="gamma"): return _TEST_DATA_FILES[key] return _get_test_data_file + + +# ============================================================================ +# Shared fixtures for row-table and export testing (Phase 3 consolidation) +# ============================================================================ + + +@pytest.fixture +def row_table_payload(): + """Valid row-table dict with all required keys.""" + return { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.0], [0.5, 0.5], [1.0, 1.0]], + } + + +@pytest.fixture +def row_table_payload_without_units(): + """Row-table dict without column_units key.""" + return { + "columns": ["time", "amplitude"], + "rows": [[0.0, 0.0], [0.5, 0.5], [1.0, 1.0]], + } + + +@pytest.fixture +def invalid_row_table_payloads(): + """Collection of invalid row-table payloads for parametrized testing.""" + return { + "missing_columns": { + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.0]], + }, + "missing_rows": { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + }, + "missing_column_units": { + "columns": ["time", "amplitude"], + "rows": [[0.0, 0.0]], + }, + "column_units_length_mismatch": { + "columns": ["time", "amplitude"], + "column_units": ["ns"], + "rows": [[0.0, 0.0]], + }, + "row_length_mismatch": { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0]], + }, + "non_numeric_value": { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [["not", "numeric"]], + }, + } 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 656fcab599..89f5ea10c8 100644 --- a/tests/unit_tests/data_model/test_model_data_writer.py +++ b/tests/unit_tests/data_model/test_model_data_writer.py @@ -8,6 +8,7 @@ import pytest from astropy.io.registry.base import IORegistryError from astropy.table import Table +from jsonschema.exceptions import ValidationError import simtools.data_model.metadata_collector as metadata_collector import simtools.data_model.model_data_writer as writer @@ -224,6 +225,25 @@ def test_dump_model_parameter(tmp_test_directory): mock_db_check.assert_called_once_with(num_gains_name, instrument, parameter_version) +def test_dump_model_parameter_does_not_write_metadata_on_validation_failure(tmp_test_directory): + output_file = "num_gains.json" + + with pytest.raises(ValueError, match=r"^Value for column '0' out of range."): + writer.ModelDataWriter.dump_model_parameter( + parameter_name="num_gains", + value=25, + instrument="LSTN-01", + parameter_version="1.1.0", + output_file=output_file, + output_path=tmp_test_directory, + metadata_input_dict={"name": "test_metadata"}, + check_db_for_existing_parameter=False, + ) + + assert not (Path(tmp_test_directory) / output_file).exists() + assert not Path(tmp_test_directory / "num_gains.meta.yml").exists() + + def test_get_validated_parameter_dict(): w1 = writer.ModelDataWriter() assert w1.get_validated_parameter_dict( @@ -284,6 +304,154 @@ def test_get_validated_parameter_dict(): } +def test_get_validated_parameter_dict_fadc_pulse_shape_embedded(): + w1 = writer.ModelDataWriter() + embedded_value = { + "columns": ["time", "amplitude", "amplitude (low gain)"], + "column_units": ["ns", "dimensionless", "dimensionless"], + "rows": [ + [0.0, 0.0, 0.0], + [0.12, 0.0, 0.0], + [0.25, 0.01323, 0.000945], + ], + } + + validated_dict = w1.get_validated_parameter_dict( + parameter_name="fadc_pulse_shape", + value=embedded_value, + instrument="LSTN-01", + parameter_version="0.0.1", + model_parameter_schema_version="0.2.0", + ) + + assert validated_dict["type"] == "dict" + assert not validated_dict["file"] + assert validated_dict["model_parameter_schema_version"] == "0.2.0" + assert validated_dict["value"] == embedded_value + + +def test_get_parameter_type_for_schema_uses_selected_schema_version(): + writer_instance = writer.ModelDataWriter() + + assert writer_instance.get_parameter_type_for_schema("fadc_pulse_shape", "0.2.0") == "dict" + assert writer_instance.get_parameter_type_for_schema("fadc_pulse_shape", "0.1.0") == "file" + + +def test_parameter_uses_row_table_schema_true(mocker): + writer_instance = writer.ModelDataWriter() + mocker.patch.object( + writer_instance, + "_read_schema_dict", + return_value=( + { + "data": [ + { + "type": "dict", + "json_schema": { + "required": ["columns", "column_units", "rows"], + "properties": { + "columns": {}, + "column_units": {}, + "rows": {}, + }, + }, + } + ] + }, + "schema.yml", + ), + ) + + assert writer_instance.parameter_uses_row_table_schema("fadc_pulse_shape", "0.2.0") + + +def test_parameter_uses_row_table_schema_false_for_generic_dict(mocker): + writer_instance = writer.ModelDataWriter() + mocker.patch.object( + writer_instance, + "_read_schema_dict", + return_value=( + { + "data": [ + { + "type": "dict", + "json_schema": { + "required": ["name", "multiplicity"], + "properties": { + "name": {}, + "multiplicity": {}, + }, + }, + } + ] + }, + "schema.yml", + ), + ) + + assert not writer_instance.parameter_uses_row_table_schema("array_triggers", "0.2.0") + + +def test_get_validated_parameter_dict_fadc_pulse_shape_file_legacy(): + w1 = writer.ModelDataWriter() + + validated_dict = w1.get_validated_parameter_dict( + parameter_name="fadc_pulse_shape", + value="pulse_shape.dat", + instrument="LSTN-01", + parameter_version="0.0.1", + model_parameter_schema_version="0.1.0", + ) + + assert validated_dict["type"] == "file" + assert validated_dict["file"] + assert validated_dict["model_parameter_schema_version"] == "0.1.0" + assert validated_dict["value"] == "pulse_shape.dat" + + +def test_get_validated_parameter_dict_fadc_pulse_shape_embedded_invalid_columns(): + w1 = writer.ModelDataWriter() + invalid_embedded_value = { + "columns": ["time", "HG"], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.0], [0.12, 0.01]], + } + + with pytest.raises(ValidationError): + w1.get_validated_parameter_dict( + parameter_name="fadc_pulse_shape", + value=invalid_embedded_value, + instrument="LSTN-01", + parameter_version="0.0.1", + model_parameter_schema_version="0.2.0", + ) + + +@pytest.mark.parametrize( + "invalid_rows", + [ + [[0.0], [0.12]], + [[0.0, 0.0, 0.0, 0.0], [0.12, 0.01, 0.001, 0.0001]], + ], +) +def test_get_validated_parameter_dict_fadc_pulse_shape_embedded_invalid_row_length(invalid_rows): + w1 = writer.ModelDataWriter() + invalid_embedded_value = { + "columns": ["time", "amplitude", "amplitude (low gain)"], + "column_units": ["ns", "dimensionless", "dimensionless"], + "rows": invalid_rows, + } + + with pytest.raises(ValidationError): + w1.get_validated_parameter_dict( + parameter_name="fadc_pulse_shape", + value=invalid_embedded_value, + instrument="LSTN-01", + parameter_version="0.0.1", + model_parameter_schema_version="0.2.0", + ) + + def test_prepare_data_dict_for_writing(): data_dict_5 = { "value": [5.5, 6.6], diff --git a/tests/unit_tests/data_model/test_row_table_utils.py b/tests/unit_tests/data_model/test_row_table_utils.py new file mode 100644 index 0000000000..1c58e18e40 --- /dev/null +++ b/tests/unit_tests/data_model/test_row_table_utils.py @@ -0,0 +1,304 @@ +#!/usr/bin/python3 +"""Tests for row_table_utils module.""" + +import pytest +from astropy.units import dimensionless_unscaled, ns + +from simtools.data_model import row_table_utils + + +class TestIsRowTableDict: + """Test row-table dict identification.""" + + def test_is_row_table_dict_valid(self): + """Identify valid row-table dict.""" + payload = { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.1], [1.0, 0.2]], + } + assert row_table_utils.is_row_table_dict(payload) + + def test_is_row_table_dict_missing_key(self): + """Reject incomplete dict.""" + payload = { + "columns": ["time", "amplitude"], + "rows": [[0.0, 0.1]], + } + assert not row_table_utils.is_row_table_dict(payload) + + def test_is_row_table_dict_non_dict(self): + """Reject non-dict values.""" + assert not row_table_utils.is_row_table_dict("string") + assert not row_table_utils.is_row_table_dict([1, 2, 3]) + assert not row_table_utils.is_row_table_dict(None) + + +class TestIsRowTableSchema: + """Test row-table schema detection.""" + + def test_is_row_table_schema_valid(self): + """Identify valid row-table JSON schema.""" + json_schema = { + "required": ["columns", "column_units", "rows"], + "properties": { + "columns": {}, + "column_units": {}, + "rows": {}, + }, + } + assert row_table_utils.is_row_table_schema(json_schema) + + def test_is_row_table_schema_missing_required(self): + """Reject schema without all required keys.""" + json_schema = { + "required": ["columns", "rows"], + "properties": { + "columns": {}, + "column_units": {}, + "rows": {}, + }, + } + assert not row_table_utils.is_row_table_schema(json_schema) + + def test_is_row_table_schema_missing_property(self): + """Reject schema without all properties.""" + json_schema = { + "required": ["columns", "column_units", "rows"], + "properties": { + "columns": {}, + "rows": {}, + }, + } + assert not row_table_utils.is_row_table_schema(json_schema) + + def test_is_row_table_schema_extra_keys_ok(self): + """Accept schema with extra keys alongside required ones.""" + json_schema = { + "required": ["columns", "column_units", "rows"], + "properties": { + "columns": {}, + "column_units": {}, + "rows": {}, + "extra_field": {}, + }, + } + assert row_table_utils.is_row_table_schema(json_schema) + + +class TestValidateRowTableStructure: + """Test row-table structure validation.""" + + def test_validate_row_table_structure_valid(self): + """Accept valid row-table structure.""" + payload = { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.1], [1.0, 0.2]], + } + row_table_utils.validate_row_table_structure("test_param", payload) + + def test_validate_row_table_structure_missing_columns(self): + """Reject missing columns key.""" + payload = { + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.1]], + } + with pytest.raises(ValueError, match="'columns'"): + row_table_utils.validate_row_table_structure("test_param", payload) + + def test_validate_row_table_structure_missing_rows(self): + """Reject missing rows key.""" + payload = { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + } + with pytest.raises(ValueError, match="'rows'"): + row_table_utils.validate_row_table_structure("test_param", payload) + + def test_validate_row_table_structure_missing_column_units(self): + """Reject missing column_units key.""" + payload = { + "columns": ["time", "amplitude"], + "rows": [[0.0, 0.1]], + } + with pytest.raises(ValueError, match="'column_units'"): + row_table_utils.validate_row_table_structure("test_param", payload) + + def test_validate_row_table_structure_column_units_length_mismatch(self): + """Reject mismatched column_units length.""" + payload = { + "columns": ["time", "amplitude"], + "column_units": ["ns"], + "rows": [[0.0, 0.1]], + } + with pytest.raises(ValueError, match="column_units length"): + row_table_utils.validate_row_table_structure("test_param", payload) + + def test_validate_row_table_structure_invalid_columns_type(self): + """Reject columns when not list or tuple.""" + payload = { + "columns": "time,amplitude", + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.1]], + } + with pytest.raises(ValueError, match="'columns' must be a list or tuple"): + row_table_utils.validate_row_table_structure("test_param", payload) + + def test_validate_row_table_structure_invalid_rows_type(self): + """Reject rows when not list or tuple.""" + payload = { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": {"time": 0.0, "amplitude": 0.1}, + } + with pytest.raises(ValueError, match="'rows' must be a list or tuple"): + row_table_utils.validate_row_table_structure("test_param", payload) + + def test_validate_row_table_structure_non_string_column_name(self): + """Reject non-string column names.""" + payload = { + "columns": ["time", 1], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.1]], + } + with pytest.raises(ValueError, match="all column names"): + row_table_utils.validate_row_table_structure("test_param", payload) + + @pytest.mark.parametrize( + "invalid_rows", + [ + [[0.0]], + [[0.0, 0.1, 0.2]], + ], + ) + def test_validate_row_table_structure_row_length_mismatch(self, invalid_rows): + """Reject rows with incorrect length.""" + payload = { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": invalid_rows, + } + with pytest.raises(ValueError, match="row length"): + row_table_utils.validate_row_table_structure("test_param", payload) + + def test_validate_row_table_structure_non_numeric_value(self): + """Reject non-numeric row values.""" + payload = { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [["not", "numeric"]], + } + with pytest.raises(ValueError, match=r"non-real-numeric|non-numeric"): + row_table_utils.validate_row_table_structure("test_param", payload) + + def test_validate_row_table_structure_complex_number(self): + """Reject complex numbers in rows.""" + payload = { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 1 + 2j]], + } + with pytest.raises(ValueError, match="non-real-numeric"): + row_table_utils.validate_row_table_structure("test_param", payload) + + def test_validate_row_table_structure_non_sequence_row(self): + """Reject non-sequence rows.""" + payload = { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [5.0], + } + with pytest.raises(ValueError, match="must be a sequence"): + row_table_utils.validate_row_table_structure("test_param", payload) + + +class TestNormalizeColumnUnit: + """Test column unit normalization.""" + + def test_normalize_column_unit_none(self): + """Convert None to dimensionless.""" + assert row_table_utils.normalize_column_unit(None) == "dimensionless" + + def test_normalize_column_unit_empty_string(self): + """Convert empty string to dimensionless.""" + assert row_table_utils.normalize_column_unit("") == "dimensionless" + + def test_normalize_column_unit_string(self): + """Pass through string units unchanged.""" + assert row_table_utils.normalize_column_unit("ns") == "ns" + assert row_table_utils.normalize_column_unit("km") == "km" + + def test_normalize_column_unit_dimensionless_unscaled(self): + """Convert astropy dimensionless to string.""" + assert row_table_utils.normalize_column_unit(dimensionless_unscaled) == "dimensionless" + + def test_normalize_column_unit_astropy_unit(self): + """Convert astropy unit to string.""" + result = row_table_utils.normalize_column_unit(ns) + assert isinstance(result, str) + assert "ns" in result or result == "ns" + + +class TestValidateRowTableStructureParametrized: + """Parametrized tests for various invalid row-table payloads.""" + + @pytest.mark.parametrize( + ("invalid_key", "payload"), + [ + ("missing_columns", {"column_units": ["ns", "dimensionless"], "rows": [[0.0, 0.0]]}), + ( + "missing_rows", + {"columns": ["time", "amplitude"], "column_units": ["ns", "dimensionless"]}, + ), + ("missing_column_units", {"columns": ["time", "amplitude"], "rows": [[0.0, 0.0]]}), + ( + "column_units_mismatch", + {"columns": ["time", "amplitude"], "column_units": ["ns"], "rows": [[0.0, 0.0]]}, + ), + ( + "row_length_mismatch", + { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0]], + }, + ), + ( + "invalid_columns_type", + { + "columns": "time,amplitude", + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.0]], + }, + ), + ( + "invalid_rows_type", + { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": {"time": 0.0, "amplitude": 0.0}, + }, + ), + ( + "non_string_column_name", + { + "columns": ["time", 1], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.0]], + }, + ), + ( + "non_numeric", + { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [["not", "numeric"]], + }, + ), + ], + ) + def test_validate_row_table_structure_invalid_payload(self, invalid_key, payload): + """Test various invalid payloads with parametrization.""" + with pytest.raises(ValueError, match="Row-table"): + row_table_utils.validate_row_table_structure("test_param", payload) diff --git a/tests/unit_tests/db/test_db_handler.py b/tests/unit_tests/db/test_db_handler.py index c83ca4f24d..e8ba626769 100644 --- a/tests/unit_tests/db/test_db_handler.py +++ b/tests/unit_tests/db/test_db_handler.py @@ -53,7 +53,7 @@ def mock_get_collection_name(mocker): @pytest.fixture def mock_read_simtel_table(mocker): return mocker.patch( - "simtools.db.db_handler.simtel_table_reader.read_simtel_table", + "simtools.db.parameter_exporter.simtel_table_reader.read_simtel_table", return_value="test_table", ) @@ -77,7 +77,7 @@ def export_files_setup(db, mocker): mock_get_file_mongo_db = mocker.patch.object( db.mongo_db_handler, "get_file_from_db", return_value=mocker.Mock(_id="file_id") ) - mock_write_file = mocker.patch.object(db, "_write_file_from_db_to_disk") + mock_write_file = mocker.patch.object(db, "write_file_from_db_to_disk") return {"get_file_mongo_db": mock_get_file_mongo_db, "write_file": mock_write_file} @@ -505,7 +505,7 @@ def test_export_model_files_with_parameters( def test_export_model_files_file_exists(db, mocker, tmp_test_directory, test_db, test_file): """Test export_model_files method when file already exists.""" mock_get_file_mongo_db = mocker.patch.object(db.mongo_db_handler, "get_file_from_db") - mock_write_file = mocker.patch.object(db, "_write_file_from_db_to_disk") + mock_write_file = mocker.patch.object(db, "write_file_from_db_to_disk") mock_path_exists = mocker.patch("pathlib.Path.exists", return_value=True) file_names = [test_file] @@ -522,7 +522,7 @@ def test_export_model_files_file_not_found(db, mocker, tmp_test_directory, test_ mock_get_file_mongo_db = mocker.patch.object( db.mongo_db_handler, "get_file_from_db", side_effect=FileNotFoundError ) - mock_write_file = mocker.patch.object(db, "_write_file_from_db_to_disk") + mock_write_file = mocker.patch.object(db, "write_file_from_db_to_disk") parameters = {"param1": {"file": True, "value": test_file}} @@ -533,6 +533,39 @@ def test_export_model_files_file_not_found(db, mocker, tmp_test_directory, test_ mock_write_file.assert_not_called() +def test_export_parameter_data_delegates_to_parameter_exporter(db, mocker): + """Delegate parameter payload export to parameter_exporter helper.""" + expected = ["output.dat"] + export_mock = mocker.patch( + "simtools.db.db_handler.parameter_exporter.export_parameter_data", + return_value=expected, + ) + + result = db.export_parameter_data( + parameter="mirror_reflectivity", + site="North", + array_element_name="LSTN-01", + parameter_version="1.0.0", + model_version="6.0.2", + output_file=None, + export_model_file=True, + export_model_file_as_table=False, + ) + + assert result == expected + export_mock.assert_called_once_with( + db=db, + parameter="mirror_reflectivity", + site="North", + array_element_name="LSTN-01", + parameter_version="1.0.0", + model_version="6.0.2", + output_file=None, + export_model_file=True, + export_model_file_as_table=False, + ) + + def test_get_query_from_parameter_version_table(db): """Test _get_query_from_parameter_version_table method.""" or_list = [ @@ -1152,6 +1185,47 @@ def test_export_model_file_variants( assert result == test_case["expected_result"] +def test_export_model_file_dict_type_returns_table(db, mocker): + """Test export_model_file returns an astropy Table for dict-typed parameters.""" + row_data = { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.0], [0.5, 0.12]], + } + mock_parameters = {"fadc_pulse_shape": {"type": "dict", "value": row_data}} + mocker.patch.object(db, "get_model_parameter", return_value=mock_parameters) + export_files_mock = mocker.patch.object(db, "export_model_files") + + table = db.export_model_file( + parameter="fadc_pulse_shape", + site="North", + array_element_name="LSTN-01", + model_version="1.0.0", + export_file_as_table=True, + ) + + export_files_mock.assert_not_called() + assert list(table.colnames) == ["time", "amplitude"] + assert len(table) == 2 + + +def test_export_model_file_dict_type_without_table_flag_returns_none(db, mocker): + """Test export_model_file returns None for dict-typed parameters when flag is False.""" + row_data = {"columns": ["time"], "column_units": ["ns"], "rows": [[0.0]]} + mock_parameters = {"fadc_pulse_shape": {"type": "dict", "value": row_data}} + mocker.patch.object(db, "get_model_parameter", return_value=mock_parameters) + + result = db.export_model_file( + parameter="fadc_pulse_shape", + site="North", + array_element_name="LSTN-01", + model_version="1.0.0", + export_file_as_table=False, + ) + + assert result is None + + def test_get_array_element_list_configuration_sim_telarray(db, mocker): """Test _get_array_element_list method for configuration_sim_telarray collection.""" array_element_name = "LSTN-01" @@ -1237,7 +1311,7 @@ def test_write_file_from_db_to_disk_delegation(db, mocker, tmp_test_directory): """Test _write_file_from_db_to_disk delegates to mongo_db_handler.""" mock_write = mocker.patch.object(db.mongo_db_handler, "write_file_from_db_to_disk") mock_file = mocker.Mock() - db._write_file_from_db_to_disk("test_db", tmp_test_directory, mock_file) + db.write_file_from_db_to_disk("test_db", tmp_test_directory, mock_file) mock_write.assert_called_once_with("test_db", tmp_test_directory, mock_file) diff --git a/tests/unit_tests/db/test_mongo_db.py b/tests/unit_tests/db/test_mongo_db.py index eefd8a1304..6628e0b833 100644 --- a/tests/unit_tests/db/test_mongo_db.py +++ b/tests/unit_tests/db/test_mongo_db.py @@ -16,8 +16,14 @@ @pytest.fixture(autouse=True) def reset_db_client(): """Reset db_client before each test.""" + existing_client = mongo_db.MongoDBHandler.db_client + if existing_client is not None and hasattr(existing_client, "close"): + existing_client.close() mongo_db.MongoDBHandler.db_client = None yield + existing_client = mongo_db.MongoDBHandler.db_client + if existing_client is not None and hasattr(existing_client, "close"): + existing_client.close() mongo_db.MongoDBHandler.db_client = None @@ -36,8 +42,9 @@ def valid_db_config(): @pytest.fixture -def mongo_handler(valid_db_config): +def mongo_handler(mocker, valid_db_config): """Create a MongoDBHandler instance.""" + mocker.patch("simtools.db.mongo_db.MongoClient", return_value=mocker.MagicMock()) return mongo_db.MongoDBHandler(valid_db_config) @@ -113,8 +120,9 @@ def test_get_db_name_incomplete(): assert result is None -def test_init_with_valid_config(valid_db_config): +def test_init_with_valid_config(mocker, valid_db_config): """Test initialization with valid configuration.""" + mocker.patch("simtools.db.mongo_db.MongoClient", return_value=mocker.MagicMock()) handler = mongo_db.MongoDBHandler(valid_db_config) assert handler.db_config == valid_db_config assert handler.list_of_collections == {} @@ -240,16 +248,18 @@ def test_idle_connection_monitor(mocker): assert monitor.open_connections == 0 -def test_is_remote_database_true(valid_db_config): +def test_is_remote_database_true(mocker, valid_db_config): """Test is_remote_database with remote server.""" valid_db_config["db_server"] = "cta-simpipe-protodb.zeuthen.desy.de" + mocker.patch("simtools.db.mongo_db.MongoClient", return_value=mocker.MagicMock()) handler = mongo_db.MongoDBHandler(valid_db_config) assert handler.is_remote_database() is True -def test_is_remote_database_false_localhost(valid_db_config): +def test_is_remote_database_false_localhost(mocker, valid_db_config): """Test is_remote_database with localhost.""" valid_db_config["db_server"] = "localhost" + mocker.patch("simtools.db.mongo_db.MongoClient", return_value=mocker.MagicMock()) handler = mongo_db.MongoDBHandler(valid_db_config) assert handler.is_remote_database() is False @@ -260,8 +270,9 @@ def test_is_remote_database_false_no_config(): assert handler.is_remote_database() is False -def test_print_connection_info(valid_db_config, caplog): +def test_print_connection_info(mocker, valid_db_config, caplog): """Test print_connection_info.""" + mocker.patch("simtools.db.mongo_db.MongoClient", return_value=mocker.MagicMock()) handler = mongo_db.MongoDBHandler(valid_db_config) with caplog.at_level(logging.INFO): handler.print_connection_info("test_db") diff --git a/tests/unit_tests/db/test_parameter_exporter.py b/tests/unit_tests/db/test_parameter_exporter.py new file mode 100644 index 0000000000..174151f6d7 --- /dev/null +++ b/tests/unit_tests/db/test_parameter_exporter.py @@ -0,0 +1,245 @@ +"""Tests for db parameter export orchestration.""" + +# pylint: disable=redefined-outer-name + +import pytest + +from simtools.db import parameter_exporter + +pytestmark = pytest.mark.db_unit_test + + +@pytest.fixture +def db_handler_mock(mocker): + """Create a basic mocked DB handler with output file helper.""" + db = mocker.Mock() + db.io_handler.get_output_file.return_value = mocker.MagicMock() + return db + + +def test_export_parameter_data_writes_ecsv_for_dict_parameter(mocker, db_handler_mock): + """Export dict-typed parameter values as ECSV using output_file.""" + db_handler_mock.get_model_parameter.return_value = { + "fadc_pulse_shape": { + "type": "dict", + "value": { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[1.0, 2.0]], + }, + } + } + table = mocker.Mock() + mock_export_single = mocker.patch.object( + parameter_exporter, "export_single_model_file", return_value=table + ) + db_handler_mock.io_handler.get_output_file.return_value.with_suffix.return_value = ( + "fadc_pulse_shape.ecsv" + ) + + output_files = parameter_exporter.export_parameter_data( + db=db_handler_mock, + parameter="fadc_pulse_shape", + site="North", + array_element_name="LSTN-01", + parameter_version="2.0.0", + model_version=None, + output_file="fadc_pulse_shape.json", + export_model_file=True, + export_model_file_as_table=False, + ) + + mock_export_single.assert_called_once_with( + db=db_handler_mock, + parameter="fadc_pulse_shape", + site="North", + array_element_name="LSTN-01", + parameter_version="2.0.0", + model_version=None, + export_file_as_table=True, + parameters=db_handler_mock.get_model_parameter.return_value, + par_info=db_handler_mock.get_model_parameter.return_value["fadc_pulse_shape"], + ) + table.write.assert_called_once_with( + "fadc_pulse_shape.ecsv", format="ascii.ecsv", overwrite=True + ) + assert output_files == ["fadc_pulse_shape.ecsv"] + + +def test_export_parameter_data_requires_output_file_for_dict_parameter(db_handler_mock): + """Require output_file for dict-typed export.""" + db_handler_mock.get_model_parameter.return_value = { + "fadc_pulse_shape": { + "type": "dict", + "value": { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[1.0, 2.0]], + }, + } + } + + with pytest.raises(ValueError, match="--output_file"): + parameter_exporter.export_parameter_data( + db=db_handler_mock, + parameter="fadc_pulse_shape", + site="North", + array_element_name="LSTN-01", + parameter_version="2.0.0", + model_version=None, + output_file=None, + export_model_file=True, + export_model_file_as_table=False, + ) + + +def test_export_parameter_data_requires_export_model_file_for_table_export(db_handler_mock): + """Reject export_model_file_as_table without export_model_file.""" + with pytest.raises(ValueError, match="Use --export_model_file together"): + parameter_exporter.export_parameter_data( + db=db_handler_mock, + parameter="mirror_reflectivity", + site="North", + array_element_name="LSTN-01", + parameter_version=None, + model_version="6.0.2", + output_file=None, + export_model_file=False, + export_model_file_as_table=True, + ) + + +def test_export_parameter_data_rejects_output_file_for_file_parameter(db_handler_mock): + """Reject output_file for file-backed parameter export.""" + db_handler_mock.get_model_parameter.return_value = { + "mirror_reflectivity": {"type": "file", "value": "ref_LST1_2022_04_01.dat"} + } + + with pytest.raises(ValueError, match="Do not use --output_file"): + parameter_exporter.export_parameter_data( + db=db_handler_mock, + parameter="mirror_reflectivity", + site="North", + array_element_name="LSTN-01", + parameter_version=None, + model_version="6.0.2", + output_file="mirror_reflectivity.dat", + export_model_file=True, + export_model_file_as_table=False, + ) + + +def test_export_parameter_data_returns_file_and_table_outputs(mocker, db_handler_mock): + """Return both original file and ECSV file for file-backed table export.""" + db_handler_mock.get_model_parameter.return_value = { + "mirror_reflectivity": {"type": "file", "value": "ref_LST1_2022_04_01.dat"} + } + table = mocker.Mock() + mock_export_single = mocker.patch.object( + parameter_exporter, "export_single_model_file", return_value=table + ) + file_path = mocker.MagicMock() + file_path.suffix = ".dat" + file_path.with_suffix.return_value = "ref_LST1_2022_04_01.ecsv" + db_handler_mock.io_handler.get_output_file.return_value = file_path + + output_files = parameter_exporter.export_parameter_data( + db=db_handler_mock, + parameter="mirror_reflectivity", + site="North", + array_element_name="LSTN-01", + parameter_version=None, + model_version="6.0.2", + output_file=None, + export_model_file=True, + export_model_file_as_table=True, + ) + + mock_export_single.assert_called_once_with( + db=db_handler_mock, + parameter="mirror_reflectivity", + site="North", + array_element_name="LSTN-01", + parameter_version=None, + model_version="6.0.2", + export_file_as_table=True, + parameters=db_handler_mock.get_model_parameter.return_value, + par_info=db_handler_mock.get_model_parameter.return_value["mirror_reflectivity"], + ) + table.write.assert_called_once_with( + "ref_LST1_2022_04_01.ecsv", format="ascii.ecsv", overwrite=True + ) + assert output_files == [file_path, "ref_LST1_2022_04_01.ecsv"] + + +def test_export_model_files_requires_destination(db_handler_mock): + """Require destination path for exporting files from DB.""" + with pytest.raises(ValueError, match="Destination path is required"): + parameter_exporter.export_model_files( + db=db_handler_mock, + parameters={"mirror_reflectivity": {"file": True, "value": "test.dat"}}, + dest=None, + ) + + +def test_normalize_file_names_returns_empty_list_for_no_inputs(): + """Return empty file list when neither file_names nor parameters are provided.""" + assert parameter_exporter._normalize_file_names() == [] + + +def test_export_parameter_data_file_parameter_ecsv_suffix_skips_table_conversion( + mocker, db_handler_mock +): + """Skip creating an extra ECSV file when exported model file is already .ecsv.""" + db_handler_mock.get_model_parameter.return_value = { + "mirror_reflectivity": {"type": "file", "value": "mirror_reflectivity.ecsv"} + } + table = mocker.Mock() + mocker.patch.object(parameter_exporter, "export_single_model_file", return_value=table) + + output_file = mocker.MagicMock() + output_file.suffix = ".ecsv" + db_handler_mock.io_handler.get_output_file.return_value = output_file + + output_files = parameter_exporter.export_parameter_data( + db=db_handler_mock, + parameter="mirror_reflectivity", + site="North", + array_element_name="LSTN-01", + parameter_version=None, + model_version="6.0.2", + output_file=None, + export_model_file=True, + export_model_file_as_table=True, + ) + + assert output_files == [output_file] + table.write.assert_not_called() + + +class TestExportParameterDataValidationErrors: + """Parametrized tests for export parameter validation error conditions.""" + + def test_export_parameter_data_requires_export_model_file_flag(self, db_handler_mock): + """Test that export_model_file_as_table requires export_model_file.""" + with pytest.raises(ValueError, match="Use --export_model_file together"): + parameter_exporter.export_parameter_data( + db=db_handler_mock, + parameter="test_param", + site="North", + array_element_name="LSTN-01", + export_model_file=False, + export_model_file_as_table=True, + ) + + def test_export_parameter_data_no_export_returns_empty_list(self, db_handler_mock): + """Test that no export flags returns empty list.""" + result = parameter_exporter.export_parameter_data( + db=db_handler_mock, + parameter="test_param", + site="North", + array_element_name="LSTN-01", + export_model_file=False, + export_model_file_as_table=False, + ) + assert result == [] diff --git a/tests/unit_tests/model/test_legacy_model_parameter.py b/tests/unit_tests/model/test_legacy_model_parameter.py index 1ac4671ab9..06e8db51eb 100644 --- a/tests/unit_tests/model/test_legacy_model_parameter.py +++ b/tests/unit_tests/model/test_legacy_model_parameter.py @@ -5,7 +5,10 @@ from simtools.model.legacy_model_parameter import ( UPDATE_HANDLERS, _get_unsupported_update_message, + _update_corsika_starting_grammage, _update_dsum_threshold, + _update_fadc_pulse_shape, + _update_file_backed_table_parameter, _update_flasher_pulse_shape, apply_legacy_updates_to_parameters, register_update, @@ -17,11 +20,11 @@ def test_register_update(): """Test register_update decorator.""" @register_update("test_parameter") - def test_handler(parameters, schema_version): + def test_handler(*_args, **_kwargs): return {"test": "value"} assert "test_parameter" in UPDATE_HANDLERS - assert UPDATE_HANDLERS["test_parameter"] == test_handler + assert UPDATE_HANDLERS["test_parameter"] is test_handler # Clean up del UPDATE_HANDLERS["test_parameter"] @@ -182,3 +185,159 @@ def test_update_parameter_returns_handler_result(): assert "flasher_pulse_shape" in result assert result["flasher_pulse_shape"]["value"] == ["gaussian", 5.0, 10.0] assert result["flasher_pulse_shape"]["model_parameter_schema_version"] == "0.2.0" + + +@pytest.mark.parametrize("legacy_type", ["file", "string"]) +def test_update_fadc_pulse_shape_from_file_to_embedded_row_data(mocker, legacy_type): + """Test file-backed fadc_pulse_shape migration to embedded row data.""" + expected_value = { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.1], [1.0, 0.2]], + } + parameters = { + "fadc_pulse_shape": { + "parameter": "fadc_pulse_shape", + "value": "pulse.dat", + "model_parameter_schema_version": "0.1.0", + "type": legacy_type, + "file": True, + } + } + resolver = mocker.Mock(return_value=expected_value) + + result = _update_fadc_pulse_shape(parameters, "0.2.0", value_resolver=resolver) + + resolver.assert_called_once_with("fadc_pulse_shape", "pulse.dat") + assert result["fadc_pulse_shape"]["value"] == expected_value + assert result["fadc_pulse_shape"]["model_parameter_schema_version"] == "0.2.0" + assert result["fadc_pulse_shape"]["type"] == "dict" + assert result["fadc_pulse_shape"]["file"] is False + + +@pytest.mark.parametrize( + ("legacy_value", "expected_value"), + [ + ( + { + "columns": ["time", "amplitude", "amplitude (low gain)"], + "column_units": ["ns", "dimensionless", "dimensionless"], + "rows": [[0.0, 0.1, 0.01], [1.0, 0.2, 0.02]], + }, + { + "columns": ["time", "amplitude", "amplitude (low gain)"], + "column_units": ["ns", "dimensionless", "dimensionless"], + "rows": [[0.0, 0.1, 0.01], [1.0, 0.2, 0.02]], + }, + ), + ], +) +def test_update_fadc_pulse_shape_embedded_formats(legacy_value, expected_value): + """Test embedded fadc_pulse_shape migration for legacy and canonical dict layouts.""" + parameters = { + "fadc_pulse_shape": { + "parameter": "fadc_pulse_shape", + "value": legacy_value, + "model_parameter_schema_version": "0.2.0", + "type": "dict", + "file": False, + } + } + + result = _update_fadc_pulse_shape(parameters, "0.2.0") + + assert result["fadc_pulse_shape"]["value"] == expected_value + assert result["fadc_pulse_shape"]["model_parameter_schema_version"] == "0.2.0" + + +def test_update_fadc_pulse_shape_embedded_column_data_is_unsupported(): + """Reject legacy embedded column-data representation for fadc_pulse_shape.""" + parameters = { + "fadc_pulse_shape": { + "parameter": "fadc_pulse_shape", + "value": { + "columns": ["time", "amplitude", "amplitude (low gain)"], + "dtype": ["float64", "float64", "float64"], + "unit": ["ns", "dimensionless", "dimensionless"], + "data": { + "time": [0.0, 1.0], + "amplitude": [0.1, 0.2], + "amplitude (low gain)": [0.01, 0.02], + }, + }, + "model_parameter_schema_version": "0.2.0", + "type": "dict", + "file": False, + } + } + + with pytest.raises( + ValueError, + match=r"Unsupported update for legacy parameter fadc_pulse_shape.*0.2.0 to 0.2.0", + ): + _update_fadc_pulse_shape(parameters, "0.2.0") + + +def test_update_parameter_passes_value_resolver(mocker): + """Test update_parameter forwards the optional value_resolver to the handler.""" + parameters = { + "fadc_pulse_shape": { + "parameter": "fadc_pulse_shape", + "value": "pulse.dat", + "model_parameter_schema_version": "0.1.0", + "type": "file", + "file": True, + } + } + resolver = mocker.Mock( + return_value={ + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.1]], + } + ) + + result = update_parameter( + "fadc_pulse_shape", + parameters, + "0.2.0", + value_resolver=resolver, + ) + + resolver.assert_called_once_with("fadc_pulse_shape", "pulse.dat") + assert result["fadc_pulse_shape"]["model_parameter_schema_version"] == "0.2.0" + + +def test_update_file_backed_table_parameter_requires_value_resolver(): + """Raise when updating file-backed table parameter without value_resolver.""" + parameters = { + "fadc_pulse_shape": { + "parameter": "fadc_pulse_shape", + "value": "pulse.dat", + "model_parameter_schema_version": "0.1.0", + "type": "file", + "file": True, + } + } + + with pytest.raises(ValueError, match="value_resolver is required"): + _update_file_backed_table_parameter( + parameter_name="fadc_pulse_shape", + parameters=parameters, + schema_version="0.2.0", + ) + + +def test_update_corsika_starting_grammage_returns_remove_placeholder(): + """Return None placeholder update for legacy corsika_starting_grammage.""" + parameters = { + "corsika_starting_grammage": { + "parameter": "corsika_starting_grammage", + "value": 1.0, + "model_parameter_schema_version": "0.1.0", + } + } + + result = _update_corsika_starting_grammage(parameters, "0.2.0") + + assert result == {"corsika_starting_grammage": None} diff --git a/tests/unit_tests/model/test_model_parameter.py b/tests/unit_tests/model/test_model_parameter.py index d37e6dfcbf..7a2bc09ee6 100644 --- a/tests/unit_tests/model/test_model_parameter.py +++ b/tests/unit_tests/model/test_model_parameter.py @@ -764,5 +764,41 @@ def test_check_model_parameter_versions_triggers_legacy_update(mocker): _check_model_parameter_versions(parameters, ignore_software_version=False) # Legacy update triggered because schema version mismatch (0.9.0 != 1.0.0) - mock_update.assert_called_once_with("num_gains", parameters, "1.0.0") + mock_update.assert_called_once_with( + "num_gains", + parameters, + "1.0.0", + value_resolver=None, + ) mock_apply.assert_called_once_with(parameters, {"num_gains": {"value": 99}}) + + +def test_resolve_legacy_table_parameter_value_exports_and_resolves(mocker): + """Export legacy table file to temp dir and resolve to row-data dict.""" + model_parameter = ModelParameter.__new__(ModelParameter) + model_parameter.db = mocker.Mock() + expected = { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.0], [0.1, 0.2]], + } + resolve_mock = mocker.patch( + "simtools.model.model_parameter.simtel_table_reader.resolve_dict_parameter_value", + return_value=expected, + ) + + result = model_parameter._resolve_legacy_table_parameter_value( + "fadc_pulse_shape", + "pulse.dat", + ) + + assert result == expected + model_parameter.db.export_model_files.assert_called_once() + export_kwargs = model_parameter.db.export_model_files.call_args.kwargs + assert export_kwargs["file_names"] == ["pulse.dat"] + assert isinstance(export_kwargs["dest"], Path) + resolve_mock.assert_called_once_with( + "pulse.dat", + "fadc_pulse_shape", + data_path=export_kwargs["dest"], + ) diff --git a/tests/unit_tests/simtel/test_simtel_config_writer.py b/tests/unit_tests/simtel/test_simtel_config_writer.py index e603d6a5c5..c93efb1cb3 100644 --- a/tests/unit_tests/simtel/test_simtel_config_writer.py +++ b/tests/unit_tests/simtel/test_simtel_config_writer.py @@ -9,6 +9,7 @@ import numpy as np import pytest +import simtools.simtel.simtel_table_writer as simtel_table_writer from simtools.simtel.simtel_config_writer import SimtelConfigWriter logger = logging.getLogger() @@ -325,6 +326,38 @@ def test_convert_model_parameters_to_simtel_format_hard_stereo_false( assert "width 10" in content +def test_convert_model_parameters_to_simtel_format_returns_none_on_attribute_error( + simtel_config_writer, tmp_test_directory +): + """Return (None, None) when conversion needs telescope model but model is missing.""" + model_path = Path(tmp_test_directory) / "model" + model_path.mkdir(exist_ok=True) + + simtel_name, value = simtel_config_writer._convert_model_parameters_to_simtel_format( + "array_triggers", + [{"name": "LSTS_single_telescope", "multiplicity": {"value": 1}}], + model_path, + None, + ) + + assert simtel_name is None + assert value is None + + +def test_write_table_parameter_file_passes_through_non_dict_value( + simtel_config_writer, tmp_test_directory +): + """Keep string values unchanged in table-parameter file conversion helper.""" + result = simtel_config_writer._write_table_parameter_file( + "fadc_pulse_shape", + "already_a_file.dat", + Path(tmp_test_directory) / "dummy.cfg", + None, + ) + + assert result == "already_a_file.dat" + + def test_get_sim_telarray_metadata_with_model_parameters(simtel_config_writer): model_parameters = {"test_param": {"value": 42, "meta_parameter": True}} @@ -845,7 +878,7 @@ def _read_pulse_table(path: Path): def test_write_light_pulse_table_gauss_exp_conv_creates_normalized_file(tmp_test_directory): """Writer should create a pulse table with peak amplitude ~1 and expected window.""" out = Path(tmp_test_directory) / "pulse_shape_test.dat" - result = SimtelConfigWriter.write_light_pulse_table_gauss_exp_conv( + result = simtel_table_writer.write_light_pulse_table_gauss_exp_conv( file_path=out, width_ns=2.5, exp_decay_ns=5.0, @@ -876,7 +909,7 @@ def test_write_light_pulse_table_gauss_exp_conv_time_spacing(tmp_test_directory) """Time column should be spaced by dt_ns consistently.""" out = Path(tmp_test_directory) / "pulse_shape_spacing.dat" dt = 0.5 - SimtelConfigWriter.write_light_pulse_table_gauss_exp_conv( + simtel_table_writer.write_light_pulse_table_gauss_exp_conv( file_path=out, width_ns=3.0, exp_decay_ns=6.0, @@ -892,7 +925,7 @@ def test_write_light_pulse_table_gauss_exp_conv_missing_params_raises(tmp_test_d """Missing width/decay should raise ValueError.""" out = Path(tmp_test_directory) / "pulse_missing_params.dat" with pytest.raises(ValueError, match="width_ns"): - SimtelConfigWriter.write_light_pulse_table_gauss_exp_conv( + simtel_table_writer.write_light_pulse_table_gauss_exp_conv( file_path=out, width_ns=None, exp_decay_ns=5.0, @@ -901,12 +934,12 @@ def test_write_light_pulse_table_gauss_exp_conv_missing_params_raises(tmp_test_d def test_write_ascii_pulse_table_writes_header_and_values(tmp_test_directory): - """_write_ascii_pulse_table should create a two-column file with a header and values.""" + """write_ascii_pulse_table should create a two-column file with a header and values.""" out = Path(tmp_test_directory) / "pulse_ascii.dat" t = np.array([0.0, 0.1, 0.2]) y = np.array([0.25, 1.0, 0.5]) - result = SimtelConfigWriter._write_ascii_pulse_table(out, t, y) + result = simtel_table_writer.write_ascii_pulse_table(out, t, y) assert result == out assert out.exists() @@ -1091,7 +1124,7 @@ def test_write_angular_distribution_table_lambertian(tmp_test_directory): file_path = Path(tmp_test_directory) / "lambertian.dat" # Test default parameters - SimtelConfigWriter.write_angular_distribution_table_lambertian( + simtel_table_writer.write_angular_distribution_table_lambertian( file_path=file_path, max_angle_deg=90.0, n_samples=100, @@ -1120,7 +1153,7 @@ def test_write_angular_distribution_table_lambertian(tmp_test_directory): # Test with max_angle > 90 (should be clipped to 0) file_path_large = Path(tmp_test_directory) / "lambertian_large.dat" - SimtelConfigWriter.write_angular_distribution_table_lambertian( + simtel_table_writer.write_angular_distribution_table_lambertian( file_path=file_path_large, max_angle_deg=180.0, n_samples=181, diff --git a/tests/unit_tests/simtel/test_simtel_output_validator.py b/tests/unit_tests/simtel/test_simtel_output_validator.py index f08cb543a0..6566495ad7 100644 --- a/tests/unit_tests/simtel/test_simtel_output_validator.py +++ b/tests/unit_tests/simtel/test_simtel_output_validator.py @@ -12,6 +12,7 @@ _assert_sim_telarray_seed, _is_equal_floats_or_ints, _item_to_check_from_sim_telarray, + _resolve_dict_parameter_metadata_value, _sim_telarray_name_from_parameter_name, assert_events_of_type, assert_expected_sim_telarray_metadata, @@ -224,6 +225,93 @@ def test_missing_parameter_in_metadata(): assert len(result) == 0 +def test_assert_model_parameters_resolves_dict_metadata_file(tmp_test_directory): + """Resolve dict-valued metadata filenames before comparing to model values.""" + metadata = {"fadc_pulse_shape": "fadc_pulse_shape-LSTN-01.dat"} + model_mock = MagicMock() + model_mock.parameters = { + "fadc_pulse_shape": { + "value": { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.0], [0.1, 0.2]], + }, + "type": "dict", + }, + } + model_directory = Path(tmp_test_directory) / "model" + model_mock.config_file_directory = model_directory + + with patch( + "simtools.simtel.simtel_output_validator.simtel_table_reader.resolve_dict_parameter_value" + ) as resolve_mock: + resolve_mock.return_value = model_mock.parameters["fadc_pulse_shape"]["value"] + + result = _assert_model_parameters(metadata, model_mock) + + resolve_mock.assert_called_once_with( + "fadc_pulse_shape-LSTN-01.dat", + "fadc_pulse_shape", + data_path=model_directory, + ) + assert len(result) == 0 + + +def test_resolve_dict_parameter_metadata_value_returns_input_for_non_dict_type(): + """Keep metadata value unchanged when parameter type is not dict.""" + model = MagicMock() + value = "fadc_pulse_shape.dat" + + result = _resolve_dict_parameter_metadata_value( + value=value, + model_value={"columns": ["time"], "rows": [[0.0]]}, + parameter_type="string", + param="fadc_pulse_shape", + model=model, + ) + + assert result == value + + +def test_resolve_dict_parameter_metadata_value_returns_input_for_non_string_value(): + """Keep metadata value unchanged for dict parameters with non-string metadata values.""" + model = MagicMock() + value = {"columns": ["time"], "rows": [[0.0]]} + + result = _resolve_dict_parameter_metadata_value( + value=value, + model_value={"columns": ["time"], "rows": [[0.0]]}, + parameter_type="dict", + param="fadc_pulse_shape", + model=model, + ) + + assert result == value + + +def test_resolve_dict_parameter_metadata_value_returns_input_when_resolution_fails( + tmp_test_directory, +): + """Return original metadata value when table resolution raises expected exceptions.""" + model = MagicMock() + model.config_file_directory = Path(tmp_test_directory) + value = "missing_file.dat" + + with patch( + "simtools.simtel.simtel_output_validator.simtel_table_reader.resolve_dict_parameter_value" + ) as resolve_mock: + resolve_mock.side_effect = FileNotFoundError("missing") + result = _resolve_dict_parameter_metadata_value( + value=value, + model_value={"columns": ["time"], "rows": [[0.0]]}, + parameter_type="dict", + param="fadc_pulse_shape", + model=model, + ) + + assert result == value + + def test_telescope_count_mismatch(tmp_path): """Test error when telescope count mismatches.""" sim_file = tmp_path / "test.simtel.zst" diff --git a/tests/unit_tests/simtel/test_simtel_table_reader.py b/tests/unit_tests/simtel/test_simtel_table_reader.py index dae92b2efd..63b6b6c871 100644 --- a/tests/unit_tests/simtel/test_simtel_table_reader.py +++ b/tests/unit_tests/simtel/test_simtel_table_reader.py @@ -1,10 +1,12 @@ #!/usr/bin/python3 import logging +from pathlib import Path from unittest import mock import astropy.units as u import pytest +from astropy.table import Table from astropy.tests.helper import assert_quantity_allclose import simtools.simtel.simtel_table_reader as simtel_table_reader @@ -100,6 +102,134 @@ def test_read_simtel_table_to_table(spe_test_file, spe_meta_test_comment): mock_read.assert_called_once() +def test_read_simtel_table_as_row_data(): + table = Table( + { + "time": [0.0, 0.12], + "amplitude": [0.0, 0.01323], + "amplitude (low gain)": [0.0, 0.000945], + } + ) + table["time"].unit = u.ns + table["amplitude"].unit = u.dimensionless_unscaled + table["amplitude (low gain)"].unit = u.dimensionless_unscaled + + with mock.patch("simtools.simtel.simtel_table_reader.read_simtel_table", return_value=table): + result = simtel_table_reader.read_simtel_table_as_row_data("fadc_pulse_shape", "dummy.dat") + + assert result == { + "columns": ["time", "amplitude", "amplitude (low gain)"], + "column_units": ["ns", "dimensionless", "dimensionless"], + "rows": [ + [0.0, 0.0, 0.0], + [0.12, 0.01323, 0.000945], + ], + } + + +def test_resolve_dict_parameter_value_from_inline_json(tmp_test_directory): + value = '{"columns": ["time"], "dtype": ["float64"], "unit": ["ns"], "data": {"time": [0.0]}}' + + with mock.patch( + "simtools.simtel.simtel_table_reader.read_simtel_table_as_row_data" + ) as read_mock: + result = simtel_table_reader.resolve_dict_parameter_value( + value, + "fadc_pulse_shape", + str(tmp_test_directory), + ) + + read_mock.assert_not_called() + assert isinstance(result, dict) + assert result["columns"] == ["time"] + + +def test_resolve_dict_parameter_value_from_file_path(tmp_test_directory): + with mock.patch( + "simtools.simtel.simtel_table_reader.read_simtel_table_as_row_data", + return_value={"columns": ["time"]}, + ) as read_mock: + result = simtel_table_reader.resolve_dict_parameter_value( + "pulse.dat", + "fadc_pulse_shape", + str(tmp_test_directory), + ) + + read_mock.assert_called_once_with("fadc_pulse_shape", Path(tmp_test_directory) / "pulse.dat") + assert result == {"columns": ["time"]} + + +def test_row_data_to_astropy_table_returns_correct_table(): + row_data = { + "columns": ["time", "amplitude"], + "column_units": ["ns", "dimensionless"], + "rows": [[0.0, 0.0], [0.5, 0.12], [1.0, 0.48]], + } + table = simtel_table_reader.row_data_to_astropy_table(row_data) + + assert list(table.colnames) == ["time", "amplitude"] + assert len(table) == 3 + assert table["time"][1] == pytest.approx(0.5) + assert table["amplitude"][2] == pytest.approx(0.48) + assert str(table["time"].unit) == "ns" + assert table["amplitude"].unit == u.dimensionless_unscaled + + +def test_row_data_to_astropy_table_raises_on_missing_keys(): + with pytest.raises(ValueError, match="'columns' and 'rows'"): + simtel_table_reader.row_data_to_astropy_table({"columns": ["time"]}) + + +def test_row_data_to_astropy_table_raises_on_wrong_type(): + with pytest.raises(ValueError, match="'columns' and 'rows'"): + simtel_table_reader.row_data_to_astropy_table("not a dict") + + +def test_resolve_dict_parameter_value_raises_without_column_units(): + with pytest.raises(ValueError, match="column_units"): + simtel_table_reader.resolve_dict_parameter_value( + {"columns": ["time"], "rows": [[0.0]]}, + "fadc_pulse_shape", + ) + + +def test_resolve_dict_parameter_value_invalid_inline_json_falls_back_to_file( + tmp_test_directory, +): + """Fallback to file-path resolution if inline JSON parsing fails.""" + with mock.patch( + "simtools.simtel.simtel_table_reader.read_simtel_table_as_row_data", + return_value={"columns": ["time"], "column_units": ["ns"], "rows": [[0.0]]}, + ) as read_mock: + result = simtel_table_reader.resolve_dict_parameter_value( + "{not valid json}", + "fadc_pulse_shape", + data_path=tmp_test_directory, + ) + + read_mock.assert_called_once_with( + "fadc_pulse_shape", + Path(tmp_test_directory) / "{not valid json}", + ) + assert result["columns"] == ["time"] + + +def test_resolve_dict_parameter_value_non_string_non_dict_uses_file_reader(tmp_test_directory): + """Resolve Path-like input values through table-file reader path.""" + with mock.patch( + "simtools.simtel.simtel_table_reader.read_simtel_table_as_row_data", + return_value={"columns": ["time"], "column_units": ["ns"], "rows": [[0.0]]}, + ) as read_mock: + result = simtel_table_reader.resolve_dict_parameter_value( + Path("pulse.dat"), + "fadc_pulse_shape", + data_path=tmp_test_directory, + ) + + read_mock.assert_called_once_with("fadc_pulse_shape", Path(tmp_test_directory) / "pulse.dat") + assert result["rows"] == [[0.0]] + + def test_data_simple_columns(): columns = [ "pm_photoelectron_spectrum", diff --git a/tests/unit_tests/simtel/test_simtel_table_writer.py b/tests/unit_tests/simtel/test_simtel_table_writer.py new file mode 100644 index 0000000000..8fc7c46762 --- /dev/null +++ b/tests/unit_tests/simtel/test_simtel_table_writer.py @@ -0,0 +1,93 @@ +#!/usr/bin/python3 + +import pytest + +import simtools.simtel.simtel_table_writer as simtel_table_writer + + +def test_write_simtel_table_two_columns(tmp_test_directory): + value = { + "columns": ["time", "amplitude"], + "rows": [[-1.0, 0.0], [0.0, 0.5], [1.0, 1.0]], + } + result = simtel_table_writer.write_simtel_table( + "fadc_pulse_shape", value, tmp_test_directory, "LSTN-01" + ) + + assert result == "fadc_pulse_shape-LSTN-01.dat" + out_file = tmp_test_directory / result + assert out_file.exists() + lines = out_file.read_text(encoding="utf-8").splitlines() + assert lines[0] == "# time amplitude" + assert lines[1] == "-1.0 0.0" + assert lines[2] == "0.0 0.5" + assert lines[3] == "1.0 1.0" + + +def test_write_simtel_table_three_columns(tmp_test_directory): + value = { + "columns": ["time", "amplitude", "amplitude (low gain)"], + "rows": [[0.0, 1.0, 0.5], [1.0, 0.0, 0.0]], + } + result = simtel_table_writer.write_simtel_table( + "fadc_pulse_shape", value, tmp_test_directory, "MSTN-05" + ) + + assert result == "fadc_pulse_shape-MSTN-05.dat" + out_file = tmp_test_directory / result + lines = out_file.read_text(encoding="utf-8").splitlines() + assert lines[0] == "# time amplitude amplitude (low gain)" + assert lines[1] == "0.0 1.0 0.5" + + +def test_write_simtel_table_raises_on_non_dict(tmp_test_directory): + with pytest.raises(ValueError, match="'columns' and 'rows' keys"): + simtel_table_writer.write_simtel_table( + "fadc_pulse_shape", "some_file.dat", tmp_test_directory, "LSTN-01" + ) + + +def test_write_simtel_table_raises_on_missing_rows_key(tmp_test_directory): + with pytest.raises(ValueError, match="'columns' and 'rows' keys"): + simtel_table_writer.write_simtel_table( + "fadc_pulse_shape", + {"columns": ["time", "amplitude"]}, + tmp_test_directory, + "LSTN-01", + ) + + +def test_write_simtel_table_raises_on_row_length_mismatch(tmp_test_directory): + value = { + "columns": ["time", "amplitude"], + "rows": [[0.0], [1.0, 0.5]], + } + + with pytest.raises(ValueError, match="invalid row length"): + simtel_table_writer.write_simtel_table( + "fadc_pulse_shape", value, tmp_test_directory, "LSTN-01" + ) + + +def test_write_simtel_table_raises_on_non_sequence_row(tmp_test_directory): + value = { + "columns": ["time", "amplitude"], + "rows": [0.0, [1.0, 0.5]], + } + + with pytest.raises(ValueError, match="invalid row"): + simtel_table_writer.write_simtel_table( + "fadc_pulse_shape", value, tmp_test_directory, "LSTN-01" + ) + + +def test_write_simtel_table_raises_on_non_numeric_row_value(tmp_test_directory): + value = { + "columns": ["time", "amplitude"], + "rows": [[0.0, "bad_value"]], + } + + with pytest.raises(ValueError, match="non-real-numeric value"): + simtel_table_writer.write_simtel_table( + "fadc_pulse_shape", value, tmp_test_directory, "LSTN-01" + ) diff --git a/tests/unit_tests/simtel/test_simulator_light_emission.py b/tests/unit_tests/simtel/test_simulator_light_emission.py index 774a620918..e9e043dc3d 100644 --- a/tests/unit_tests/simtel/test_simulator_light_emission.py +++ b/tests/unit_tests/simtel/test_simulator_light_emission.py @@ -220,7 +220,7 @@ def test__get_pulse_shape_argument_for_sim_telarray_gauss_exp_dat_file( with patch( "simtools.simtel.simulator_light_emission." - "SimtelConfigWriter.write_light_pulse_table_gauss_exp_conv" + "simtel_table_writer.write_light_pulse_table_gauss_exp_conv" ) as mock_writer: result = simulator_instance._get_pulse_shape_argument_for_sim_telarray() @@ -237,7 +237,7 @@ def test__get_pulse_shape_argument_for_sim_telarray_gauss_exp_dat_file( def test__get_pulse_shape_argument_for_sim_telarray_gauss_exp_failure(simulator_instance): - """Test that Gauss-Exponential raises error when DAT file writing fails.""" + """Test that Gauss-Exponential fails gracefully and logs warning when DAT file writing fails.""" # Mock Gauss-Exponential pulse shape simulator_instance.calibration_model.get_parameter_value.return_value = [ "Gauss-Exponential", @@ -258,9 +258,16 @@ def test__get_pulse_shape_argument_for_sim_telarray_gauss_exp_failure(simulator_ "light_source": "NectarCam", } - # Should raise ValueError when DAT writing fails - with pytest.raises(ValueError, match="Failed to write Gauss-Exponential pulse shape table"): - simulator_instance._get_pulse_shape_argument_for_sim_telarray() + # Should log warning and return token string instead of raising + result = simulator_instance._get_pulse_shape_argument_for_sim_telarray() + + # Verify warning was logged + simulator_instance._logger.warning.assert_called_once() + assert "Failed to write pulse shape table" in simulator_instance._logger.warning.call_args[0][0] + + # Verify token string was returned instead of raising + assert isinstance(result, str) + assert result == "gauss-exponential" def test__add_illuminator_command_options(simulator_instance): @@ -730,7 +737,7 @@ def test__add_flasher_command_options_with_pulse_table(simulator_instance, tmp_t ), patch( "simtools.simtel.simulator_light_emission." - "SimtelConfigWriter.write_light_pulse_table_gauss_exp_conv" + "simtel_table_writer.write_light_pulse_table_gauss_exp_conv" ) as mock_writer, ): mock_distance_value = Mock() @@ -766,6 +773,99 @@ def test__add_flasher_command_options_with_pulse_table(simulator_instance, tmp_t ) +def test__add_flasher_command_options_writer_fallback(simulator_instance, tmp_test_directory): + """If pulse table writing fails, a warning is logged and token is used.""" + + # Calibration parameters + def mock_get_param_with_unit(name): + if name == "flasher_position": + return [1.0 * u.cm, -1.0 * u.cm] + if name == "flasher_wavelength": + return 420.0 * u.nm + if name == "flasher_pulse_shape": + return ["Gauss-Exponential", 2.0, 6.0] + return None + + simulator_instance.calibration_model.get_parameter_value_with_unit.side_effect = ( + mock_get_param_with_unit + ) + + # Provide specific returns for plain-valued params used inside the call + def mock_get_param(name): + if name == "flasher_bunch_size": + return 4000 + if name == "flasher_pulse_shape": + return ["Gauss-Exponential", 2.0, 6.0] + return None + + simulator_instance.calibration_model.get_parameter_value.side_effect = mock_get_param + + # Telescope parameters + mock_diameter = Mock() + mock_diameter.to.return_value.value = 160.0 + simulator_instance.telescope_model.get_parameter_value_with_unit.return_value = mock_diameter + simulator_instance.telescope_model.get_parameter_value.side_effect = lambda key: ( + 40 if key == "fadc_sum_bins" else "hexagonal" + ) + + # IO and helpers + pulse_dir = Path(tmp_test_directory) / "pulse_shapes" + pulse_dir.mkdir(parents=True, exist_ok=True) + io_mock = Mock() + io_mock.get_output_directory.return_value = pulse_dir + simulator_instance.io_handler = io_mock + + # Distance and other string helpers + with ( + patch.object( + simulator_instance, "calculate_distance_focal_plane_calibration_device" + ) as mock_distance, + patch( + "simtools.simtel.simulator_light_emission.fiducial_radius_from_shape", + return_value=75.0, + ), + patch.object( + simulator_instance, + "_get_angular_distribution_string_for_sim_telarray", + return_value="uniform", + ), + patch.object( + simulator_instance, + "_get_pulse_shape_string_token", + return_value="gauss-exponential-token", + ), + patch( + "simtools.simtel.simulator_light_emission." + "simtel_table_writer.write_light_pulse_table_gauss_exp_conv", + side_effect=OSError("boom"), + ), + ): + mock_distance_value = Mock() + mock_distance_value.to.return_value.value = 900.0 + mock_distance.return_value = mock_distance_value + + simulator_instance.light_emission_config = { + "number_of_events": 5, + "flasher_photons": 250000, + "telescope": "LSTN-01", + "light_source": "NectarCam", + } + + result = simulator_instance._add_flasher_command_options() + + # Expect a warning via the instance logger + assert simulator_instance._logger.warning.called + assert any( + "Failed to write pulse shape table" in str(call.args[0]) + for call in simulator_instance._logger.warning.mock_calls + ) + + # Fallback token should be used for --lightpulse, not a .dat file + lightpulse_args = [arg for arg in result if str(arg).startswith("--lightpulse ")] + assert len(lightpulse_args) == 1 + assert lightpulse_args[0] == "--lightpulse gauss-exponential-token" + + def test__add_flasher_command_options_invalid_gauss_exponential_width(simulator_instance): """Gauss-Exponential with non-positive width must raise ValueError.""" diff --git a/tests/unit_tests/visualization/test_plot_tables.py b/tests/unit_tests/visualization/test_plot_tables.py index d9141cdc26..14325c57dc 100644 --- a/tests/unit_tests/visualization/test_plot_tables.py +++ b/tests/unit_tests/visualization/test_plot_tables.py @@ -365,13 +365,17 @@ def test_generate_output_file_name( @mock.patch("simtools.visualization.plot_tables._read_table_from_model_database") +@mock.patch("simtools.visualization.plot_tables._read_parameter_dict_from_model_database") @mock.patch("simtools.visualization.plot_tables.ascii_handler.collect_data_from_file") -def test_generate_plot_configurations(mock_collect_data, mock_read_table, tmp_test_directory): +def test_generate_plot_configurations( + mock_collect_data, mock_read_parameter_dict, mock_read_table, tmp_test_directory +): # Mock the table data mock_table = Table() mock_table["time"] = [1.0, 2.0, 3.0] mock_table["amplitude"] = [0.1, 0.2, 0.3] mock_read_table.return_value = mock_table + mock_read_parameter_dict.return_value = {"model_parameter_schema_version": "0.2.0"} # Test with parameter that has no plot configuration mock_collect_data.return_value = {} @@ -473,8 +477,9 @@ def test_get_plotting_label_multiple_duplicates(): @mock.patch("simtools.visualization.plot_tables.ascii_handler.collect_data_from_file") @mock.patch("simtools.visualization.plot_tables._read_table_from_model_database") +@mock.patch("simtools.visualization.plot_tables._read_parameter_dict_from_model_database") def test_generate_plot_configurations_with_nan_and_missing_columns( - mock_read_table, mock_collect_data, tmp_test_directory + mock_read_parameter_dict, mock_read_table, mock_collect_data, tmp_test_directory ): """Test handling of NaN values and missing columns in generate_plot_configurations.""" # Create mock table with valid and NaN columns @@ -484,6 +489,7 @@ def test_generate_plot_configurations_with_nan_and_missing_columns( mock_table["amplitude_low_gain"] = [np.nan, np.nan, np.nan] mock_read_table.return_value = mock_table + mock_read_parameter_dict.return_value = {"model_parameter_schema_version": "0.2.0"} # Mock schema with multiple table configs including one with missing column mock_schema = { @@ -548,3 +554,65 @@ def test_generate_plot_configurations_with_nan_and_missing_columns( # Should return None since no valid configs were found for this plot type assert result is None + + +@mock.patch("simtools.visualization.plot_tables._read_table_from_model_database") +@mock.patch("simtools.visualization.plot_tables._read_parameter_dict_from_model_database") +@mock.patch("simtools.visualization.plot_tables.ascii_handler.collect_data_from_file") +def test_generate_plot_configurations_selects_schema_matching_parameter_version( + mock_collect_data, mock_read_parameter_dict, mock_read_table, tmp_test_directory +): + """Select the schema entry matching model_parameter_schema_version from a list.""" + mock_table = Table() + mock_table["time"] = [1.0, 2.0, 3.0] + mock_table["amplitude"] = [0.1, 0.2, 0.3] + mock_read_table.return_value = mock_table + mock_read_parameter_dict.return_value = {"model_parameter_schema_version": "0.1.0"} + + mock_collect_data.return_value = [ + { + "schema_version": "0.1.0", + "plot_configuration": [ + {"type": "legacy_plot", "tables": [{"column_x": "time", "column_y": "amplitude"}]} + ], + }, + { + "schema_version": "0.2.0", + "plot_configuration": [ + { + "type": "fadc_pulse_shape", + "tables": [{"column_x": "time", "column_y": "amplitude"}], + } + ], + }, + ] + + configs, output_files = plot_tables.generate_plot_configurations( + parameter="fadc_pulse_shape", + parameter_version="1.0.0", + site="South", + telescope="SSTS-design", + output_path=tmp_test_directory, + plot_type="legacy_plot", + ) + + assert len(configs) == 1 + assert configs[0]["type"] == "legacy_plot" + assert len(output_files) == 1 + + +def test_select_schema_entry_returns_latest_when_version_not_found(): + """Fall back to newest schema entry when requested version is unavailable.""" + schema_data = [ + {"schema_version": "0.1.0", "plot_configuration": [{"type": "old"}]}, + {"schema_version": "0.3.0", "plot_configuration": [{"type": "new"}]}, + ] + + result = plot_tables._select_schema_entry(schema_data, schema_version="0.2.0") + + assert result["schema_version"] == "0.3.0" + + +def test_select_schema_entry_returns_empty_dict_for_empty_input(): + """Return an empty dict when schema input is empty/invalid.""" + assert plot_tables._select_schema_entry([], schema_version="0.2.0") == {}