Streamline cleaning functionality in `fourier_filter` and `high_pass_fourier_filter`

HERA-Team / uvtools

Tools useful for the handling, visualization, and analysis of interferometric data.

MIT License

1 stars 1 forks source link

Streamline cleaning functionality in `fourier_filter` and `high_pass_fourier_filter` #70

Open aewallwi opened 4 years ago

aewallwi commented 4 years ago

fourier_filter has lots of nearly duplicate code from high_pass_fourier_filter for doing cleaning. high_pass_fourier_filter has some half baked dayenu functionality that is no longer needed. Ideally, fourier_filter should be able to call high_pass_fourier_filter for it's cleaning functionality. The holdup on this is that high_pass_fourier_filter does not support cleaning of arbitrary numbers areas not centered around delay=0. If we generalize high_pass_fourier_filter to support off center cleaning, we can just have fourier_filter call it and avoid lots of redundant code. We should also remove the half-baked dayenu functionality and rename it to something more appropriate like vis_clean since it will do cleaning only.

jsdillon commented 4 years ago

Wouldn't it make more sense to go the other way? Have vis_clean call fourier_filter?

aewallwi commented 4 years ago

Wouldn't it make more sense to go the other way? Have vis_clean call fourier_filter?

Yes.

aewallwi commented 4 years ago

Wouldn't it make more sense to go the other way? Have vis_clean call fourier_filter?

Yes.

I agree with @jsdillon here. Let's rename high_pass_fourier_filter to something like vis_clean, throw out the half-baked dayenu functionality, and have it call fourier_filter

jsdillon commented 4 years ago

I'm fine with that as long as we maintain a high_pass_fourier_filter function (even as a thin wrapper) to maintain backwards compatibility.

(we can stick in a deprecation warning though)

jsdillon commented 4 years ago

Also, I would suggest not naming it visclean, since it's also used on calibration solutions. Maybe CLEAN_filter or clean_filter?

aewallwi commented 4 years ago

Some discussion items with @jsdillon

move fitting_options to kwargs in fourier_filter.
Also specify that the returned model will have zeros in the flags for dayenu in the fourier_filter docstring.
Make it clear that one and only one fitting_option parameter should be provided. We can choose default versions for these kwargs just like for clean.
get rid of filter2d arg of fourier_filter and just determine from filter_dims.
move max_contiguous_edge_flags to kwargs

Here is the planned organization we have settled on.