Skip to content

Add production limits for list of telescopes#2128

Merged
tobiaskleiner merged 8 commits into
mainfrom
add-production-limits-for-list-of-telescopes
Apr 24, 2026
Merged

Add production limits for list of telescopes#2128
tobiaskleiner merged 8 commits into
mainfrom
add-production-limits-for-list-of-telescopes

Conversation

@tobiaskleiner
Copy link
Copy Markdown
Collaborator

@tobiaskleiner tobiaskleiner commented Apr 21, 2026

This PR adds the option to pass a list of telescopes to compute the production limits.

Fixes reading of NSB values from the merged tables.

@tobiaskleiner tobiaskleiner marked this pull request as draft April 21, 2026 08:40
@tobiaskleiner tobiaskleiner marked this pull request as ready for review April 22, 2026 08:22
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 extends CORSIKA production limit derivation to support passing an inline list of telescope IDs, and improves robustness when reading NSB and other numeric values from merged/combined event data tables.

Changes:

  • Add support in derive_corsika_limits.generate_corsika_limits_grid() for array_element_list as an alternative telescope configuration input.
  • Fix/robustify parsing of NSB and other numeric metadata/FILE_INFO columns when values are string/byte-string encoded.
  • Update histogram filling to accumulate across indexed datasets in merged event files, with accompanying unit tests.

Reviewed changes

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

Show a summary per file
File Description
src/simtools/sim_events/writer.py Cast nsb_integrated_flux metadata to float with fallback to filename parsing.
src/simtools/sim_events/reader.py Add numeric conversion helper and use it to handle byte/string encoded numeric FILE_INFO columns.
src/simtools/sim_events/histograms.py Preserve/accumulate histogram counts across multiple indexed datasets.
src/simtools/production_configuration/derive_corsika_limits.py Accept array_element_list as a telescope configuration source; raise on missing configuration.
src/simtools/applications/production_derive_corsika_limits.py Document array_element_list usage in CLI docstring/examples.
tests/unit_tests/sim_events/test_writer.py Add tests for NSB metadata string casting and fallback behavior.
tests/unit_tests/sim_events/test_reader.py Add tests for decoding/convert helper and handling invalid numeric data.
tests/unit_tests/sim_events/test_histograms.py Add test ensuring histogram counts accumulate across multiple datasets.
tests/unit_tests/production_configuration/test_derive_corsika_limits.py Add tests for array_element_list input and missing telescope configuration error.
docs/changes/2128.feature.md Changelog entry for inline telescope list support.

Comment thread src/simtools/sim_events/reader.py
Comment thread src/simtools/applications/production_derive_corsika_limits.py
Comment thread src/simtools/sim_events/histograms.py
Copy link
Copy Markdown
Contributor

@GernotMaier GernotMaier left a comment

Choose a reason for hiding this comment

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

Thanks for this.

Please check if commandline_parser:_add_model_option_layout() does not give you already the required command line option.

Removed the inline list option for telescope IDs in the documentation.
@ctao-sonarqube
Copy link
Copy Markdown

@tobiaskleiner
Copy link
Copy Markdown
Collaborator Author

Thanks for this.

Please check if commandline_parser:_add_model_option_layout() does not give you already the required command line option.

Right and this is exactly what is being used here, but you are right that it should not appear as arguement in the dosctring, as it is implicit. Let me know if this is fine now.

Copy link
Copy Markdown
Contributor

@GernotMaier GernotMaier left a comment

Choose a reason for hiding this comment

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

All good now, thanks!

@tobiaskleiner tobiaskleiner merged commit bc7a85c into main Apr 24, 2026
16 checks passed
@tobiaskleiner tobiaskleiner deleted the add-production-limits-for-list-of-telescopes branch April 24, 2026 10:48
@tobiaskleiner
Copy link
Copy Markdown
Collaborator Author

Thanks @GernotMaier!

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