Improve fireball detector recall with stdpixel decontamination - #866
Improve fireball detector recall with stdpixel decontamination#866alextudorica wants to merge 3 commits into
Conversation
7ac217a to
2f18ff2
Compare
f88aa48 to
b714310
Compare
b714310 to
0c788ea
Compare
A bright fireball inflates stdpixel along its trail within the 256-frame FF block, pushing the threshold (avepixel + k1*stdpixel + j1) above 255. The uint8 clip kills near-saturated trail pixels (250-254). Fix: before thresholding, replace stdpixel at contaminated pixels (maxpixel >= P90 AND stdpixel > 3x background median) with the background median. Pure numpy, no Cython changes needed. Also: clip threshold to 254 (not 255) so near-saturated pixels can pass, and replace the hard-coded 2 px/frame fireball velocity cap with a configurable angular velocity (fireball_max_ang_vel, default 60 deg/s) converted to a per-sensor px/frame limit at runtime using fps and the platepar-derived deg/pixel scale. Tested on 129-event reference dataset (1755 FF files, 743 stations): FR-positive recall 99.0% (679/686), missed station rescue 74.0% (77/104), overall station recall 86.0% -> 96.4% (716/743). Zero threshold failures on real data. Line finder has zero failures in all 686 tests -- the problem was entirely upstream in the threshold stage.
- Exclude masked pixels from contamination set, not just background sample - Use O(n) np.partition instead of O(n log n) np.percentile for P90 - Copy only stdpixel layer instead of full compressed array (~2.8 MB saved) - Report which specific config attributes are missing/zero in warning - Fix Cython comment: only max_val=254 is newly enabled, not 250-254 - Document fallback velocity cap (~39 deg/s) vs production default (60 deg/s) - Add from __future__ import division for Python 2 safety
dvida
left a comment
dvida
left a comment
There was a problem hiding this comment.
Thanks -- the diagnosis that the recall loss lives in the threshold stage (stdpixel contamination pushing ave + k1*std + j1 past the uint8 ceiling) is convincing, the decontamination approach is sound, and the benchmark results are impressive. I verified the parts that can be checked statically:
- Velocity conversion math is correct: deg/s / (deg/px) / fps / f correctly lands in subsampled px/frame (matches
thresholdAndSubsample'sx//f,y//f, per-frame z). The fallback 2.0 px/frame reproduces legacy behavior, and the ~39.5 deg/s equivalence claimed in the comment checks out at 720p/25fps/f=16. - No side effects on saved FF files:
self.compressedis never mutated (decontamination writes to a copy), anddetectionCutOutonly reads the avepixel plane, so passing it the original array is consistent. - Config plumbing (
.config->parseFireballDetection->findCoefficients) is correct and backward compatible (config=Nonekeeps legacy behavior for external callers).
One blocking issue (inline): the mask handling crashes the extractor process on every FF block for stations with detection_binning_factor > 1 -- reproduced, details and a one-line fix in the inline comment.
One request before merge: a false-positive control benchmark. The decontamination strictly lowers thresholds -- it re-enables exactly the bright slow objects the old clipping accidentally suppressed (planes, car headlights, lightning), and the velocity cap simultaneously widens from an effective ~2.0 to ~3.0 subsampled px/frame at default config (+52%). The benchmark covers only fireball-positive FF files, so the FP cost of both changes is unmeasured. Please run a few full ordinary nights (no fireballs) stock vs. this PR and report the FR-file count and extractor CPU-time deltas. If plane/headlight FRs go up noticeably, that's a disk/bandwidth cost for the whole network.
Non-blocking notes:
- The magic constants (P90 bright cut,
3*std_refcontamination factor, 100-px minimum background sample) would be easier to tune later as named module-level constants, even if not config options. - More threshold passers means the
max_points(500) random subsample can dilute fireball points on noisy frames -- benchmark shows a net win, just flagging the mechanism. - A small synthetic unit test for the decontamination block (clean image no-op / contaminated trail gets replaced / binned-mask shape case) and the deg/s conversion would protect this logic; nothing in
Tests/covers this pipeline today.
| # Exclude masked-out pixels (camera borders/obstructions) from the background sample | ||
| if self.mask is not None: | ||
| mask_valid = self.mask.img > 0 | ||
| bg_mask = bg_mask & mask_valid |
There was a problem hiding this comment.
Blocking: this crashes the extractor for every station with detection_binning_factor > 1.
Extractor.__init__ bins the mask via binImageCalibration() (using config.detection_binning_factor), but the extractor operates on capture-resolution frames (Compression.py passes the full-res compressed array). With binning factor 2, self.mask.img is e.g. 360x640 while compressed[0] is 720x1280. The existing maskImage() tolerates this with a silent shape check (MaskImage.py), but this new code doesn't:
ValueError: operands could not be broadcast together with shapes (720,1280) (360,640)
(Reproduced with a synthetic 720p compressed array + a 2x-binned mask, running this block verbatim.) Since findPoints() runs inside the Extractor process, the exception kills the process on every FF block -- fireball detection is completely disabled for any station with detection binning enabled.
Suggested fix -- mirror the shape guard from maskImage():
if self.mask is not None and self.mask.img.shape == maxpix.shape:
mask_valid = self.mask.img > 0
bg_mask = bg_mask & mask_valid
else:
mask_valid = None| maxpix = self.compressed[0] | ||
| stdpix = self.compressed[3] | ||
| # O(n) partition-based percentile instead of O(n log n) full sort | ||
| bright_threshold = np.partition(maxpix.ravel(), -maxpix.size // 10)[-maxpix.size // 10] |
There was a problem hiding this comment.
Minor: by this point executeAll() has already run maskImage() on maxpixel/avepixel, which fills masked pixels with the image mean -- and those filled pixels are included in this percentile. On heavily-masked stations this skews bright_threshold. Once the shape guard (see comment below) is in place, consider computing the percentile over mask-valid pixels only.
| contaminated = contaminated & mask_valid | ||
| if np.any(contaminated): | ||
| compressed = np.array(self.compressed) | ||
| compressed[3] = self.compressed[3].copy() |
There was a problem hiding this comment.
np.array(self.compressed) already deep-copies all 4 planes, so this line copies plane 3 a second time -- it can be deleted. The comment above ("Only the stdpixel layer is copied") also doesn't match the code: the whole 4-plane array is copied when contamination is present (~3.7 MB at 720p, ~8 MB at 1080p -- acceptable, but the comment should say what actually happens). Making the copy truly std-only would require passing planes separately to the Cython function, which isn't worth it -- just fix the comment and drop this line.
| # Make sure the threshold limit is not above the maximum possible value | ||
| if avg_std > 255: | ||
| avg_std = 255 | ||
| # Clip threshold to 254 so that pixels at exactly max_val=254 can still pass |
There was a problem hiding this comment.
The old code was not actually blind to saturated pixels: the pass condition below is max_val >= avg_std, so with the old clip at 255 a saturated pixel (255 >= 255) always passed. What actually failed were the unsaturated bright trail pixels (200-254) whose threshold got clipped to 255 -- and those are fixed by the stdpixel decontamination, not by this clip.
This change only additionally rescues pixels with max_val == 254, so it's a near-no-op (harmless, fine to keep). But please update this comment and the PR description ("maxpixel > 255 is always false for uint8", "the fireball becomes invisible at the very pixels where it is brightest") -- the stated mechanism is wrong and will mislead future readers about why the decontamination was needed.
| max_lines: 5 | ||
|
|
||
| ; Maximum angular velocity for fireball candidates in deg/s. Converted to a per-sensor | ||
| ; px/frame cap at runtime using fps and the platepar-derived deg/pixel scale, so the same |
There was a problem hiding this comment.
"platepar-derived" is inaccurate -- the scale comes from the nominal fov_w/fov_h values in the [Capture] section of the config file, not from the platepar. Worth stating precisely, because users need to know which value has to be kept correct for this cap to be meaningful (nominal config FOV can be stale or wrong even when the platepar is good). Same wording appears in the ConfigReader.py comment above self.fireball_max_ang_vel.
- Fix extractor crash on stations with detection_binning_factor > 1: the binned mask no longer matches the capture-resolution frames, so guard the mask by shape (mirrors maskImage()) instead of raising a broadcast error in findPoints(). - Extract the decontamination into a testable decontaminateStdpixel() helper; compute the brightness percentile over mask-valid pixels only (executeAll fills masked-out maxpixel with the image mean, which otherwise skews it on heavily-masked cameras). - Drop the redundant second copy of the stdpixel plane and correct the comment (np.array() already deep-copies all 4 planes). - Name the tuning constants (BRIGHT_PERCENTILE, CONTAMINATION_STD_FACTOR, MIN_BACKGROUND_PIXELS) at module level. - Correct the clip-254 comment and the .config/ConfigReader wording: the recall loss was unsaturated trail pixels clipped to 255 (fixed by decontamination), not saturated pixels; the velocity scale comes from the nominal [Capture] FOV, not the platepar. - Add Tests/test_fireball_decontamination.py: decontamination (clean no-op, contaminated trail, binned-mask shape guard, mask exclusion, replacement clamp) and the deg/s -> px/frame velocity conversion.
|
Thanks for the thorough review — all points addressed in 2d8994f. Blocking (binning crash): fixed exactly as you suggested. The decontamination now FP control benchmark (your merge gate): ran full ordinary nights (no fireballs) through
On the clear night the two modes produce the identical 16 FRs (planes / bright objects Non-blocking, all done:
One correction to my own numbers while re-verifying: my original recall table came from a |
2 participants
Summary
The fireball detector misses events when a bright fireball inflates
stdpixelalong itstrail within the 256-frame FF block. The threshold
avepixel + k1*stdpixel + j1thenexceeds the uint8 range and is clipped.
thresholdAndSubsamplekeeps a pixel whenmaxpixel >= threshold, so a fully-saturated pixel (maxpixel == 255) still passes — butthe unsaturated trail pixels, whose true value sits below the clipped threshold, are
dropped. The trail loses its fainter edges exactly where recall matters.
The line finder (
find3DLines) is not the problem — it has zero failures when given aclean point cloud. The bug is entirely upstream in the threshold stage.
Changes
VideoExtraction.py— stdpixel decontamination (main fix)decontaminateStdpixel()replacesstdpixelat contaminated pixels with the backgroundmedian before
thresholdAndSubsample:std_ref = median(stdpixel)over background (non-bright,mask-valid) pixels.
maxpixelin the top decile ANDstdpixel > 3 * std_ref.stdpixel[contaminated] = max(1, round(std_ref)).Pure numpy, Pi/Py2-compatible. The 4-plane compressed array is copied only when
contamination is present, so the no-fireball path is zero-copy. Mask handling is
shape-guarded (mirrors
maskImage()), so it is a no-op — not a crash — on stations withdetection_binning_factor > 1.Grouping3Dcy.pyx— threshold clip 254Clip the threshold to 254 so
maxpixel == 254pixels can also pass. This is a minor extrarescue on top of the decontamination (which does the real work): saturated pixels already
passed under the old 255 clip.
Grouping3D.py/.config/ConfigReader.py— FOV-aware velocity capfindCoefficientsconvertsfireball_max_ang_vel(deg/s, default 60) into a per-sensorpx/frame cap using fps and the nominal
[Capture]FOV, replacing the hardcodedtotal > 2.Falls back to the legacy 2.0 px/frame when no config is passed. On the fleet's cameras
(~87×45° / 720p) this is ~2.3 px/frame — slightly wider than legacy.
Recall benchmark (129-event v2 reference dataset, real per-station configs)
Each station's shipped
.config(real FOV / k1 / j1 / fps), stock vs this PR.FR-positive recall (676 FFs where stock produced an FR file):
Missed-station rescue (100 stations stock produced no FR for):
0 regressions, +11 stations rescued, +2 FR-positive gains.
False-positive control (full ordinary nights, no fireballs)
Full nights of real FF files through stock vs this PR — the concern being that the lower
threshold and wider cap could re-admit planes / headlights / clouds:
objects stock already extracts).
(they flip with the point-subsample RNG seed), not planes/satellites, not a new failure
mode (stock's own FRs already include a plane and clouds). At ~0.66 MB/FR that is a few
MB per cloudy camera-night.
per ~10 s FF block → <0.1% duty cycle.
Known limitation: for 180° all-sky fisheye cameras the linear deg/px conversion gives a
very tight cap (~0.8 px/frame). A fisheye's scale is not uniform across the frame, so the
cap is not meaningful there — those are the only configs where it is tighter than legacy.
Tests
Tests/test_fireball_decontamination.py: decontamination (clean no-op, contaminated trail,binned-mask shape guard, mask exclusion, replacement clamp) and the deg/s → px/frame
velocity conversion.