Add experimental per-trace and batched mode refinement - #4
Open
jimsweeney wants to merge 9 commits into
Open
jimsweeney wants to merge 9 commits into
jimsweeney wants to merge 9 commits into
Conversation
This was referenced Sep 22, 2026
Adds cuphoton.xray.synthetic_validation: a fixture of known damped modes (the synthetic_trace modes plus white Gaussian noise), Cramer-Rao bounds from the exact Fisher information of the same model, a Monte Carlo sweep that reports per-mode bias, standard deviation and rmse of every parameter against the bound together with the mode-loss rate, optional model-mismatch traces with a residual-to-noise ratio, and a schema_version 1 summary (config, runtime via runtime_metadata, results per condition with attempted/successful/failed trials, artifacts). The linear-prediction-validate (lpv) command runs the sweep and, with --output-dir, writes summary.json and a Bokeh figure outside the checkout. Documented in docs/xray/LINEAR-PREDICTION-VALIDATION.md. The sweep is the reproducer for the mode loss discussed in NVIDIA#1: a lightly damped mode whose fitted decay crosses zero under noise is dropped by the root filter. No default behaviour changes. Refs NVIDIA#1 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014wyj5iPSHmtR3vSTvJWKfw Signed-off-by: James Sweeney <jmsweene@gmail.com>
Keep small-trial summaries valid JSON, integrate the requested chirp, and retain floating parameters for Fisher derivatives. Match nearby modes globally and reject incompatible combined sweep settings. Signed-off-by: Trent Nelson <trentn@nvidia.com>
Use the same amplitude and phase convention for truth, fitted modes, and summary metadata so equivalent modes have zero parameter error. Signed-off-by: Trent Nelson <trentn@nvidia.com>
Keep text formatting separate from command setup and execution. Signed-off-by: Trent Nelson <trentn@nvidia.com>
Reject invalid numerical inputs before trials and distinguish estimator exceptions from mode loss in the text report. Record revisions only for tracked source checkouts, together with tracked- file dirtiness. Embed plot resources for offline use without changing Bokeh output state. Clarify numerical Fisher bounds and conditional scatter, and anchor the Gaussian distortion envelope at the first sample. Signed-off-by: Trent Nelson <trentn@nvidia.com>
cuphoton.xray.mode_refinement refines the damped-mode model by nonlinear least squares from the linear-prediction result (analytic Jacobian, scipy.optimize.least_squares), seeding a mode the root filter dropped from the residual spectrum; RefinedModes carries Jacobian uncertainties and the residual rms. cuphoton.xray.mode_refinement_batched runs the same Levenberg-Marquardt for a batch of traces on NumPy or CuPy and matches the per-trace solver to about 1e-8. linear-prediction-validate gains --refine, which adds the refined estimator to the sweep and the summary; linear-prediction-refine-benchmark (lprb) times the SciPy loop against the batched solver on NumPy and CuPy (best of --repeat, transfers excluded, warm-up call). examples/xray_lp_modes_review.py writes a Bokeh review page of traces, reconstructions and modes. Documented in docs/xray/LINEAR-PREDICTION-REFINEMENT.md with RTX 4050 numbers. Overlaps the fit-diagnostics, iterative-fitting and GPU-batching work the maintainers describe in NVIDIA#1; posted so the overlap can be sorted out. Refs NVIDIA#1 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014wyj5iPSHmtR3vSTvJWKfw Signed-off-by: James Sweeney <jmsweene@gmail.com>
Keep LP modal parameters relative to the first sample during refinement. Recognize stationary starts and use positive diagonal damping so a zero-amplitude mode does not make the batch solve singular. Signed-off-by: Trent Nelson <trentn@nvidia.com>
Normalize Jacobian columns for batched solves and require a small gradient to converge. Estimate local uncertainty with a rank-aware SVD and report unavailable uncertainties for rank-deficient fits. Signed-off-by: Trent Nelson <trentn@nvidia.com>
Keep the reproducible harness and explain the experimental fit scope, local uncertainty assumptions, and convergence limits. Remove fixed-run accuracy statistics and laptop timing tables. Signed-off-by: Trent Nelson <trentn@nvidia.com>
tpn
changed the title
tpn
force-pushed
the
xray-lp-refinement
branch
from
September 24, 2026 22:25
f052bdf to
647fa99
Compare
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry. Review configuration: ⚙️ Run configurationConfiguration used: Repository: NVIDIA/cuPhoton/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (16)
Comment |
Collaborator
|
@coderabbitai full review |
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds experimental, opt-in nonlinear refinement for comparing damped-mode estimators. Builds on #2, including its current validation fixes. The production detector fitter and its defaults are unchanged.
Linear prediction can miss a lightly damped mode under noise. This contribution uses its recovered modes as starting guesses, seeds missing modes from the residual spectrum, and refines them with SciPy least squares. It reports local parameter uncertainties from a rank-aware Jacobian SVD; unidentifiable fits report unavailable uncertainties.
The NumPy/CuPy batch solver refines many independent traces together. Jacobian scaling makes the convergence criterion independent of time units mathematically, and a tiny damped step no longer counts as convergence. These fits are unconstrained and separate from the production fitter's positive-decay and bounded-frequency contract.
lpv --refinecompares the estimators using #2's synthetic validation.lprbcompares per-trace and batched refinement from supplied starting parameters; its timings exclude initialization, input I/O, and detector processing. The Bokeh example supports visual inspection. Documentation retains reproduction commands and interpretation while removing machine-score tables and fixed-run accuracy guarantees.Validation: 521 CPU core/XRay tests passed, 39 skipped; 32 refinement tests passed with a CUDA 13 CuPy environment on an RTX PRO 6000 Blackwell GPU. Lint, formatting and pre-commit checks passed. Independent review verified the uncertainty calculation and corrected stopping behavior. Synthetic and distorted validation sweeps and a 128-trace CPU/GPU benchmark smoke completed; this is correctness evidence, not an end-to-end performance or science qualification.
Known limit: a seeded 32-trace noisy test reported 3 unconverged rows on both NumPy and CuPy at the strict default
tol=1e-9, despite close parameter agreement. The same test converged on all rows with explicittol=1e-8. Results retain per-trace convergence flags; the default was not weakened to hide rounding-limited progress.Historical contributor measurements
The earlier laptop timings and single-run accuracy figures remain in the original refinement note. They predate the current fixes and are review history, not release performance baselines.
Refs #1. Remains stacked on #2 pending its merge.