Autos only abs amp logcal #832
Conversation
|
I think passing both |
Codecov ReportBase: 97.14% // Head: 97.06% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #832 +/- ##
==========================================
- Coverage 97.14% 97.06% -0.09%
==========================================
Files 18 16 -2
Lines 8445 8101 -344
==========================================
- Hits 8204 7863 -341
+ Misses 241 238 -3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| @@ -3436,29 +3443,62 @@ def post_redcal_abscal(model, data, data_wgts, rc_flags, edge_cut=0, tol=1.0, ke | |||
| data_wgts=data_wgts, tol=tol, keep_flagged_ants=True) | |||
| reds = redcal.get_reds(idealized_antpos, pols=data.pols(), bl_error_tol=redcal.IDEALIZED_BL_TOL) | |||
|
|
|||
| # separate autos and crosses if running amplitude calibration with autos only: | |||
There was a problem hiding this comment.
I think you've made this function way more complicated than it needs to be. Instead of splitting up the passed data and model, I think instead one should pass in a separate autos_model and autos_data, which default to None but must be specified if autos_only_abs_amp_logcal is True
There was a problem hiding this comment.
I think this makes sense, and I will make this change
| @@ -3212,6 +3212,13 @@ def match_baselines(data_bls, model_bls, data_antpos, model_antpos=None, pols=[] | |||
| data_bl_to_load = set(utils.filter_bls(data_bls, pols=pols, antpos=data_antpos, min_bl_cut=min_bl_cut, max_bl_cut=max_bl_cut)) | |||
| model_bl_to_load = set(utils.filter_bls(model_bls, pols=pols, antpos=model_antpos, min_bl_cut=min_bl_cut, max_bl_cut=max_bl_cut)) | |||
|
|
|||
| #Add autos to bl_to_load if we are running with autos_only_abs_amp_logcal: | |||
| if autos_only_abs_amp_logcal: | |||
| autos_data = [bl for bl in data_bls if bl[0]==bl[1] and bl[2] in pols] | |||
There was a problem hiding this comment.
I'd be careful about using bl[0] == bl[1] for finding autos, since you might end up identifying (1, 1, 'ne') as an auto, which it's not. Instead use utils.split_bl()
| df = np.median(np.diff(data.freqs)) | ||
| for time_avg in [True, False]: # first use the time-averaged solution to try to avoid false minima | ||
| gains_here = delay_slope_lincal(model, data, idealized_antpos, wgts=binary_wgts, df=df, f0=data.freqs[0], medfilt=True, kernel=kernel, | ||
| if autos_only_abs_amp_logcal: |
There was a problem hiding this comment.
Having a separate call to each calibration function depending on whether autos_only_abs_amp_logcal is true or makes the code a lot more complex and annoying to test and maintain.
changed steps 1 and 2 in post_redcal_abscal_run