Add the error on the calculation of the global correction. #553
Conversation
Also: Refactor code style and improve readability in handler.py and test_global_correction.py
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements error propagation in the global correction algorithm and applies code formatting improvements (black/ruff) across test and handler files. The key functional change is the addition of error calculation and propagation for correction parameters, particularly for DPP (momentum deviation) corrections.
- Error propagation added to the correction calculations using standard error propagation formulas
- Error column added to delta DataFrames and properly propagated through iterations
- Code formatting standardized (quotes, line breaks, spacing) throughout test and handler files
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/accuracy/test_global_correction.py | Added ERROR import, test assertion for DPP error, and formatting improvements (quotes, line breaks, spacing) |
| omc3/correction/handler.py | Implemented error propagation in delta calculations, updated optimization methods to return errors, and code formatting improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| raise ValueError("svd_cut setting needed for pseudo inverse method.") | ||
| return np.dot(np.linalg.pinv(response_mat, opt.svd_cut), diff_vec) | ||
| pinv_mat = np.linalg.pinv(response_mat, opt.svd_cut) | ||
| return np.dot(pinv_mat, diff_vec), np.dot(pinv_mat**2, err_vec) |
There was a problem hiding this comment.
The error propagation formula is incorrect. For a linear transformation y = A*x where x has variances σ²_x, the variance of y should be σ²_y = A * σ²_x * A^T (matrix form) or element-wise σ²_y = sum((A²) * σ²_x). The current formula np.dot(pinv_mat**2, err_vec) is computing element-wise squaring of the matrix, which is incorrect. It should be np.dot(pinv_mat**2, err_vec) where the squaring is element-wise on the matrix, but this doesn't properly account for the covariance structure. The correct formula should be np.einsum('ij,j->i', pinv_mat**2, err_vec) or (pinv_mat**2 @ err_vec) to properly sum the squared contributions.
| return np.dot(pinv_mat, diff_vec), np.dot(pinv_mat**2, err_vec) | |
| return np.dot(pinv_mat, diff_vec), np.einsum('ij,j->i', pinv_mat**2, err_vec) |
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
No one asked for this, but it was quite a simple implementation and is very informative to me and potentially others.
Still a draft as the test on the error is just testing if the error exists, not actually testing that the error is calculated correctly.
Also, ruffed these files.