Save pulse shape parameters directly in json (and not in a table file)#2088
Open
GernotMaier wants to merge 44 commits intomainfrom
Open
Save pulse shape parameters directly in json (and not in a table file)#2088GernotMaier wants to merge 44 commits intomainfrom
GernotMaier wants to merge 44 commits intomainfrom
Conversation
Pulse shape as value b
Contributor
There was a problem hiding this comment.
Pull request overview
Enable storing table-like model parameter values directly inside model-parameter JSON (initially for fadc_pulse_shape), while unifying export/validation logic across file-backed and dict-backed parameters and selecting schema entries by model_parameter_schema_version.
Changes:
- Add row-oriented table serialization + table-file writer/reader utilities and wire them into sim_telarray config generation and validation flows.
- Introduce
parameter_exportermodule and delegate DB export logic to it (including dict-backed → ECSV export support). - Update schemas, CLI apps, docs, reference sim_telarray configs, and unit tests for schema-version selection and dict-backed table handling.
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit_tests/visualization/test_plot_tables.py | Update plotting tests for schema-version selection and parameter-dict lookup. |
| tests/unit_tests/simtel/test_simulator_light_emission.py | Update patches to use new simtel_table_writer functions. |
| tests/unit_tests/simtel/test_simtel_table_writer.py | Add unit tests for the new sim_telarray table writer. |
| tests/unit_tests/simtel/test_simtel_table_reader.py | Add tests for row-data serialization and dict-value resolution helpers. |
| tests/unit_tests/simtel/test_simtel_output_validator.py | Add tests ensuring dict-typed metadata values can be resolved from table files. |
| tests/unit_tests/simtel/test_simtel_config_writer.py | Update tests to use new writer functions; add tests for new conversion helpers. |
| tests/unit_tests/model/test_model_parameter.py | Add coverage for legacy table-value resolution during schema upgrades. |
| tests/unit_tests/model/test_legacy_model_parameter.py | Add/extend legacy migration handlers for fadc_pulse_shape and resolver plumbing. |
| tests/unit_tests/db/test_parameter_exporter.py | Add unit tests for the new DB parameter export orchestration module. |
| tests/unit_tests/db/test_db_handler.py | Update DB handler tests for delegation and dict-backed export behavior. |
| tests/unit_tests/data_model/test_model_data_writer.py | Extend writer tests for embedded fadc_pulse_shape and schema-version type lookup. |
| tests/resources/sim_telarray_configurations/6.0.2/CTA-South-SSTS-01_test.cfg | Update expected fadc_pulse_shape filename and config_release. |
| tests/resources/sim_telarray_configurations/6.0.2/CTA-South-MSTS-01_test.cfg | Update expected fadc_pulse_shape filename and config_release. |
| tests/resources/sim_telarray_configurations/6.0.2/CTA-South-LSTS-01_test.cfg | Update expected fadc_pulse_shape filename and config_release. |
| tests/resources/sim_telarray_configurations/6.0.2/CTA-North-MSTN-01_test.cfg | Update expected fadc_pulse_shape filename and config_release. |
| tests/resources/sim_telarray_configurations/6.0.2/CTA-North-LSTN-02_test.cfg | Update expected fadc_pulse_shape filename and config_release. |
| tests/resources/sim_telarray_configurations/6.0.2/CTA-North-LSTN-01_test.cfg | Update expected fadc_pulse_shape filename and config_release. |
| tests/resources/sim_telarray_configurations/5.0.0/CTA-South-SSTS-01_test.cfg | Update expected fadc_pulse_shape filename and config_release. |
| tests/resources/sim_telarray_configurations/5.0.0/CTA-South-MSTS-01_test.cfg | Update expected fadc_pulse_shape filename and config_release. |
| tests/resources/sim_telarray_configurations/5.0.0/CTA-South-LSTS-01_test.cfg | Update expected fadc_pulse_shape filename and config_release. |
| tests/resources/sim_telarray_configurations/5.0.0/CTA-North-MSTN-01_test.cfg | Update expected fadc_pulse_shape filename and config_release. |
| tests/resources/sim_telarray_configurations/5.0.0/CTA-North-LSTN-02_test.cfg | Update expected fadc_pulse_shape filename and config_release. |
| tests/resources/sim_telarray_configurations/5.0.0/CTA-North-LSTN-01_test.cfg | Update expected fadc_pulse_shape filename and config_release. |
| src/simtools/visualization/plot_tables.py | Select schema entry by parameter’s schema version before generating plot configs. |
| src/simtools/simtel/simulator_light_emission.py | Switch pulse/angular-distribution table writing to simtel_table_writer. |
| src/simtools/simtel/simtel_table_writer.py | New module for writing sim_telarray table files (pulse/angle/generic tables). |
| src/simtools/simtel/simtel_table_reader.py | Add row-data serialization, unit normalization, and dict-value resolution helpers. |
| src/simtools/simtel/simtel_output_validator.py | Resolve dict-typed metadata values from table files prior to comparison. |
| src/simtools/simtel/simtel_config_writer.py | Remove embedded table-writer methods; write dict table parameters via new writer. |
| src/simtools/schemas/model_parameters/fadc_pulse_shape.schema.yml | Add schema v0.2.0 for embedded row-data representation and plot configuration. |
| src/simtools/model/model_parameter.py | Pass legacy value resolver into schema-upgrade path and implement table-value resolver. |
| src/simtools/model/legacy_model_parameter.py | Add value_resolver plumbing + fadc_pulse_shape migration to embedded row-data. |
| src/simtools/io/ascii_handler.py | Update JSON writing and add compact encoding for numeric lists (row tables). |
| src/simtools/db/parameter_exporter.py | New shared export logic for file-backed and dict-backed parameter payloads. |
| src/simtools/db/db_handler.py | Delegate export operations to parameter_exporter; add export_parameter_data. |
| src/simtools/data_model/model_data_writer.py | Write metadata only after successful validation; add schema-version type lookup. |
| src/simtools/applications/submit_model_parameter_from_external.py | Add schema-version arg and pre-resolve dict parameter values for submission. |
| src/simtools/applications/db_get_parameter_from_db.py | Rework CLI export modes; use export_parameter_data for unified exports. |
| src/simtools/applications/db_get_file_from_db.py | Improve error handling by raising a clear FileNotFoundError with context. |
| docs/source/api-reference/sim_telarray.md | Add API docs entry for simtel_table_writer. |
| docs/source/api-reference/db_handler.md | Add API docs entry for db.parameter_exporter. |
| docs/changes/2088.feature.md | Add changelog entry describing unified export flow and schema selection changes. |
src/simtools/schemas/model_parameters/fadc_pulse_shape.schema.yml
Outdated
Show resolved
Hide resolved
- Move astropy.units import to top of row_table_utils.py to fix pylint c0415 - Move row_table_utils import to top of model_data_writer.py to fix pylint c0415 - All pre-commit hooks now pass - All 806 tests continue to pass
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.




