-
Notifications
You must be signed in to change notification settings - Fork 0
Kf experiemnts intercpt #68
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
FayFrost107
commented
Oct 23, 2025
- I have updated the KF so that it includes the intercept of the linear emulator
- I have altered some code in the calibration module so that a dynamic sigma is recorded at each time step (turns out this is the same every timestep so actually is static, but worth keeping my edits in anyway)
- Some new notebooks for my plots
…ry beat - not just at the end of the loop
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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
This PR refactors the Kalman filter implementation to handle time-varying posterior covariances and fixes the intercept handling in the emulator. Key changes include augmenting state vectors with an intercept term, modifying the Kalman filter to work with time-varying covariance matrices, and updating plotting functions to correctly handle 3D covariance arrays.
- Refactored Kalman filter to augment state vectors with an intercept term instead of treating it separately
- Modified calibration workflow to save time-varying posterior covariances for real data
- Updated plotting utilities to handle both static and time-varying covariance matrices
Reviewed Changes
Copilot reviewed 12 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/cvdnet_pipeline/utils/kf_emulator.py | Augments state vectors and matrices with intercept term; refactors Kalman update equations to use augmented observation matrix H |
| src/cvdnet_pipeline/calibrate_parameters.py | Collects and saves time-varying posterior covariances for real data; adds logic to save variance time series |
| src/cvdnet_pipeline/utils/plot_utils.py | Adds logic to handle both 3D (time-varying) and 2D (static) covariance matrices when plotting parameter trajectories |
| src/cvdnet_pipeline/utils/bayesian_calibration.py | Reduces initial variance from 0.01 to 0.0001 for tighter prior on parameters |
| src/cvdnet_pipeline/kalman_filter_giessen.py | Reduces initial variance for T parameter and adds unused means variable |
| input_parameters_jsons/parameters_sensitive_ci_6_wide.json | New configuration file defining parameter ranges for 6-parameter sensitivity analysis |
| input_parameters_jsons/parameters_patr_drift_3.json | New configuration file with modified parameter ranges |
| input_parameters_jsons/parameters_patr_drift_2_rveact.json | New configuration file for 2-parameter drift analysis focusing on RV E_act |
| input_parameters_jsons/parameters_patr_drift_2_rc.json | Updates parameter ranges for pat vessel resistance and compliance |
| input_parameters_jsons/parameters_patr_drift.json | Updates pat vessel resistance upper bound from 1.5 to 3.0 |
| config/synthetic.json | Updates to use 6-parameter configuration with 1024 samples and removes calibration steps |
| config/real.json | Updates configuration for real data processing with 6 parameters and modified step ordering |
Comments suppressed due to low confidence (1)
src/cvdnet_pipeline/kalman_filter_giessen.py:84
- Variable means is not used.
means = input_prior.mean().loc[:'T'].values
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…n code, as its usually called manually once the class is instantiated
|
@LevanBokeria I've opened a new pull request, #69, to work on those changes. Once the pull request is ready, I'll request review from you. |
Change to a more stable linear algebra operation
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.
@FayFrost107 since we don't yet have tests for this, could you just check that the linear algebra operation we changed with Copilot's suggestion gives you the same results on some data where you know the outputs?