simonsobs / sodetlib

Tools for performing core instrument testing, quality control, and analysis tasks.
BSD 2-Clause "Simplified" License
5 stars 0 forks source link

optimize_tracking() turns off all channels, returns ValueError #66

Closed dpdutcher closed 3 years ago

dpdutcher commented 3 years ago

The following lines in optimize_params.optimize_tracking() cause tune_and_optimize.py to crash with a ValueError:

f, _, _ = S.tracking_setup(**tracking_kwargs)
f_span = np.max(f, 0) - np.min(f, 0)

for c in S.which_on(band):
    if not 0.03 < f_span[c] < 0.25:
        S.channel_off(band, c)

# Reruns to get re-estimate of tracking freq
S.tracking_setup(**tracking_kwargs)

The for-loop turns off all channels, and the second tracking_setup() call then returns the ValueError.

jlashner commented 3 years ago

Do you know what the normal f_span are for the device you're testing? I think Yuhan mentioned this problem on slack, and the reason was because the fspan you guys were seeing was more like 20 kHz (instead of the 30 kHz minimum).

The solution we talked about was allowing the user to pass fspan range as params to the function.

Now that I think about it, I think that it might be even better to use the tracking_quality r-squared values to make the cut instead of f-span. Seems like that is more reliable.

dpdutcher commented 3 years ago

Well not only do I not know what the f_span for this device is, I don't know what "f_span" is in general.

jlashner commented 3 years ago

Haha fspan is just what we've been calling the magnitude of the frequency shift of the resonators during tracking setup. We were using it to try to determine which channels were tracking well before we had tracking quality.

So it's just the max(f) - min(f)

dpdutcher commented 3 years ago

So at least in this case, this was due to tracking being poor as a result of very different refPhaseDelay from what was in the pysmurf config file. After fixing that, just a few channels were dropped and this part of the code finished without error.

So perhaps running S.estimate_phase_delay(band) should be a part of this script somewhere? And perhaps an exception thrown if the active channel list is empty.

msilvafe commented 3 years ago

I agree with @dpdutcher but maybe more generally we need to consider how sodetlib will handle certain functions that need to be run in a particular order. I think we have this for some things in the device config where functions will check the device config to see if the optimization has already been run or not and make a decision based off of that. Maybe we should audit that and come up with a reasonable logic (i.e. an eta phase, rough power check, and whether or not tuning + serial algs are run is a must before doing anything with tracking and if not either an error is called or those functions are run before automatically).

In general though I think we should just replace this part of the code w/ tracking_quality if we think this is universally better than the f_span cut.