Skip to content

Allow model parameter tables to be written with different file names (and more)#2140

Merged
GernotMaier merged 19 commits into
mainfrom
allow-to-set-output-file
May 4, 2026
Merged

Allow model parameter tables to be written with different file names (and more)#2140
GernotMaier merged 19 commits into
mainfrom
allow-to-set-output-file

Conversation

@GernotMaier
Copy link
Copy Markdown
Contributor

@GernotMaier GernotMaier commented Apr 27, 2026

A couple of changes to simplify and improve the model parameter setting workflows:

  • simplify the change from files to dict-based json tables for the model parameters: allow to rename the file written to disk when we pull a file-based model parameter (e.g. fadc_pulse_shape) from the DB.
  • catch metadata on top of original sim-telarray files (in # comment fields) and add it the model parameter metadata
  • make sure that output files can be written to tmp dir (also those used by plot_tabular_data)
  • avoid duplicated log files when using simtools-run-application

All files in https://gitlab.cta-observatory.org/cta-science/simulations/simulation-model/simulation-model-parameter-setting/-/merge_requests/31 are generated with the code in this branch.

@GernotMaier GernotMaier self-assigned this Apr 27, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves model-parameter workflows by (1) allowing file-backed model parameter exports to be written under a user-chosen filename, (2) propagating sim_telarray header/comment context into collected metadata, and (3) making “workflow runner” and plotting behavior more robust when writing into temporary/output directories.

Changes:

  • Allow output_file to override the exported filename for file-backed parameters (and update CLI/docs/tests accordingly).
  • Capture sim_telarray header # comment metadata into table meta (context_from_sim_telarray) and propagate it into model parameter metadata notes.
  • Add db_export_path support for plot_tabular_data DB-backed table exports, and disable per-application log files when running via simtools-run-application.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/simtools/db/parameter_exporter.py Enables overriding exported filenames for file-backed parameters (via rename).
src/simtools/data_model/metadata_collector.py Adds extraction of context_from_sim_telarray from .ecsv value files into metadata notes.
src/simtools/simtel/simtel_table_reader.py Standardizes table meta key to context_from_sim_telarray.
src/simtools/visualization/plot_tables.py Adds optional db_export_path to export DB-backed files into a chosen directory during table reading.
src/simtools/schemas/plot_configuration.metaschema.yml Extends plot configuration schema with db_export_path.
src/simtools/runners/simtools_runner.py Preserves existing output_path in configs; disables file logging for sub-apps; adjusts metadata propagation timing.
src/simtools/application_control.py Adds disable_log_file switch to suppress log file creation.
src/simtools/configuration/commandline_parser.py Introduces hidden --disable_log_file CLI argument.
src/simtools/applications/db_get_parameter_from_db.py Updates user-facing docs/examples for overridden file-backed exports.
src/simtools/applications/submit_model_parameter_from_external.py Minor formatting change.
tests/unit_tests/db/test_parameter_exporter.py Updates coverage for file-backed output_file override behavior.
tests/unit_tests/data_model/test_metadata_collector.py Adds tests for extracting simtel context from .ecsv metadata into notes.
tests/unit_tests/simtel/test_simtel_table_reader.py Updates tests to match new meta key casing.
tests/unit_tests/visualization/test_plot_tables.py Adds test for db_export_path temporary output path override.
tests/unit_tests/runners/test_simtools_runner.py Updates tests for runner output path preservation and disabled log files.
tests/unit_tests/test_application_control.py Adds unit test for disabling log file creation.
docs/changes/2140.feature.md Changelog fragment describing the new behavior and metadata improvements.
Comments suppressed due to low confidence (1)

src/simtools/db/parameter_exporter.py:190

  • Allowing output_file to rename file-backed exports without validating the suffix can produce misleading outputs (e.g., renaming a .dat payload to .ecsv without conversion) and also suppresses the optional ECSV table export because the suffix check thinks the exported file is already ECSV. It would be safer to either (a) require output_file to keep the original suffix, or (b) apply the source suffix when output_file has none and reject .ecsv for raw file exports. Also consider using an overwrite-safe move (replace) or explicitly handling an existing destination path to avoid platform-dependent rename behavior.
    source_file = db.io_handler.get_output_file(par_info["value"])
    table_file = source_file
    if output_file is not None:
        table_file = db.io_handler.get_output_file(output_file)
        if table_file != source_file:
            source_file.rename(table_file)
    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)

Comment thread src/simtools/data_model/metadata_collector.py Outdated
Comment thread src/simtools/runners/simtools_runner.py Outdated
Comment thread src/simtools/applications/db_get_parameter_from_db.py Outdated
GernotMaier and others added 3 commits April 28, 2026 16:03
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@GernotMaier GernotMaier changed the title Allow model parameter tables to be written with different file names Allow model parameter tables to be written with different file names (and more) Apr 28, 2026
@GernotMaier GernotMaier marked this pull request as ready for review April 28, 2026 14:46
@GernotMaier GernotMaier marked this pull request as draft April 29, 2026 09:31
@GernotMaier GernotMaier marked this pull request as draft April 29, 2026 09:31
@GernotMaier
Copy link
Copy Markdown
Contributor Author

@EshitaJoshi - please wait with the review. I pushed (again) something accidentally into the wrong branch.

@GernotMaier GernotMaier marked this pull request as ready for review May 4, 2026 07:41
@GernotMaier
Copy link
Copy Markdown
Contributor Author

@EshitaJoshi - this one is now ready for review!

@ctao-sonarqube
Copy link
Copy Markdown

ctao-sonarqube Bot commented May 4, 2026

Copy link
Copy Markdown
Collaborator

@EshitaJoshi EshitaJoshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks!

@GernotMaier
Copy link
Copy Markdown
Contributor Author

@EshitaJoshi - does 'Looks good' mean 'Approved'?

@EshitaJoshi
Copy link
Copy Markdown
Collaborator

@GernotMaier Approved!

@GernotMaier
Copy link
Copy Markdown
Contributor Author

Thanks!

@GernotMaier GernotMaier merged commit 8fadf38 into main May 4, 2026
16 checks passed
@GernotMaier GernotMaier deleted the allow-to-set-output-file branch May 4, 2026 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants