-
Notifications
You must be signed in to change notification settings - Fork 3
Add feature to simulate an arbitrary layout with CORSIKA. Use telescope as 'probes' to calculate Cherenkov photon later distribution. #1995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
(intermeditate PR into other dev branch - ignore)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds support for simulating arbitrary telescope layouts with CORSIKA (including “probe” telescopes for lateral density comparisons), and refactors overwrite-parameter handling to be read/validated once and reused across models.
Changes:
- Refactors overwrite-model-parameter application to pass a pre-loaded
overwrite_model_parameter_dictinto models instead of repeatedly reading YAML files. - Updates CORSIKA histogram generation to support two normalization methods (
per-telescopeprobe-based andper-binfrom counts) with configurable axis selection width. - Extends regular-array generation to support square/star layouts and emits an overwrite-compatible
.info.ymlalongside the ECSV layout.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit_tests/utils/test_general.py | Adds unit coverage for validating list/dict types. |
| tests/unit_tests/simtel/test_simulator_light_emission.py | Updates constructor usage after overwrite refactor. |
| tests/unit_tests/ray_tracing/test_mirror_panel_psf.py | Updates telescope model initialization after overwrite refactor. |
| tests/unit_tests/model/test_model_utils.py | Updates tests for new overwrite dict flow in model initialization. |
| tests/unit_tests/model/test_model_parameter.py | Reworks overwrite tests to use dict-based overwrite API. |
| tests/unit_tests/model/test_array_model.py | Updates array model tests for new overwrite dict attribute. |
| tests/unit_tests/layout/test_array_layout_utils.py | Adds extensive coverage for square/star layout creation and YAML export. |
| tests/unit_tests/corsika/test_corsika_histograms.py | Updates histogram tests for new density population/normalization paths. |
| tests/integration_tests/config/generate_regular_arrays_run_south_4LST_square.yml | Adds integration config for generating a 4-LST square layout. |
| tests/integration_tests/config/generate_regular_arrays_run_south_1SST.yml | Adds integration config for generating a 1-SST layout. |
| tests/integration_tests/config/generate_regular_arrays_run_south.yml | Removes old monolithic integration workflow config. |
| tests/integration_tests/config/generate_regular_arrays_run_north_40MST_star.yml | Adds integration config for generating a star layout (currently inconsistent fields). |
| tests/integration_tests/config/generate_regular_arrays_run_north.yml | Removes old monolithic integration workflow config. |
| src/simtools/utils/general.py | Extends validate_data_type to support list and dict. |
| src/simtools/simtel/simulator_light_emission.py | Stops passing overwrite file path directly; relies on centralized overwrite handling. |
| src/simtools/ray_tracing/mirror_panel_psf.py | Stops passing overwrite file path directly; relies on centralized overwrite handling. |
| src/simtools/ray_tracing/incident_angles.py | Stops passing overwrite file path directly; relies on centralized overwrite handling. |
| src/simtools/model/telescope_model.py | Renames overwrite input to overwrite_model_parameter_dict. |
| src/simtools/model/site_model.py | Renames overwrite input to overwrite_model_parameter_dict. |
| src/simtools/model/model_utils.py | Adds read_overwrite_model_parameter_dict() and uses it during model initialization. |
| src/simtools/model/model_parameter.py | Removes file-reading overwrite method; applies dict-based overwrite and adjusts validation logic. |
| src/simtools/model/calibration_model.py | Renames overwrite input to overwrite_model_parameter_dict. |
| src/simtools/model/array_model.py | Reads overwrite YAML once and passes dict to sub-models. |
| src/simtools/layout/array_layout_utils.py | Adds square/star layout helpers and YAML writer for overwrite-compatible layout changes. |
| src/simtools/db/db_handler.py | Adds --ignore_missing_design_model fallback behavior for array-element queries. |
| src/simtools/data_model/schema.py | Caches schema retrieval from URI via lru_cache. |
| src/simtools/corsika/corsika_histograms.py | Implements probe-based density computation and selectable normalization method. |
| src/simtools/configuration/commandline_parser.py | Adds --ignore_missing_design_model CLI option. |
| src/simtools/camera/camera_efficiency.py | Stops passing overwrite file path directly; relies on centralized overwrite handling. |
| src/simtools/applications/validate_optics.py | Stops passing overwrite file path directly; relies on centralized overwrite handling. |
| src/simtools/applications/validate_cumulative_psf.py | Removes explicit overwrite-file application; relies on centralized overwrite handling. |
| src/simtools/applications/generate_regular_arrays.py | Adds CLI args for array generation and writes overwrite-compatible .info.yml. |
| src/simtools/applications/generate_corsika_histograms.py | Adds CLI for normalization choice + axis distance; expands usage notes. |
| src/simtools/applications/derive_pulse_shape_parameters.py | Stops passing overwrite file path directly; relies on centralized overwrite handling. |
| src/simtools/applications/derive_psf_parameters.py | Stops passing overwrite file path directly; relies on centralized overwrite handling. |
tests/integration_tests/config/generate_regular_arrays_run_north_40MST_star.yml
Outdated
Show resolved
Hide resolved
tests/integration_tests/config/generate_regular_arrays_run_south_4LST_square.yml
Outdated
Show resolved
Hide resolved
tests/integration_tests/config/generate_regular_arrays_run_south_1SST.yml
Outdated
Show resolved
Hide resolved
…th_40MST_star.yml Co-authored-by: Copilot <[email protected]>
…th_4LST_square.yml Co-authored-by: Copilot <[email protected]>
…th_1SST.yml Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
tobiaskleiner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @GernotMaier, have added a few comments. Obviously quite a bit hacky to get the density profiles. Was thinking if it'd be possible to have one large sphere, and record all events to get a density?
| layout with non-overlapping telescope definitions | ||
|
|
||
| - run CORSIKA simulations with the desired settings using this telescope layout (use the | ||
| 'overwrite_model_parameters' option to point to the generated layout info.yml file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use a better file naming?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest? This is how we called those files for the simulation model (also have a simulation_models_info.schema.yml).
I have clarified the sentence a bit now that it is hopefully clearer.
|
|
||
| The arrays consist of one telescope at the center of the array and or of 4 telescopes | ||
| in a square grid. These arrays are used for trigger rate simulations. | ||
| Arrays can consist of single (central) telescopes, square grids or star-like patterns. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain what is star-like in this context? + or x? or different shape.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improved the docstring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no return in fill.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the docstring (also 'raise' is in this module).
|
|
||
| ax = self.axis_distance | ||
|
|
||
| avg_x, unc_x = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand here the x/y density profiles average telescope samples within a transverse slice, but are not normalized by the slice width, so they depend on telescope layout rather than representing a true spatial density. Fine but maybe you want to apply an explicit length normalization?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really - each telescopes provides a density and we average over those - so there is no need to have a length normalization, but simply a 'number of telescopes' simulations. This is the main difference introduced with this PR: we can use e.g. SSTs as probes to calculate densities without being worried about edge effects.
| required="--list_available_layouts" not in self._option_string_actions, | ||
| ) | ||
|
|
||
| _job_group.add_argument( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand why this is needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean the ignore_missing_design_model command line argument?
- currently telescope models are defined only for the prod6 hyper array layout; introducing a layout of e.g. 500 will fail (as we can pull only design values for the defined telescopes from the DB)
- this command line option tells the code to use the design option for missing telescopes.
|
|
||
|
|
||
| def create_regular_array(array_name, site, telescope_distance): | ||
| def _create_star_array(tel_name, pos_x, pos_y, pos_z, n_telescopes, tel_type, site, distance): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I see now what you mean with star shape. Maybe cross or plus pattern?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do like 'stars'. None of the names are clear and consistent and I hopefully with the improved docstring this is clear.
|
|
@tobiaskleiner - thanks a lot for the review. I tried to improve the docstrings to make things a bit clearer. Let me know if this is fine. |
|
@GernotMaier, thanks for your response and clarifications. All good then and you can merge. |




This PR covers the use case to compare lateral photon density distributions (plus other distributions) between e.g. different CORSIKA versions. The typical workflow is:
Example
Attached an example simulating 500 SSTs in a star like pattern to cover the x / y axis for the lateral density distribution. Note the change to the previous way of calculating densities: here each telescope is used as a 'probe' and we do not care about the spatial distribution inside a telescope sphere - simply calculating number of photons per telescope divided by projected area.
The attached example is for illustration - probably not useful otherwise to compare a couple of 100 GeV proton showers generated with QGSJet and Epos.
example.pdf
Main changes
This solves the issue with incorrect normalization when using a histogram and area normalization by the bin area - CORSIKA is writing photons at the position of spheres on the ground.
Improved the efficiency of reading changes through the
overwrite_parametersfunctionality. Read the yaml file with the changes once (and validate it once) and bass on the dict of values instead of the file names to the different Model classes (before: each telescope/site/calibration/array model opened the yaml file; significantly slowed down the model preparation phase)Improve the
generate_regular_arraysapplication: allow square and star shape layouts; more telescopes; flexible distances, output in the format required to be used with theoverwrite_model_parameteroption.