Derive MRRA via gradient descent #1911
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements gradient descent optimization for deriving the mirror_reflection_random_angle parameter in Monte Carlo simulations for gamma-ray astronomy. The implementation replaces the previous iterative comparison-based approach with a more robust RMSD-based gradient descent optimization.
- Extends the existing PSF parameter optimization framework to support optimizing only
mirror_reflection_random_angle - Introduces gradient descent optimization with adaptive learning rates and configurable RMSD thresholds (default 3%)
- Adds final PSF comparison visualization showing optimized parameters and fit quality
- Replaces D80 containment-based optimization with full PSF curve RMSD minimization
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/simtools/ray_tracing/psf_parameter_optimisation.py | Extended PSFParameterOptimizer to support selective parameter optimization, added epsilon parameter to gradient descent methods, and added support for single mirror mode in ray tracing |
| src/simtools/ray_tracing/mirror_panel_psf.py | Refactored to use gradient descent optimization instead of iterative D80 comparison, removed old optimization logic, updated result printing to show RMSD instead of D80 |
| src/simtools/applications/derive_mirror_rnda.py | Updated command-line interface to support gradient descent parameters (threshold, learning_rate), removed old PSF measurement arguments, added cleanup option for intermediate files |
| src/simtools/visualization/plot_psf.py | Enhanced plot functions to handle optional parameter groups, added create_final_psf_comparison_plot for final optimization results |
| tests/unit_tests/ray_tracing/test_psf_parameter_optimisation.py | Added comprehensive tests for gradient descent workflow, refactored tests to use helper fixtures, added tests for single mirror and full telescope modes |
| tests/unit_tests/ray_tracing/test_mirror_panel_psf.py | Replaced old optimization tests with gradient descent tests, added tests for various mirror modes and random focal length configurations |
| tests/unit_tests/visualization/test_plot_psf.py | Added test for create_final_psf_comparison_plot function |
| docs/changes/1911.feature.md | Added changelog entry for gradient descent implementation |
|
GernotMaier
left a comment
GernotMaier
left a comment
There was a problem hiding this comment.
Minor comment - please implement. I approve already. Thanks!!
| * Final PSF comparison plot showing measured vs simulated cumulative PSF | ||
| * Optimized mirror_reflection_random_angle parameters | ||
| (3-component: sigma1, fraction2, sigma2) | ||
| * JSON model parameter file (if RMSD converges below threshold) |
There was a problem hiding this comment.
Do you want to apply some reasonable rounding? I just don't like 0.22040325851139586 in the DB - very hard to see how many valid digits there are.
| --site North \\ | ||
| --telescope LSTN-01 \\ | ||
| --model_version 6.0.2 \\ | ||
| --mirror_list tests/resources/mirror_list_CTA-N-LST1_v2019-03-31_rotated.ecsv \\ |
There was a problem hiding this comment.
Not sure about this: this is the single mirror PSF, but we use a list of mirrors? So the result is the average over all curves? Maybe indicate this in the docstring.
There was a problem hiding this comment.
The mirror list provided here is to get the focal lengths of the mirrors (alternatively use --use_random_focal_length). Running with --test we use the first 10 mirrors, but without it we use all mirrors. Each mirror is simulated separately but then the photons are all appended in one place, so we get one combined PSF image which we use for the optimisation. It's not an average, it's generating one PSF image from the total photon distribution.
There was a problem hiding this comment.
I will explain it in the docstring
| Measured D80: | ||
| Mean = 1.400 cm | ||
| Optimization Results (RMSD-based): | ||
| RMSD (full PSF curve): 0.022782 |
There was a problem hiding this comment.
Do we want units here and in the sigma values below?
| f"RMSD {panel_psf.final_rmsd:.6f} > threshold {threshold:.6f}" | ||
| ) | ||
|
|
||
| # Cleanup intermediate files if requested |
There was a problem hiding this comment.
Unnecessary comment (your function is called cleanup...)
| def write_optimization_data(self): | ||
| """ | ||
| Write optimization results to an astropy table (ecsv file). | ||
| Write optimization results as a JSON model parameter file. |
There was a problem hiding this comment.
Very minor: no need to say it is a JSON file - maybe tomorrow we change the format of the model parameter files: we then don't want to change all those docstrings.
|
|
||
| while iteration < max_iterations: | ||
| if current_metric <= rmsd_threshold: | ||
| tolerance = 0.0001 # 0.01% tolerance |
There was a problem hiding this comment.
Hardcoded tolerance is well motivated?
There was a problem hiding this comment.
The tolerance is technically not needed, but I thought that if we're setting a threshold of <= 3% then an rmsd of 3.004% is reasonable to accept. The tolerance is chosen so we can round to the nearest 3 significant figures in the percentage.
| test: true | ||
| use_random_focal_length: true | ||
| use_random_focal_length: false | ||
| integration_tests: |
There was a problem hiding this comment.
Wondering if we want to test that the output values (three model parameter values) is consistent with expectations? This would require to put an example json file into tests/resources/model_parameters and add a reference_output_file point like in tests/integration_tests/config/db_get_parameter_from_db_telescope_parameter_version.yml (don't ask about the formatting in this file)
There was a problem hiding this comment.
What do you mean consistent with expectations? The current implementation already restricts the gradient descent so that the parameter values do not step out of the allowed range defined in the schema file. Is that enough? Or do you mean validate the json file to have the correct format?
Use the pre-existing gradient descent implementation for deriving PSF parameters to first derive
mirror_reflection_random_angle.To run use:
Uses the first 10 mirror panels in the ray tracing when running with
--test, and a default RMSD threshold of 3%.