-
Notifications
You must be signed in to change notification settings - Fork 3
New flasher test file #2033
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
base: main
Are you sure you want to change the base?
New flasher test file #2033
Conversation
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
Updates the flasher event test resource(s) and removes temporary workarounds that were added to handle outdated sim_telarray metadata (per PR #2023), aligning integration/unit tests and plotting configs with the new flasher simtel file.
Changes:
- Remove temporary fallback behavior in simtel event reading / plotting code paths.
- Update unit/integration tests to reference the new LSTN flasher simtel resource and expected warning message.
- Add flasher full-simulation workflow YAML and corresponding sim_telarray log to test resources.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit_tests/simtel/test_simtel_event_reader.py | Updates expected warning text when telescope lookup fails. |
| tests/integration_tests/config/plot_simtel_events.yml | Points integration test plotting to the new flasher simtel resource + updates expected outputs. |
| src/simtools/simtel/simtel_event_reader.py | Removes temporary tel_id fallback and now cleanly returns when telescope not found. |
| src/simtools/visualization/plot_simtel_events.py | Removes temporary handling comment around event_index defaulting. |
| src/simtools/applications/plot_simtel_events.py | Updates CLI doc example to reference the new flasher simtel file. |
| tests/resources/flasher_full_simulation_lst_north/simulate_flasher_full_simulation_lst_north.yml | Adds workflow config used to generate the updated flasher simtel resource. |
| tests/resources/flasher_full_simulation_lst_north/full_simulation_run000010_North_7.0.0_simulate_flasher.simtel.log | Adds a reference log from sim_telarray execution for the updated flasher resource. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@EshitaJoshi - this is a short follow up to #2023 . Don't worry about the linter test, which fails while trying to lint the log file from sim-telarray (uploaded to ensure a bit of reproducibiliy) |
EshitaJoshi
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 for the PR!
| --simtel_files tests/resources/ff-1m_flasher.simtel.zst \ | ||
| --simtel_files full_simulation_run000010_North_7.0.0_simulate_flasher.simtel.zst \ | ||
| --plots signals time_traces \ | ||
| --telescope MSTN-04 \ |
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.
This should be LSTN-04 in the example (the example command didnt work with MSTN-04, and I also had to specify the full file path of course)
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.
Ah - fixed.
|





The flasher test file tests/resources/ff-1m_flasher.simtel.zst contained outdated metadata - this PR replaces the test file and remove the temporary settings introduced in PR #2023.
Note that the old test file was for MSTN-04, while the new one is for LSTN-04 (should not matter for a test file).
Note: allow the linter workflow to fail, as this is an issue in the sim_telarray log file (not an issue)) and not in simtools.