Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 54 additions & 20 deletions Utils/TrackStack.py
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#!/usr/bin/env python3

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

from __future__ import print_function

import os, sys
Expand All @@ -14,6 +15,7 @@
from RMS.Astrometry.ApplyAstrometry import xyToRaDecPP, raDecToXYPP
from RMS.Astrometry.Conversions import date2JD, jd2Date
from RMS.Formats.FFfile import validFFName, getMiddleTimeFF
from RMS.Formats.FTPdetectinfo import readFTPdetectinfo
from RMS.Formats.FFfile import read as readFF
from RMS.Formats.Platepar import Platepar
from RMS.Math import angularSeparation
Expand All @@ -26,11 +28,33 @@
import time
import datetime

def find_ftp_file(dir_path, config):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if os.path.isfile(os.path.join(dir_path,'.config')):
tmpcfg = cr.loadConfigFromDirectory('.config', dir_path)
else:
tmpcfg = config
ftp_list = glob(os.path.join(dir_path, 'FTPdetectinfo_{}*.txt'.format(tmpcfg.stationID)))
ftp_list = [x for x in ftp_list if 'backup' not in x and 'unfiltered' not in x]
Comment on lines +36 to +37

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This filter duplicates validDefaultFTPdetectinfo() in RMS/Formats/FTPdetectinfo.py:31, and misses uncalibrated, which that predicate also excludes. findFTPdetectinfoFile(path) at RMS/Formats/FTPdetectinfo.py:251 covers most of this too. The existing helper doesn't do the stationID/.config matching, so a thin wrapper is defensible — but it should call the shared predicate rather than re-implementing the substring checks.

(To be clear, the duplication is pre-existing — this PR just moved it into a function. Since it's being touched anyway, it's a good moment to switch.)

ftp_list.sort()

if len(ftp_list) < 1:
print('unable to find FTPdetect file in {}'.format(dir_path))
return False

return ftp_list[0]
Comment on lines +40 to +43

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A single segment from the first to the last centroid under-covers fragmenting events and strongly curved (long, near-horizon) meteors. cv2.polylines over all the centroids is the same amount of code and is exact:

pts = np.array([[round(p[2]), round(p[3])] for p in ftp_points], dtype=np.int32)
meteor_mask = cv2.polylines(meteor_mask, [pts], False, 255, 1)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice suggestion, done.

