HERA-Team / baseline_dependent_averaging

Code for averaging together baselines
BSD 2-Clause "Simplified" License
0 stars 0 forks source link

Bug in apply_bda #7

Open steven-murray opened 3 years ago

steven-murray commented 3 years ago

https://github.com/HERA-Team/baseline_dependent_averaging/blob/1e7b5bc3909bbdfdf40dc6743cbf35dc19392c46/bda/bda_tools.py#L395

This line fails with UnboundLocalError: local variable 'zenith_ra' referenced before assignment if the first branch of the if statement above is taken.

r-pascua commented 2 years ago

Following up on this: we came across this from a test in hera_sim failing. I looked into the issue, and my understanding is that this error will only ever crop up when trying to apply BDA to a data set that doesn't actually need BDA applied to it. I can think of two ways to address this: 1) Assume that analysts using this are sensible and will not apply BDA to data that doesn't need BDA applied to it, and so don't update the code. 2) Make apply_bda a little dummy proof by protecting this in a try-except clause, maybe like so:

try:
    uv2.phase_center_ra = zenith_ra.rad
    uv2.phase_center_dec = zenith_dec.rad
    uv2.phase_type = "phased"
    ...
except UnboundLocalError:  # no bda was applied
    uv2.phase_type = uv.phase_type
    uv2.phase_center_ra = uv.phase_center_ra
    ...

I don't understand the process well enough to know whether this is possible, but I suppose a third option would be to add an additional check at the beginning of the routine (with all the other checks that are run, before the uv2 object is created) to see if it even makes sense to apply BDA to the input uv, and just return uv.copy() if it doesn't make sense to apply BDA.