Check that target is up for duration of raster #428
Conversation
Added straight-forward code to check that the source remains above horizon for the estimated duration of the raster. To facilitate this, I had to re-factor the code to separate (A) selection of the raster scan pattern from (B) the execution of the selected pattern. This change does not affect any of the scan patterns or other logic.
| # Confirm that the target will be "up" for the entire duration. | ||
| raster_duration = raster_params["num_scans"] * (raster_params["scan_duration"] + 2) # Extra for slew | ||
| if not session.target_visible(target, duration=raster_duration): | ||
| continue |
There was a problem hiding this comment.
In session's raster_scan there is already a check like this:
# Calculate average time that noise diode is operated per scan,
# to add to scan duration in check below
nd_time = session.nd_params['on'] + session.nd_params['off']
nd_time *= scan_duration / max(session.nd_params['period'],
scan_duration)
nd_time = nd_time if session.nd_params['period'] >= 0 else 0.
# Check whether target is visible for entire duration of raster scan
if not session.target_visible(target,
(scan_duration + nd_time) * num_scans):
user_logger.warning("Skipping raster scan, as target %r will be "
"below horizon", target.name)
return FalseDoesn't that work? Maybe this script just needs to check the return value of session.raster_scan.
There was a problem hiding this comment.
i agree, will strip this code from my PR.
i also think the doc comment in session.track(), scan() & raster_scan() should explicitly state that this visibility check is performed. @ajoubertza i'll leave that decision and follow-up to you.
There was a problem hiding this comment.
hang on, i take that back.
to address the original issue #5 one must skip out before the code reaches session.raster_scan(), since otherwise there will still be a compscan with a small number of dumps but without raster data.
There was a problem hiding this comment.
True. This is an artifact of the way compscan labels work. If we ever had kattelmod-driven scripts, you could chuck out the raster scan without an actual label. Hey, one is allowed to dream...
Thinking about it again, we could actually rework raster_scan and friends to include this functionality, although we would have to fix existing observation scripts - a potentially daunting task. Could you make a ticket so that we can remember? Anton can advise which project (CB?).
There was a problem hiding this comment.
@ludwigschwardt You can use the MT project.
There was a problem hiding this comment.
@ludwigschwardt you should make your own ticket - i don't particularly care for the task that you have identified, and you are best placed to describe what you think must be done. i am only interested in seeing my change through against the current code base.
There was a problem hiding this comment.
@ludwigschwardt what's the status? has the raster_scan functionality been reworked?.
| # Confirm that the target will be "up" for the entire duration. | ||
| raster_duration = raster_params["num_scans"] * (raster_params["scan_duration"] + 2) # Extra for slew | ||
| if not session.target_visible(target, duration=raster_duration): | ||
| continue |
As requested in PR #428 I've now added similar code as in session.raster_scan() to anticipate the potential noise diode cycle time per scan.
To address one of the two "bugs" noted in issue #5 , this PR adds straight-forward code to check that the source remains above horizon for the estimated duration of the raster.
To facilitate this, I had to re-factor the code to separate (A) selection of the raster scan pattern from (B) the execution of the selected pattern. This change does not affect any of the scan patterns or other logic.