slaclab / pysmurf

Other
2 stars 9 forks source link

Bug fix for setup_notches for unassigned channels, add SystemConfigured check. #741

Closed swh76 closed 1 year ago

swh76 commented 1 year ago

Description

This PR implements a fix for issue737 and sodetlib issue300 where users were seeing duplicate data for unassigned channels stored in the freq_resp dictionary populated by setup_notches. The issue was that the method setup_notches uses to sweep channels can only run on assigned channels. That method is used because it is faster than sweeping every resonator serially, but a bug was causing it to populate the resp data of the unassigned channels, which it wasn't sweeping, with data from other channels. Here's where the bug was:

https://github.com/slaclab/pysmurf/blob/eb9f7be55e6e49446fc72621e21e85596f5962a6/python/pysmurf/client/tune/smurf_tune.py#L3974

Unassigned channels get assigned channel -1 by the assign_channels routine, which resulted in a misindexing here:

https://github.com/slaclab/pysmurf/blob/eb9f7be55e6e49446fc72621e21e85596f5962a6/python/pysmurf/client/tune/smurf_tune.py#L3975

resulting in a copy, although different for each band, of the wrong data being stored with every unassigned channel in the S.freq_resp dictionary.

I implemented the following fix : added a new optional keyword scan_unassigned to setup_notches which if False, just doesn't scan unassigned channels or if True it scans them serial after the time optimized sweep over the assigned channels. I made it optional, and default disabled, because while this is useful for resonator lab characterization, it would slow down tuning in the field. Here's what the entry to in the freq_resp dictionary for an unassigned channel looks like if it's not scanned:

In [23]: S.freq_resp[2]['resonances'][0]
Out[23]: 
{'freq': 5250.0,
 'eta': 1,
 'eta_scaled': 1,
 'eta_phase': 0,
 'r2': 1,
 'eta_mag': 1,
 'latency': 0,
 'Q': 1,
 'freq_eta_scan': None,
 'resp_eta_scan': None,
 'subband': 255,
 'channel': -1,
 'offset': 0.0}

Some other fixes in this PR: