Skip to content

Wrap setup_amps in try during relock for two attempts#508

Open
tpsatt wants to merge 4 commits into
masterfrom
retry_amps
Open

Wrap setup_amps in try during relock for two attempts#508
tpsatt wants to merge 4 commits into
masterfrom
retry_amps

Conversation

@tpsatt

@tpsatt tpsatt commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Semi-frequently after a hammer setup_amps fails on uxm_relock, usually due to a key error for the name of one of the amplifiers. It's pretty much always fixed by running uxm_relock again, but ROCs don't know to do this and smurf-tsars can be slow to respond.

This is a very simple solution of wrapping setup_amps in a try. If it fails, it tries again. If the second attempt fails, then it errors out.

@tpsatt tpsatt requested review from dpdutcher and tristpinsm June 11, 2026 21:00

@tristpinsm tristpinsm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not loving the nested try/except here but ok :)

Comment thread sodetlib/operations/uxm_relock.py Outdated
Comment thread sodetlib/operations/uxm_relock.py Outdated
@dpdutcher

Copy link
Copy Markdown
Collaborator

Yeah I also don't love the nested trys. Maybe include it in a for-loop, like

for attempt in range(max_attempts):
    try:
        <x>
        break
    except Exception as e:
        <log message>
else:
    raise e

Also, could this logic be moved within setup_amps itself, instead of being in uxm_relock?

@tristpinsm tristpinsm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, sorry I approved too soon. One more request: Could we log the exception when it is caught? Otherwise it gets really hard to figure out what is going on.

Comment thread sodetlib/operations/uxm_relock.py Outdated
Change to while loop and handle Exception explicitly

Co-authored-by: Tristan Pinsonneault-Marotte <tristpinsm@gmail.com>

@dpdutcher dpdutcher left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this logic be moved to be within uxm_setup.setup_amps instead?
Here:

amp_biases = S.get_amplifier_biases()
# For C04, first check drain voltages
if cc_rev == 'c04':
for amp in amp_list:
Vd = amp_biases[f"{amp}_drain_volt"]

That's the cause of the failure and where it should be caught and dealt with, not in this function. uxm_setup.uxm_setup also has a call to setup_amps, and repeating this check there would duplicate code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants