Implement strict fixed-source-state calibration and synthetic calibration coverage#8
Conversation
|
@copilot that's a good start. This will primarily be used with circularly polarised input states, will that work? |
Yes, if the source state is known to be circular. The adjustment I’d make to the plan is: don’t remove circular input from the forward model, remove it from the fitted unknowns. In other words, keep S3 as fixed acquisition metadata (for example |
|
@copilot ok let's just assume a fixed S_i_hat input that isn't optimised. I also want to add a systematic test suite that generates synthetic images, optionally adds some noise, and runs the calibration. Let's go ahead an implement please |
Co-authored-by: benjym <3380296+benjym@users.noreply.github.com>
Co-authored-by: benjym <3380296+benjym@users.noreply.github.com>
Implemented. Calibration now treats |
|
@copilot can you remove the fallback to a default s_i_hat everywhere and just make it fail without it? Also remove redundant legacy variable names, just one variable name per thing. No fallbacks or backwards compatibility, just a clean interface |
Co-authored-by: benjym <3380296+benjym@users.noreply.github.com>
Co-authored-by: benjym <3380296+benjym@users.noreply.github.com>
Done on |
There was a problem hiding this comment.
Pull request overview
This PR updates the calibration workflow to treat the incoming polarisation state (S_i_hat) as an explicit fixed input, fitting only per-wavelength stress-optic coefficients (C), and aligns tests/docs with that new contract.
Changes:
- Require
fit.S_i_hatin calibration configs; reject legacyfit.initial_S_i_hat. - Remove no-load-derived/default
S_i_hatseeding and simplify the optimizer to fitConly. - Update synthetic calibration tests and user configuration documentation accordingly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
photoelastimetry/calibrate.py |
Enforces explicit fixed fit.S_i_hat, removes S_i_hat fitting/initialisation, and fits C only. |
tests/test_calibrate.py |
Updates synthetic fixtures and tests for fixed S_i_hat and new validation behavior. |
docs/user/configuration.md |
Documents fit.S_i_hat as a required calibration input and clarifies fitting behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| ) | ||
| fit_s_i_hat = np.asarray(fit_cfg["S_i_hat"], dtype=float) | ||
| if fit_s_i_hat.shape != (3,): | ||
| raise ValueError(f"fit.S_i_hat must have shape (3,), got {fit_s_i_hat.shape}.") |
| fit_cfg = dict(config.get("fit", {})) | ||
| if "initial_S_i_hat" in fit_cfg: | ||
| raise ValueError("fit.initial_S_i_hat is no longer supported; use fit.S_i_hat.") | ||
| if "S_i_hat" not in fit_cfg: | ||
| raise ValueError( | ||
| "Calibration config must include fit.S_i_hat as a 3-component array " "[S1_hat, S2_hat, S3_hat]." | ||
| ) | ||
| fit_s_i_hat = np.asarray(fit_cfg["S_i_hat"], dtype=float) | ||
| if fit_s_i_hat.shape != (3,): | ||
| raise ValueError(f"fit.S_i_hat must have shape (3,), got {fit_s_i_hat.shape}.") | ||
|
|
||
| fit_cfg.setdefault("max_points", 6000) | ||
| fit_cfg.setdefault("loss", "soft_l1") | ||
| fit_cfg.setdefault("f_scale", 0.05) | ||
| fit_cfg.setdefault("max_nfev", 300) | ||
| fit_cfg.setdefault("seed", 0) | ||
| fit_cfg.setdefault("s3_identifiability_threshold", 0.02) | ||
| fit_cfg.setdefault("prior_weight", 0.0) | ||
| fit_cfg["S_i_hat"] = fit_s_i_hat.tolist() |
| def _resolve_fixed_s_i_hat(fit_config): | ||
| """Resolve the fixed incoming Stokes state used during calibration.""" | ||
| s_i_hat = np.asarray(fit_config["S_i_hat"], dtype=float) | ||
| if s_i_hat.shape != (3,): | ||
| raise ValueError(f"fit.S_i_hat must have shape (3,), got {s_i_hat.shape}.") | ||
| return _normalise_s_i_hat(s_i_hat) |
| - `fit.S_i_hat` as an explicit 3-component fixed source state `[S1_hat, S2_hat, S3_hat]` | ||
|
|
||
| `fit.S_i_hat` is treated as a fixed source state during calibration; the | ||
| optimiser only fits `C`. Calibration does not provide a default `S_i_hat`. |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
The calibration flow now treats the incoming source polarisation
S_i_hatas a fixed input rather than an optimised parameter, and adds a systematic synthetic test suite to validate the new behaviour across noiseless, noisy, linear, and circular-source cases.Scope
Implemented model changes
S_i_hatduring calibration.S_i_hatfrom configuration and hold it fixed while fitting onlyC[λ].S_i_hatin the saved profile as the fixed source state used for the calibration run.Config and API changes
fit.S_i_hatas the explicit fixed source-state input used during calibration.fit.initial_S_i_hatname; configs using it now fail with a direct error.S_i_hatfallback behaviour from the implemented calibration path.fit.s3_identifiability_thresholddefault/config dependency from the implemented calibration path.initial_Cas the optimiser seed input when provided.Stress-optic coefficient handling
C.Cthroughfit.c_relative_boundsandfit.prior_weight.Pipeline and schema impact
CS_i_hatSynthetic calibration test coverage
S_i_hatinputsDocumentation
fit.S_i_hatas a required fixed source state and note that calibration now fits onlyC.Original prompt
📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.