Add trackstack meteor mask - #966
Conversation
dvida
left a comment
There was a problem hiding this comment.
Thanks for this — the idea is a good one, and the before/after images make the case nicely. Suppressing planes and satellites in long trackstacks is something people have asked for.
There are three things that need fixing before this can go in, plus one design question worth settling.
Blocking
-
A leftover debug
raise RuntimeError(...)is still in theexcept KeyErrorbranch (with a deadpassafter it). This fires for every FF that has no detection — and without--shower,ff_listis every valid FF in the directory, so that's most of them.QueuedPoolcatches worker exceptions (RMS/QueuedPool.py:475), prints the traceback and returnsNone, so the run doesn't die; instead you get one traceback per detection-less FF (each dumping the full key list), those frames silently contribute nothing, andfinished_countis never incremented for them, so the progress print never reaches 100%. I suspect this is actually why your example image looks right — the detection-less FFs are being dropped by the exception rather than by intent. -
The extracted
find_ftp_file()still returnsFalseon failure, but neither call site checks it any more. Previously the inline block didreturn FalsefromtrackStack(). NowFalseflows intoshowerAssociation(config, [False], ...)in the shower branch, and intoos.path.dirname(False)in the new mask branch, which raisesTypeError: expected str, bytes or os.PathLike object, not bool. -
The
for i in range(1, 10)loop doesn't do what its comment says.breakon the first hit, plus meteor numbering that starts at 1 and is contiguous, means the body runs exactly once — only meteor #1 is ever masked, so an FF with two meteors keeps only the first. The comment's reasoning is also off:ftp_pointsis built from the whole FTPdetectinfo file with no shower filtering, so a shower filter is not the reason for a miss — FFs with no detection at all are. Buildingftp_pointsasff_name -> [meteor_meas, ...]and OR-ing the per-meteor line masks together removes the magic10, thei * 1.0float-key coercion, and the try/except control flow all at once.
Design question
Masking avepixel as well as maxpixel throws away the sky background, not just the planes. Planes and satellites are carried into the stack by max_deavg = maxpixel - avepixel; masking that alone suppresses them. Masking avepixel too means avg_stack_count only increments inside the meteor strips, so avg_stack_sum / avg_stack_count is zero everywhere no strip landed — the star field survives only as narrow bands, and the auto-crop shrinks with it (which is why the "after" image is 1065x1116 vs 1165x1174). Masking only max_deavg gives the same suppression with the full star background intact. If the strips-only look is deliberate, that's fine, but it should be said in the --mask-meteors help text.
Smaller things are in the inline comments: reusing the existing findFTPdetectinfoFile/validDefaultFTPdetectinfo helpers, the hard-coded 150 px dilation, camelCase naming, and the missing docstring entry.
For the record, a few things I checked that are correct as written: the [2]/[3] centroid indices match meteor_meas' [calib_status, frame_n, x, y, ...] layout and the FTPdetectinfo Col Row header ordering, so the cv2.line((x, y)) call is right; the (ff_name, meteor_No) float-keyed dict mirrors the existing shouldInclude/associations convention; and pickling ftp_points through func_extra_args is cheap at realistic detection counts.
| for i in range(1, 10): | ||
| # Attempt to make something work for multiple meteors in one frame. | ||
| # Some of them may not be in ftp_points because of a shower filter. | ||
| # This is not water tight. | ||
| try: | ||
| ff_mask = make_mask(ftp_points[(os.path.basename(ff_name), i * 1.0)], mask) | ||
| break | ||
| except KeyError: | ||
| raise RuntimeError(f"Can't find {(os.path.basename(ff_name), i * 1.0)} in {list(ftp_points.keys())}") | ||
| pass |
There was a problem hiding this comment.
Blocking — leftover debug code. The raise RuntimeError(...) needs to go (and the pass after it is unreachable). As written, every FF without a detection raises here. QueuedPool catches it, logs the traceback and returns None, so you get one traceback per detection-less FF — each interpolating the entire key list — the frame contributes nothing, and finished_count is never incremented so the progress print stalls short of 100%.
Blocking — the loop only ever masks meteor #1. break on the first hit plus contiguous meteor numbering from 1 means the body runs exactly once. An FF with two meteors keeps only the first, contrary to the comment. Also, ftp_points is unfiltered by shower, so the shower filter isn't the reason for a miss; detection-less FFs are.
Both go away if ftp_points maps ff_name -> [meteor_meas, ...]:
ff_mask = mask.img
if mask_meteors:
meteor_tracks = ftp_points.get(ff_basename, [])
if meteor_tracks:
ff_mask = makeMeteorMask(meteor_tracks, mask.img)No magic 10, no i * 1.0, no try/except as control flow — and it handles multiple meteors for real. Worth deciding explicitly what a detection-less FF should contribute when mask_meteors is on: currently it falls back to the full station mask, so its planes and satellites go in unmasked, which is the opposite of the intent.
| return False | ||
|
|
||
| ftp_file = ftp_list[0] | ||
| ftp_file = find_ftp_file(dir_path, config) |
There was a problem hiding this comment.
Blocking — regression. The inline block this replaced did return False from trackStack() when no FTPdetectinfo was found. find_ftp_file still returns False, but nothing checks it, so showerAssociation(config, [False], ...) gets called with a bool where a path is expected. Please restore the early return here (and at the new call site on line 140).
| if len(ftp_list) < 1: | ||
| print('unable to find FTPdetect file in {}'.format(dir_path)) | ||
| return False | ||
|
|
||
| return ftp_list[0] |
There was a problem hiding this comment.
Returning False as a sentinel is what makes the two call sites silently misbehave. Since RMS/Formats/FTPdetectinfo.py:251 already raises FileNotFoundError for this case, raising here would be consistent with the rest of the codebase; otherwise return None and check it. Either way both call sites need to handle it.
| for dir_path in dir_paths: | ||
| ftp_file = find_ftp_file(dir_path, config) | ||
| for ftp_entry in readFTPdetectinfo(os.path.dirname(ftp_file), os.path.basename(ftp_file)): | ||
| ftp_points[(ftp_entry[0], ftp_entry[2])] = ftp_entry[-1] |
There was a problem hiding this comment.
Keying on (ff_name, meteor_No) means only one meteor per key and forces the i * 1.0 probing loop downstream. ftp_points.setdefault(ftp_entry[0], []).append(ftp_entry[-1]) gives you every meteor per FF in one pass, and lets stackFrame do a plain .get(ff_basename, []).
| import time | ||
| import datetime | ||
|
|
||
| def find_ftp_file(dir_path, config): |
There was a problem hiding this comment.
Naming: this file and the wider codebase use camelCase for functions (trackStack, stackFrame, getArray, shouldInclude). Suggest findFTPFile here and makeMeteorMask on line 46.
Also, find_ftp_file has no docstring while everything else in this module does.
| def make_mask(ftp_points, initial_mask): | ||
| """Make a mask in which only the meteor is visible""" | ||
| meteor_mask = np.zeros_like(initial_mask.img) |
There was a problem hiding this comment.
This takes a MaskStructure but returns a bare ndarray, and initial_mask is really the station mask. Taking mask.img directly would be clearer, and would make the function unit-testable without constructing a MaskStructure.
| help="""Leave at least one core free""") | ||
|
|
||
| arg_parser.add_argument('--mask-meteors', action="store_true", | ||
| help="""Render only the part of the image around the meteor to suppress planes and satellites (works best for large trackstacks""") |
There was a problem hiding this comment.
Unbalanced parenthesis: "...(works best for large trackstacks". Also worth stating here whether the sky background outside the meteor is intentionally dropped (see the avepixel comment).
| @@ -1,3 +1,4 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
The shebang and the 100644 -> 100755 mode change are unrelated to the feature. Only 4 of the Utils/*.py scripts carry a shebang, so this is a bit inconsistent as housekeeping. Not harmful — but I'd rather it were a separate commit, or applied across the directory at once.
tammojan
left a comment
There was a problem hiding this comment.
Thanks Denis and AI for this thorough review. I addressed most of it.
| meteor_mask = cv2.line(meteor_mask, (round(ftp_points[0][2]), round(ftp_points[0][3])), | ||
| (round(ftp_points[-1][2]), round(ftp_points[-1][3])), 255, 1) |
There was a problem hiding this comment.
Nice suggestion, done.
| return False | ||
|
|
||
| ftp_file = ftp_list[0] | ||
| ftp_file = find_ftp_file(dir_path, config) |
| maxpixel[ff_mask == 0] = 0 | ||
| avepixel = copy.deepcopy(ff.avepixel) | ||
| avepixel[mask.img == 0] = 0 | ||
| avepixel[ff_mask == 0] = 0 |
There was a problem hiding this comment.
Nice, this fixes the background subtraction for bright meteors (one of which is visible in the example I posted).
| help="""Leave at least one core free""") | ||
|
|
||
| arg_parser.add_argument('--mask-meteors', action="store_true", | ||
| help="""Render only the part of the image around the meteor to suppress planes and satellites (works best for large trackstacks""") |
|
Awesome - once you're fully done, could you regnerate and post the example above? I'd love to see the new plot :) |

This adds an option to stack only the part of the images which have the meteor detection. With this option, in the resulting trackstack there are less planes and satellites.
Example before:

After:
