Add stimulator functionality for flexible operation - #257
Conversation
BrianJKoopman
left a comment
BrianJKoopman
left a comment
There was a problem hiding this comment.
Thanks for the PR! And apologies on the slow reply. Comments below. The biggest one is related to whether these new functions should live here or in the agent itself. If you have examples of how you imagine auto-generated schedules to take advantage of these new functions, I'd find that helpful in assessing.
| import time | ||
| import sorunlib as run | ||
| from sorunlib._internal import check_response, protect_shutdown, stop_smurfs | ||
|
|
There was a problem hiding this comment.
Conventionally I'd leave a line between imports and the rest of the code.
| if start: | ||
| resp = blh.start_rotation(forward=forward) | ||
| check_response(blh, resp) |
There was a problem hiding this comment.
Since start is the default, what occurs if start_rotation() is called when rotation is already happening?
| def set_heater_voltage(volt, force=False): | ||
| """Set the PCR500MA source voltage to the given value immediately. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| volt : float | ||
| Target voltage in V. | ||
| force : bool, optional | ||
| If True, skip the discrepancy check between the target and measured | ||
| voltages. Defaults to False. | ||
| """ | ||
| assert volt < _VOLT_ULIM_SOFT, f'Target voltage exceeds {_VOLT_ULIM_SOFT}V.' | ||
|
|
||
| pcr = run.CLIENTS['stimulator']['pcr500ma'] | ||
|
|
||
| if not force: | ||
| response = pcr.acq.status() | ||
| v_meas = response.session['data']['V_AC'] | ||
| if abs(volt - v_meas) > _VOLT_ALLOW: | ||
| raise RuntimeError( | ||
| f'Discrepancy between target and measured voltages: ' | ||
| f'{volt} / {v_meas}' | ||
| ) | ||
|
|
||
| resp = pcr.set_volt_ac(volt_set=volt) | ||
| check_response(pcr, resp) | ||
|
|
||
|
|
||
| def set_to_0V_heater(): | ||
| """Set the PCR500MA source voltage to 0V when output is OFF. | ||
|
|
||
| This is intended for resetting the voltage setpoint while the output is | ||
| disabled, bypassing the normal ramp procedure. | ||
| """ | ||
| pcr = run.CLIENTS['stimulator']['pcr500ma'] | ||
|
|
||
| if pcr.get_output().session['data']['output']: | ||
| raise RuntimeError('Output is ON. Use ramp_heater to ramp down to 0V.') | ||
|
|
||
| set_heater_voltage(0, force=True) | ||
|
|
||
|
|
||
| def ramp_heater(volt, vstep=_VOLT_STEP): | ||
| """Ramp the PCR500MA source voltage to the target value. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| volt : float | ||
| Target voltage in V. | ||
| vstep : float, optional | ||
| Voltage step for ramping. Defaults to 1 V. | ||
| """ | ||
| assert volt < _VOLT_ULIM_SOFT, f'Target voltage exceeds {_VOLT_ULIM_SOFT}V.' | ||
|
|
||
| pcr = run.CLIENTS['stimulator']['pcr500ma'] | ||
|
|
||
| # Health check | ||
| _, _, s_meas = pcr.get_volt_ac() | ||
| v_target_tmp = s_meas['data']['volt_set'] | ||
|
|
||
| if not pcr.get_output().session['data']['output']: | ||
| raise RuntimeError('Output on/off status is OFF') | ||
|
|
||
| # Voltage plan | ||
| if v_target_tmp < volt: | ||
| n_step = int((volt - v_target_tmp) / vstep) | ||
| v_plan = [v_target_tmp + i * vstep for i in range(n_step)] | ||
| elif v_target_tmp > volt: | ||
| n_step = int((v_target_tmp - volt) / vstep) | ||
| v_plan = [v_target_tmp - i * vstep for i in range(n_step)] | ||
| else: | ||
| return | ||
|
|
||
| v_plan = v_plan[1:] + [volt] | ||
|
|
||
| for _v in v_plan: | ||
| time.sleep(0.5) | ||
| set_heater_voltage(_v) | ||
| time.sleep(_RAMP_TIME_STEP - 0.5) | ||
|
|
||
|
|
||
| def set_heater_output(output, force=False): | ||
| """Turn on or off the PCR500MA source output. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| output : bool | ||
| True to turn on, False to turn off. | ||
| force : bool, optional | ||
| If True, forcibly set the output bypassing safety checks. | ||
| Defaults to False. | ||
| """ | ||
| pcr = run.CLIENTS['stimulator']['pcr500ma'] | ||
| resp = pcr.set_output(output=output, force=force) | ||
| check_response(pcr, resp) | ||
|
|
||
|
|
||
| def heater_recovery(volt, vstep=_VOLT_STEP): | ||
| """Recover the PCR500MA heater output to the target voltage after a power | ||
| outage or unexpected shutdown. | ||
|
|
||
| Retrieves the current voltage setpoint and output state, then brings the | ||
| output back to ``volt`` via the following procedure: | ||
|
|
||
| - Output ON : ramp directly to ``volt``. | ||
| - Output OFF, setpoint > 0 : reset setpoint to 0 V with | ||
| :func:`set_to_0V_heater`, turn output on, then ramp to ``volt``. | ||
| - Output OFF, setpoint == 0 : turn output on, then ramp to ``volt``. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| volt : float | ||
| Target voltage in V. | ||
| vstep : float, optional | ||
| Voltage step for ramping. Defaults to 1 V. | ||
| """ | ||
| pcr = run.CLIENTS['stimulator']['pcr500ma'] | ||
|
|
||
| _, _, s_meas = pcr.get_volt_ac() | ||
| v_current = s_meas['data']['volt_set'] | ||
| output = pcr.get_output().session['data']['output'] | ||
|
|
||
| if output: | ||
| ramp_heater(volt, vstep=vstep) | ||
| else: | ||
| if v_current > 0: | ||
| set_to_0V_heater() | ||
| set_heater_output(True) | ||
| ramp_heater(volt, vstep=vstep) |
There was a problem hiding this comment.
In general I'd like to discuss these new functions and which of them should really live in the agent instead.
ramp_heater, for instance, and the associated constants defined to limit the voltage and set step size/ramp time would make more sense (at least to me) to make into a task within the agent itself. As written here the function only interacts with the pcr500ma agent. If it required interacting with multiple stimulator/other components, I'd agree it'd belong here.
set_heater_voltage is essentially just the set_volt_ac task with protections. Should these protections (and optional force) just exist within the set_volt_ac task?
heater_recovery (which I would probably rename to recover_heater), also seems like it'd be useful as a task.
It seems like these are being built more for interactive use by a stimulator expert, than for regularly scheduled execution -- heater_recovery being a great example of that. I don't expect it would show up regularly in schedules, but would be called in one-off scripts by an expert trying to recover the system. Please correct me if I'm wrong. Do you have examples of how these functions will be used in every day schedules?
There was a problem hiding this comment.
Thanks for adding tests! There are still several code paths that are not covered. You can see them highlighted in red here.
If functions stay here instead of moving to the agent (see my other comment on stimulator.py), then I'd like for additional tests that cover those paths to be added. But again, only if the new functions don't move to the agent as tasks.
2 participants
Summary
This PR extends stimulator.py with functions for direct PCR500MA heater control and chopper operation, and adds corresponding tests.
New public functions
open_shutter() / close_shutter()— previously private; promoted to public APIrotate(speed_rpm, forward=True, start=True)— set chopper speed and optionally start rotation; start=False allows speed-only updates (e.g. mid-run speed changes)stop_rotation()— stop the chopper; previously inlined in _stop()set_heater_voltage(volt, force=False)— set PCR500MA output voltage with discrepancy check; force=True bypasses the checkramp_heater(volt, vstep=1)— ramp to target voltage in steps, with output-on guardset_to_0V_heater()— reset voltage setpoint to 0 V when output is OFFset_heater_output(output, force=False)— turn PCR500MA output on/off