Bug fix in direct-injection type flasher simulations. #2029
Conversation
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
GernotMaier
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes a critical bug where direct-injection type flasher simulations were not generating any calibration events due to improper handling of the is_calibration_run flag after a recent refactoring. The fix removes the is_calibration_run parameter from runner classes and instead relies on the corsika_config.is_calibration_run() method to determine if a simulation is a calibration run. Additionally, the PR fixes a bug in pedestal calculation where sim_telarray requires the string "none" instead of Python's None for the stars parameter.
Changes:
- Removed
is_calibration_runparameter fromSimulatorArray,SimtelRunner, andCorsikaSimtelRunnerclasses and updated all call sites to usecorsika_config.is_calibration_run()method instead - Added conversion of Python
Noneto string "none" for sim_telarray's stars parameter in calibration simulations - Fixed incorrect light source specification in MST FlashCam integration test (was using NectarCam)
- Updated model versions in LST integration tests from 6.0.2 to 7.0.0
- Improved logging in simulate_flasher.py to handle both telescope and array layout modes
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/simtools/simtel/simulator_array.py | Removed is_calibration_run parameter, now uses corsika_config.is_calibration_run() method; added None to "none" conversion for stars parameter |
| src/simtools/runners/simtel_runner.py | Removed is_calibration_run parameter from constructor |
| src/simtools/runners/corsika_simtel_runner.py | Removed is_calibration_run parameter from constructor and SimulatorArray instantiation |
| src/simtools/applications/simulate_flasher.py | Improved logging to properly handle telescope vs array layout modes |
| tests/unit_tests/simtel/test_simulator_array.py | Removed test for is_calibration_run parameter, updated calibration test to mock the method call |
| tests/unit_tests/runners/test_corsika_simtel_runner.py | Removed is_calibration_run parameter from fixture |
| tests/integration_tests/config/simulate_flasher_direct_injection_mst_flashcam.yml | Fixed light source from MSFx-NectarCam to MSFx-FlashCam and updated test name |
| tests/integration_tests/config/simulate_flasher_direct_injection_lst_south.yml | Updated model version from 6.0.2 to 7.0.0 |
| tests/integration_tests/config/simulate_flasher_direct_injection_lst_north.yml | Updated model version from 6.0.2 to 7.0.0 |
| docs/changes/2029.bugfix.md | Added changelog entry documenting the bug fix |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Direct-injection type flasher simulations did not generate any calibration events (although integration tests passed).
Reason was a recent refactoring of the simulation runners where the setting of
is_calibration_runwas messed up.Second bug fix (appeared only after above was fixed): bug in pedestal calculation setting: sim_telarray requires
none(and notNone) for the stars option.