Switch DBE's regression-surface upsample to bilinear - #26
Merged
Merged
Conversation
Re-profiling after the last two perf PRs found zoom(order=3) still showing up: _fit_background_surface upsamples its coarse regression grid with cubic interpolation, then immediately blurs the result again (sigma=patch_size*0.5, typically >=32px) -- the same "nothing left for cubic's curvature term to recover" situation gaussian_filter_ds's own upsample was already fixed for. Switched to order=1. Validated directly against real _dbe_regression_coarse_grid output (not assumed): post-blur mean diff <0.1, max <1.0 ADU. New pinning test in test_dbe_gradient.py. Found and left alone: wavelet_background_extraction has a sibling zoom(order=3) call with no follow-up blur, so the same argument doesn't automatically carry over -- and it has no real quality test to validate a change against (test_sky_model.py only mocks it). A synthetic check was inconclusive (near-zero field variance in the test data, not representative). Reverted that half of the change rather than ship it unvalidated. Measured on a real Omega Nebula session subset: Post-process 18.5s -> 17.8s. Full suite (1651 tests) passes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
Re-profiling after #22/#25's Phase 4 work found
zoom(order=3)still costing real time:_fit_background_surface(the standard DBE path, exercised bydynamic_background_extraction) upsamples its coarse regression grid with cubic interpolation, then blurs the result again immediately after (sigma=patch_size*0.5) -- same situationgaussian_filter_ds's own upsample was already fixed for earlier this session.order=1. Validated directly against real_dbe_regression_coarse_gridoutput: post-blur mean diff <0.1, max <1.0 ADU.zoom(order=3)call inwavelet_background_extractionwith no follow-up blur -- the same argument doesn't automatically carry over, and it has no real quality test (only a mocked one intest_sky_model.py). A synthetic check was inconclusive. Left it asorder=3, unchanged rather than ship an unvalidated change.Test plan
ruff check .cleantest_fit_background_surface_bilinear_upsample_matches_cubic) measures the actual post-blur difference on real regression-grid machinery, not a synthetic stand-in🤖 Generated with Claude Code