Repository navigation
Fit the RMSF over a sized window cut at the main lobe - #98
Merged
Merged
Conversation
`fit_rmsf` fitted its Gaussian over only about sigma/2 either side of the peak, sized from the analytic guess. That measures how sharply the top of the lobe curves, not its width at half maximum, so on a lobe that is not Gaussian it read 2% narrow (flat in frequency) to 7% wide (boxcar), and the answer moved with `n_samples` and with the guess. The window is now `rmsf_fitting_size` analytic FWHMs wide (default 1.25, as WSClean's `-beam-fitting-size`) and stops one sample short of the lobe's first minimum on each side, so no sidelobe can get in however wide it is set. Against the exact half-max width that lands within 1.4% on a Gaussian, boxcar, flat-in-frequency and gapped ASKAP-like band for `n_samples` >= 5. `rmsynth_3d` and `rmsynth_3d_from_fits` gain `do_fit_rmsf` and `rmsf_fitting_size`: with fitting on, `fwhm_rmsf_radm2` comes from one fit to the shared `rmsf_arr` instead of 3.8 / (lambda^2 range). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C5pn26DJht35yoKWbMJ1sW
Both sides added keywords to the same `FDFOptions(...)` calls in the 1D and 3D tools (the RMSF fit window here, Stokes I weighting in #97); both are kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C5pn26DJht35yoKWbMJ1sW
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #98 +/- ##
==========================================
+ Coverage 92.51% 92.58% +0.06%
==========================================
Files 14 14
Lines 3207 3208 +1
==========================================
+ Hits 2967 2970 +3
+ Misses 240 238 -2 ☔ View full report in Codecov by Harness. |
`nanargmax` raises its own "All-NaN slice" on a blank RMSF, before `curve_fit` rejects it. The multiscale screen test pins that a blank pixel fails in the fit, as it did before this branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C5pn26DJht35yoKWbMJ1sW
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
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.
fit_rmsffitted its Gaussian over only about sigma/2 either side of the peak, with the window sized from the analytic guess. That measures how sharply the top of the lobe curves, not its width at half maximum. On a lobe that is not Gaussian it read 2% narrow (flat in frequency) to 7% wide (boxcar), and the answer also moved withn_samplesand with the guess.Change
fit_rmsftakesfitting_size(default 1.25): the window is that many analytic FWHMs wide, like WSClean's-beam-fitting-size, and stops one sample short of the lobe's first minimum on each side. However wide it is set, no sidelobe gets in.rmsf_fitting_sizeis exposed onFDFOptions,get_rmsf_nufftandrun_rmsynth.rmsynth_3dandrmsynth_3d_from_fitsgaindo_fit_rmsfandrmsf_fitting_size. With fitting on,fwhm_rmsf_radm2comes from one fit to the sharedrmsf_arr(about 2 ms per cube) instead of 3.8 / (lambda^2 range). Off by default, so 3D output is unchanged unless asked.fit_rmsf, so they pick up the new window.test_multiscale.pypasses unchanged.Accuracy against the exact half-max width
Error at
n_samples3 / 5 / 10 / 20 / 100:Tests
n_samples10 and 100, a huge window gives the same answer as one that just reaches the first minimum,rmsf_fitting_size <= 0raises, and the 3D fit matchesfit_rmsfonrmsf_arr.test_options_defaults.pycovers the new defaults.test_options_defaults,test_synthesis_utils,test_nufft,test_multiscale,test_rmsynthandtest_tools_3d_daskpass (410 tests), and so do thermclean_1dandrmsynth_1dnotebooks.test_rmclean_graph_build_stays_linear_in_chunk_countfailed once under four xdist workers on four cores, then passed serially with and without this change. It only times RM-CLEAN graph building, which this change doesn't touch.prek run --hook-stage manual --all-filesis clean.flint-crew/flint will expose
do_fit_rmsfandrmsf_fitting_sizeonce this is released.🤖 Generated with Claude Code
https://claude.ai/code/session_01C5pn26DJht35yoKWbMJ1sW
Generated by Claude Code