meteor_mask = cv2.dilate(meteor_mask, np.ones((150, 150)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 150x150 kernel is a fixed +/-75 px halo regardless of sensor resolution or --scalefactor — about a fifth of the frame height at 1280x720, a thin line at 4K. Worth exposing as a keyword argument (default 150) and/or scaling it off pp_ref.Y_res.

Two smaller points: np.ones((150, 150)) is float64, so prefer np.ones((150, 150), np.uint8) or cv2.getStructuringElement(cv2.MORPH_RECT, (150, 150)); and this dilation runs per FF in every worker, so the structuring element could be built once.

return np.minimum(meteor_mask, initial_mask.img)

def trackStack(dir_paths, config, border=5, background_compensation=True,
hide_plot=False, showers=None, darkbackground=False, out_dir=None,
scalefactor=None, draw_constellations=False, one_core_free=False,
textoption=0):
textoption=0, mask_meteors=False):
Comment thread
tammojan marked this conversation as resolved.
""" Generate a stack with aligned stars, so the sky appears static. The folder should have a
platepars_all_recalibrated.json file.

Expand Down Expand Up @@ -88,20 +112,7 @@ def trackStack(dir_paths, config, border=5, background_compensation=True,
# Get FTP file so we can filter by shower
for dir_path in dir_paths:

if os.path.isfile(os.path.join(dir_path,'.config')):
tmpcfg = cr.loadConfigFromDirectory('.config', dir_path)
else:
tmpcfg = config

ftp_list = glob(os.path.join(dir_path, 'FTPdetectinfo_{}*.txt'.format(tmpcfg.stationID)))
ftp_list = [x for x in ftp_list if 'backup' not in x and 'unfiltered' not in x]
ftp_list.sort()

if len(ftp_list) < 1:
print('unable to find FTPdetect file in {}'.format(dir_path))
return False

ftp_file = ftp_list[0]
ftp_file = find_ftp_file(dir_path, config)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


print('Performing shower association using {}'.format(ftp_file))

Expand All @@ -123,6 +134,13 @@ def trackStack(dir_paths, config, border=5, background_compensation=True,
ff_list.append(file_name)
ff_list = list(set(ff_list))

ftp_points = {}
if mask_meteors:
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)):
Comment thread
tammojan marked this conversation as resolved.
ftp_points[(ftp_entry[0], ftp_entry[2])] = ftp_entry[-1]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, []).


# Take the platepar with the middle time as the reference one
ff_found_list = []
jd_list = []
Expand Down Expand Up @@ -272,7 +290,7 @@ def trackStack(dir_paths, config, border=5, background_compensation=True,
thead_pool = QueuedPool(stackFrame, cores=cores, backup_dir=None, print_state=False, func_extra_args=(recalibrated_platepars, mask, border,
pp_ref, img_size, jd_middle, pp_stack, config,
avg_stack_sum_shared, avg_stack_count_shared, max_deaveraged_shared,
background_compensation, finished_count, num_ffs))
background_compensation, finished_count, num_ffs, ftp_points, mask_meteors))
thead_pool.startPool()
# add jobs
for i, ff_name in enumerate(enumlist):
Expand Down Expand Up @@ -374,7 +392,7 @@ def trackStack(dir_paths, config, border=5, background_compensation=True,


def stackFrame(ff_name, recalibrated_platepars, mask, border, pp_ref, img_size, jd_middle, pp_stack, conf, avg_stack_sum_arr,
avg_stack_count_arr, max_deaveraged_arr, background_compensation, finished_count, num_ffs):
avg_stack_count_arr, max_deaveraged_arr, background_compensation, finished_count, num_ffs, ftp_points, mask_meteors):
ff_basename = os.path.basename(ff_name)

avg_stack_sum = getArray(img_size, avg_stack_sum_arr)
Expand Down Expand Up @@ -411,11 +429,24 @@ def stackFrame(ff_name, recalibrated_platepars, mask, border, pp_ref, img_size,
stack_x = stack_x[filter_arr]
stack_y = stack_y[filter_arr]

ff_mask = mask.img
if mask_meteors:
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


# Apply the mask to maxpixel and avepixel
maxpixel = copy.deepcopy(ff.maxpixel)
maxpixel[mask.img == 0] = 0
maxpixel[ff_mask == 0] = 0
avepixel = copy.deepcopy(ff.avepixel)
avepixel[mask.img == 0] = 0
avepixel[ff_mask == 0] = 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Masking avepixel is what removes the sky background rather than just the planes. Planes and satellites reach the stack via max_deavg = maxpixel - avepixel, so masking that is sufficient; stars appear in both images and largely cancel there, coming instead from the avepixel blend. Because ones_img is derived from the masked avepixel, masking it means avg_stack_count only increments inside the meteor strips, so the blended background is zero wherever no strip landed and the auto-crop shrinks to the union of the strips.

Suggestion — drop the masking here and on line 447, and mask after the subtraction instead:

max_deavg = maxpixel - avepixel
if mask_meteors:
    max_deavg[meteor_mask == 0] = 0

Same plane/satellite suppression, full star field retained. If the strips-only look is what you're after, that's a legitimate choice — just document it in the --mask-meteors help.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this fixes the background subtraction for bright meteors (one of which is visible in the example I posted).


# Compute deaveraged maxpixel image
max_deavg = maxpixel - avepixel
Expand Down Expand Up @@ -526,6 +557,9 @@ def getArray(size, shared_arr):
arg_parser.add_argument('--freecore', action="store_true",
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""")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch, fixed.


# Parse the command line arguments
cml_args = arg_parser.parse_args()

Expand All @@ -548,4 +582,4 @@ def getArray(size, shared_arr):
hide_plot=cml_args.hideplot, showers=showers,
darkbackground=cml_args.darkbackground, out_dir=cml_args.output, scalefactor=cml_args.scalefactor,
draw_constellations=cml_args.constellations, one_core_free=cml_args.freecore,
textoption = text_option)
textoption = text_option, mask_meteors=cml_args.mask_meteors)