Commandline configuration consolidation#2096
Merged
GernotMaier merged 17 commits intomainfrom Apr 9, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR consolidates command-line configuration across simtools applications by centralizing setup into simtools.application_control.build_application and standardizing argument names/types (notably introducing quantity-aware parameters like zenith_angle and source_distance). This aligns with issue #1055 by reducing duplication and inconsistent parameter definitions across the CLI surface.
Changes:
- Introduces
build_application()to encapsulate the commonConfigurator+startup_applicationflow and migrates many applications to it. - Standardizes CLI/config parameter names and units (e.g.,
zenith→zenith_angle,src_distance→source_distance,offsets→off_axis_angles,offset_steps→offset_step) and adds quantity parsing helpers. - Updates unit/integration tests and integration YAML configs to reflect the consolidated parameter names/types.
Reviewed changes
Copilot reviewed 73 out of 73 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/simtools/application_control.py | Adds build_application() wrapper to centralize application startup/configuration. |
| src/simtools/configuration/commandline_parser.py | Adds reusable application argument definitions and quantity parsing; streamlines argument-group initialization. |
| src/simtools/visualization/plot_psf.py | Switches to consolidated argument keys and supports Quantity-based offsets/steps. |
| src/simtools/ray_tracing/psf_parameter_optimisation.py | Updates ray-tracing invocation to use consolidated zenith_angle/source_distance. |
| src/simtools/ray_tracing/incident_angles.py | Makes source-distance handling robust to scalar vs Quantity inputs. |
| src/simtools/production_configuration/derive_production_statistics_handler.py | Renames offsets to off_axis_angles and normalizes to Quantity. |
| src/simtools/applications/validate_optics.py | Migrates to build_application() and consolidated argument keys/types. |
| src/simtools/applications/validate_file_using_schema.py | Migrates to build_application(). |
| src/simtools/applications/validate_cumulative_psf.py | Migrates to build_application() and consolidated zenith_angle/source_distance. |
| src/simtools/applications/validate_camera_fov.py | Migrates to build_application(). |
| src/simtools/applications/validate_camera_efficiency.py | Migrates to build_application(); keeps app-specific arg validation. |
| src/simtools/applications/submit_model_parameter_from_external.py | Migrates to build_application(). |
| src/simtools/applications/submit_data_from_external.py | Migrates to build_application(). |
| src/simtools/applications/submit_array_layouts.py | Migrates to build_application(). |
| src/simtools/applications/simulate_prod.py | Migrates to build_application() while keeping simulation configuration args. |
| src/simtools/applications/simulate_prod_htcondor_generator.py | Migrates to build_application(). |
| src/simtools/applications/simulate_pedestals.py | Migrates to build_application(). |
| src/simtools/applications/simulate_illuminator.py | Migrates to build_application() and uses shared parser utilities. |
| src/simtools/applications/simulate_flasher.py | Migrates to build_application() and uses shared parser utilities. |
| src/simtools/applications/run_application.py | Migrates to build_application() and renames CLI flag to --config_file (mapped to existing dest). |
| src/simtools/applications/production_merge_corsika_limits.py | Migrates to build_application(). |
| src/simtools/applications/production_generate_grid.py | Migrates to build_application(). |
| src/simtools/applications/production_derive_statistics.py | Migrates to build_application() and renames --offsets→--off_axis_angles. |
| src/simtools/applications/production_derive_corsika_limits.py | Migrates to build_application() and uses reusable telescope_ids arg. |
| src/simtools/applications/plot_tabular_data.py | Migrates to build_application(). |
| src/simtools/applications/plot_tabular_data_for_model_parameter.py | Migrates to build_application(). |
| src/simtools/applications/plot_simulated_event_distributions.py | Migrates to build_application(). |
| src/simtools/applications/plot_simtel_events.py | Migrates to build_application(). |
| src/simtools/applications/plot_array_layout.py | Migrates to build_application(). |
| src/simtools/applications/merge_tables.py | Migrates to build_application() and uses application IO handler for output pathing. |
| src/simtools/applications/maintain_simulation_model_write_array_element_positions.py | Migrates to build_application(). |
| src/simtools/applications/maintain_simulation_model_verify_production_tables.py | Migrates to build_application(). |
| src/simtools/applications/maintain_simulation_model_compare_productions.py | Migrates to build_application(). |
| src/simtools/applications/maintain_simulation_model_add_production.py | Migrates to build_application(). |
| src/simtools/applications/generate_simtel_event_data.py | Migrates to build_application(). |
| src/simtools/applications/generate_regular_arrays.py | Migrates to build_application(). |
| src/simtools/applications/generate_default_metadata.py | Migrates to build_application(). |
| src/simtools/applications/generate_corsika_histograms.py | Migrates to build_application(). |
| src/simtools/applications/generate_array_config.py | Migrates to build_application(). |
| src/simtools/applications/docs_produce_simulation_configuration_report.py | Migrates to build_application() and uses reusable args. |
| src/simtools/applications/docs_produce_model_parameter_reports.py | Migrates to build_application(). |
| src/simtools/applications/docs_produce_calibration_reports.py | Migrates to build_application() and uses reusable args. |
| src/simtools/applications/docs_produce_array_element_report.py | Migrates to build_application() and uses reusable args. |
| src/simtools/applications/derive_trigger_rates.py | Migrates to build_application() and uses reusable telescope_ids. |
| src/simtools/applications/derive_pulse_shape_parameters.py | Migrates to build_application(); uses application_label from config. |
| src/simtools/applications/derive_psf_parameters.py | Migrates to build_application() and consolidated zenith_angle/source_distance. |
| src/simtools/applications/derive_photon_electron_spectrum.py | Migrates to build_application(). |
| src/simtools/applications/derive_mirror_rnda.py | Migrates to build_application(). |
| src/simtools/applications/derive_incident_angle.py | Migrates to build_application() and uses reusable Quantity-based args. |
| src/simtools/applications/derive_ctao_array_layouts.py | Migrates to build_application(). |
| src/simtools/applications/db_upload_model_repository.py | Migrates to build_application() and updates config loading after DB config overrides. |
| src/simtools/applications/db_inspect_databases.py | Migrates to build_application(). |
| src/simtools/applications/db_get_parameter_from_db.py | Migrates to build_application(). |
| src/simtools/applications/db_get_file_from_db.py | Migrates to build_application(). |
| src/simtools/applications/db_get_array_layouts_from_db.py | Migrates to build_application(). |
| src/simtools/applications/db_generate_compound_indexes.py | Migrates to build_application(). |
| src/simtools/applications/db_add_value_from_json_to_db.py | Migrates to build_application(). |
| src/simtools/applications/db_add_simulation_model_from_repository_to_db.py | Migrates to build_application() and updates config loading after DB config overrides. |
| src/simtools/applications/db_add_file_to_db.py | Migrates to build_application(). |
| src/simtools/applications/convert_model_parameter_from_simtel.py | Migrates to build_application(). |
| src/simtools/applications/convert_geo_coordinates_of_array_elements.py | Migrates to build_application(). |
| src/simtools/applications/convert_all_model_parameters_from_simtel.py | Migrates to build_application(). |
| tests/unit_tests/test_application_control.py | Adds unit test coverage for build_application(). |
| tests/unit_tests/configuration/test_commandline_parser.py | Adds tests for quantity parsing and reusable application argument initialization. |
| tests/unit_tests/visualization/test_plot_psf.py | Updates tests to use consolidated arg names and Quantity inputs. |
| tests/unit_tests/ray_tracing/test_psf_parameter_optimisation.py | Updates tests to use consolidated arg names and Quantity inputs. |
| tests/unit_tests/ray_tracing/test_incident_angles.py | Updates tests to treat source_distance as Quantity. |
| tests/unit_tests/production_configuration/test_derive_production_statistics_handler.py | Updates tests for off_axis_angles rename. |
| tests/integration_tests/config/validate_optics_run.yml | Updates config keys (offset_step, source_distance, zenith_angle). |
| tests/integration_tests/config/validate_cumulative_psf_prod6_psf.yml | Updates config key zenith_angle. |
| tests/integration_tests/config/production_derive_statistics.yml | Updates config key off_axis_angles. |
| tests/integration_tests/config/derive_psf_parameters_run.yml | Updates config key zenith_angle. |
tobiaskleiner
requested changes
Apr 7, 2026
Collaborator
tobiaskleiner
left a comment
tobiaskleiner
left a comment
There was a problem hiding this comment.
@GernotMaier mostly good changes with some minor potential improvements.
Can we omit the file in every application?
The docstring under main() can be improved or matched better to the description (although I know we don't care that much about it).
In some places some info about units or details about the arguments format are lost. For instance Data file name with the measured PSF vs radius [cm].
src/simtools/application_control.py
Outdated
|
|
||
|
|
||
| def build_application( | ||
| application_path, |
Collaborator
There was a problem hiding this comment.
Is there an easier way to supply the path here? I think all applications are in the same directory by default so passing alwasy __file__ seems a bit unnecessary.
Collaborator
There was a problem hiding this comment.
Do we really need to supply __file__ everytime?
| ---------- | ||
| application_path : str | ||
| Application file path, typically ``__file__``. | ||
| description : str, optional |
Collaborator
There was a problem hiding this comment.
Should this be required?
| def main(): | ||
| """Get file from database.""" | ||
| app_context = startup_application(_parse) | ||
| app_context = build_application( |
Collaborator
There was a problem hiding this comment.
I am seeing now that the description is most often now similar or very similar to the help string below def main(). This is kind of a duplication but I guess it is required. Maybe we can make sure that these are identical for all cases?
| if args_dict.get("db_simulation_model_version"): | ||
| db_config["db_simulation_model"] = args_dict.get( | ||
| def main(): | ||
| """Application main.""" |
Collaborator
There was a problem hiding this comment.
This here is not very descriptive though.
| def _add_arguments(parser): | ||
| """Register application-specific command line arguments.""" | ||
| parser.initialize_application_arguments( | ||
| ["off_axis_angles", "source_distance", "number_of_photons"] |
Collaborator
There was a problem hiding this comment.
This is missing some of the initial information and the units. No default values anymore, is this ok?
| app_context = startup_application(_parse) | ||
| app_context = build_application( | ||
| __file__, | ||
| description="Produce a markdown report for model parameters.", |
| app_context = startup_application(_parse) | ||
| app_context = build_application( | ||
| __file__, | ||
| description="Produce a markdown report for model parameters.", |
| app_context = startup_application(_parse) | ||
| app_context = build_application( | ||
| __file__, | ||
| description="Generate a regular array of telescope and save as astropy table.", |
Collaborator
There was a problem hiding this comment.
table format not necessary to mention here
|
|
||
|
|
||
| def main(): | ||
| """Application main.""" |
|
|
||
|
|
||
| def main(): | ||
| """Plot array layout application.""" |
Collaborator
There was a problem hiding this comment.
application not needed
Contributor
Author
|
@tobiaskleiner - thanks a lot of review and suggestions! Added a mechanism to get the first line of the application docstring as CLI description - see discussions in Slack. Let me know if anything else is missing. |
Collaborator
|
Thanks @GernotMaier for the update. Only had one more open question about the file in every application (see first comment). |
Contributor
Author
|
Thanks @tobiaskleiner - I have addressed the remaining issue with the repetitive |
Collaborator
|
Very nice, looks good now, thanks @GernotMaier! |
tobiaskleiner
approved these changes
Apr 9, 2026
|
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.
Command line configuration is implemented via the Configuration and CommandLineParser tools to allow tool configuration via the command line and config files.
This maintenance PR is a cleanup in order to:
simtools.application_control.build_application.This PR should have no impact on how to use the applications (with the exception of some minor changes in command line parameters due to name/type consolidation; see the changes in the integration test config files).
Closes #1055
For the review: best to start looking at application_control first to understand the updated / merged functionality.