This is a larger PR to allow the storage of tabled date directly in the model parameter json and not in externally linked files. Implemented here for the
fadc_pulse_shapeparameter but will be extended to all tabled data (and some items working for forfadc_pulse_shapewill be generalized.This pull request introduces a unified and more robust export flow for model parameter tables, improves schema selection and validation, and enhances documentation and CLI usability. The changes ensure stricter validation and normalization of table content, support for schema versioning, and clearer export logic for both file-backed and dict-backed parameters. The documentation and command-line interfaces are updated to reflect these improvements and provide better guidance to users.
Parameter export and validation improvements:
column_unitsforfadc_pulse_shape). The export logic now ensures consistent handling and output formats for all table-type parameters. [1] [2] [3]model_parameter_schema_version(with fallback to the latest version), and exposed schema version selection in CLI and application logic. [1] [2] [3] [4] [5]Database and model parameter handling:
parameter_exportermodule, aligning sim_telarray/model/database handling and expanding unit test coverage. [1] [2] [3]Documentation and CLI usability:
parameter_exporterandsimtel_table_writermodules. [1] [2]Model data writer enhancements:
Typical applications / steps:
(Requires to have the the corresponding simulation model setting branch checked out)
Requires to run with a test branch of simulation models: fadc-pulse-shape-format-change