-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add time_constant function #179
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -198,6 +198,112 @@ def _check_temperature_sensors(): | |
_verify_temp_response(resp, 'AIN2C', 0) | ||
|
||
|
||
def _check_wiregrid_position(): | ||
"""Check the wiregrid position.""" | ||
actuator = run.CLIENTS['wiregrid']['actuator'] | ||
resp = actuator.acq.status() | ||
ls_status = resp.session['data']['fields']['limitswitch'] | ||
if (ls_status['LSR1'] == 1 or ls_status['LSL1'] == 1) and not (ls_status['LSR2'] == 1 and ls_status['LSL2'] == 1): | ||
position = 'outside' | ||
elif (ls_status['LSR2'] == 1 or ls_status['LSL2'] == 1) and not (ls_status['LSR1'] == 1 or ls_status['LSL1'] == 1): | ||
position = 'inside' | ||
else: | ||
raise RuntimeError("The wiregrid position is unknown. The limitswitch response is like this:\n" + | ||
"LSR1: {}\n".format('ON' if ls_status['LSR1'] == 1 else 'OFF') | ||
+ "LSL1: {}\n".format('ON' if ls_status['LSL1'] == 1 else 'OFF') | ||
+ "LSR2: {}\n".format('ON' if ls_status['LSR2'] == 1 else 'OFF') | ||
+ "LSL2: {}\n".format('ON' if ls_status['LSL2'] == 1 else 'OFF') | ||
+ "Aborting...") | ||
Comment on lines
+205
to
+216
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've wondered if there was a method for determining whether the wiregrid was inserted or not for a while now. It would be better to put this logic into the Agent itself, within the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with you. I created the PR to implement the position data in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great, I just reviewed that. Once it's merged we should drop the code here and use the |
||
return position | ||
|
||
|
||
def _stop_hwp_with_wiregrid(): | ||
"""Stop the HWP after comfirming the wiregrid is inserted.""" | ||
# Check the wiregrid position | ||
position = _check_wiregrid_position() | ||
if position == 'outside': | ||
raise RuntimeError("The wiregrid is not inserted. Aborting...") | ||
elif position == 'inside': | ||
# Stop the HWP | ||
print("Starting to stop the HWP.") | ||
run.hwp.stop(active=True) | ||
print("The HWP stopped successfully.") | ||
else: | ||
raise RuntimeError("Unknown wiregrid position. Aborting...") | ||
Comment on lines
+231
to
+232
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
|
||
def _spin_hwp_with_wiregrid(target_hwp_direction): | ||
"""Spin the HWP to the target direction at 2Hz after comfirming the wiregrid is inserted. | ||
|
||
Args: | ||
target_hwp_direction (str): Target HWP direction, 'forward' or 'backward'. | ||
""" | ||
# Check the wiregrid position | ||
position = _check_wiregrid_position() | ||
Comment on lines
+241
to
+242
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The checks in both If you still want to check the wiregrid is inserted, I'd say do the check in def _stop_hwp():
"""Stop the HWP."""
print("Starting to stop the HWP.")
run.hwp.stop(active=True)
print("The HWP stopped successfully.") def _spin_hwp(target_hwp_direction):
"""Spin the HWP to the target direction at 2Hz.
Args:
target_hwp_direction (str): Target HWP direction, 'forward' or 'backward'.
"""
if target_hwp_direction == 'forward':
print("Starting to spin up the HWP in forward.")
run.hwp.set_freq(freq=2.0)
print("The HWP is spinning at 2Hz in forward successfully.")
elif target_hwp_direction == 'backward':
print("Starting to spin up the HWP in backward.")
run.hwp.set_freq(freq=-2.0)
print("The HWP is spinning at 2Hz in backward successfully.") |
||
if position == 'outside': | ||
raise RuntimeError("The wiregrid is not inserted. Aborting...") | ||
elif position == 'inside': | ||
if target_hwp_direction == 'forward': | ||
print("Starting to spin up the HWP in forward.") | ||
run.hwp.set_freq(freq=2.0) | ||
print("The HWP is spinning at 2Hz in forward successfully.") | ||
elif target_hwp_direction == 'backward': | ||
print("Starting to spin up the HWP in backward.") | ||
run.hwp.set_freq(freq=-2.0) | ||
print("The HWP is spinning at 2Hz in backward successfully.") | ||
else: | ||
raise RuntimeError("Unknown HWP target direction. Aborting...") | ||
else: | ||
raise RuntimeError("Unknown wiregrid position. Aborting...") | ||
|
||
|
||
def _reverse_hwp_with_wiregrid(initial_hwp_direction, streaming=False, stepwise_before=False, stepwise_after=False): | ||
"""Change the HWP direction after comfirming the wiregrid is inserted. | ||
|
||
Args: | ||
initial_hwp_direction (str): Initial HWP direction, 'forward' or 'backward'. | ||
streaming (bool): Do SMuRF streaming during the HWP direction chaging or not . Default is False. | ||
stepwise_before (bool): Do stepwise rotation before changing HWP direction or not. Default is False. | ||
stepwise_after (bool): Do stepwise rotation after changing HWP direction or not. Default is False. | ||
""" | ||
if initial_hwp_direction not in ['forward', 'backward']: | ||
raise RuntimeError("Initial HWP direction should be either 'forward' or 'backward'. Aborting...") | ||
|
||
current_hwp_direction = initial_hwp_direction | ||
if current_hwp_direction == 'forward': | ||
target_hwp_direction = 'backward' | ||
elif current_hwp_direction == 'backward': | ||
target_hwp_direction = 'forward' | ||
Comment on lines
+273
to
+276
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to make sure we get this convention correct. The HWP supervisor agent has an argument called (I tried to avoid this question myself when writing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found the confluence page about the rotation direction definition: CHWP main page -- Rotation direction definition. The cw/ccw seems to correspond to the PID value depending on each satp and the PID corresponds to 'forward' or 'reverse' not depending on the satp. We should pay attention which physical direction the 'forward' and 'reverse' mean - cw or ccw. This document uses 'reverse' instead of 'backward'. It may be good time to integrate the convention. |
||
|
||
# Stop and spin up reversely the HWP | ||
try: | ||
# Enable SMuRF streams | ||
if streaming: | ||
stream_tag = f'wiregrid, wg_time_constant, hwp_change_to_{target_hwp_direction}' | ||
if stepwise_before or stepwise_after: | ||
stream_tag += ', wg_stepwise' | ||
if _check_zenith(): | ||
stream_tag += ', wg_el90' | ||
run.smurf.stream('on', subtype='cal', tag=stream_tag) | ||
# Stepwise rotation before stopping the HWP | ||
if stepwise_before: | ||
rotate(False) | ||
# Stop the HWP | ||
_stop_hwp_with_wiregrid() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest to add following here, to stream separately for forward and backward
|
||
# Spin up the HWP in the opposite direction | ||
_spin_hwp_with_wiregrid(target_hwp_direction) | ||
current_hwp_direction = target_hwp_direction | ||
# Stepwise rotation after spinning up the HWP | ||
if stepwise_after: | ||
rotate(False) | ||
finally: | ||
# Stop SMuRF streams | ||
if streaming: | ||
run.smurf.stream('off') | ||
|
||
return current_hwp_direction | ||
|
||
|
||
# Public API | ||
def insert(): | ||
"""Insert the wiregrid.""" | ||
|
@@ -305,3 +411,100 @@ def calibrate(continuous=False, elevation_check=True, boresight_check=True, | |
finally: | ||
# Stop SMuRF streams | ||
run.smurf.stream('off') | ||
|
||
|
||
def time_constant(initial_hwp_direction, stepwise_first=True, stepwise_last=True, stepwise_mid=False, repeat=1): | ||
""" | ||
Run a wiregrid time constant measurement. | ||
|
||
Args: | ||
initial_hwp_direction (str): Initial HWP direction, 'forward' or 'backward'. | ||
stepwise_first (bool): Do stepwise rotation or not before the first HWP speed change. Default is True. | ||
stepwise_last (bool): Do stepwise rotation or not after the last HWP speed change. Default is True. | ||
stepwise_mid (bool): Do stepwise rotation between each HWP speed change. Default is False. | ||
repeat (int): Number of repeats. Default is 1. | ||
If this is odd, the HWP direction will be changed to the opposite of the initial direction. | ||
If this is even, the HWP direction will be the same as the initial direction. | ||
""" | ||
|
||
# Check the initial HWP direction | ||
if initial_hwp_direction not in ['forward', 'backward']: | ||
raise RuntimeError("Initial HWP direction should be either 'forward' or 'backward'. Aborting...") | ||
current_hwp_direction = initial_hwp_direction | ||
Comment on lines
+430
to
+433
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This must be able to be determined automatically, no? I would think from the monitor process' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I asked the HWP team about how to know its current spin direction in a robust and fast way, but I was told there is no way. The scheduler can remember the last command to set the direction (the arugment in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'direction' in session.data of pid controller agent might be good for initial direction? This value is not reliable when we spin down hwp, or when hwp is stopped, but it is reliable when hwp is spinning stablly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now, I assumed the scheduler knows the HWP spin direction but this is also under the condition which the HWP is already spinning. Using 'direction' would be more simple than now. I will modify. Thanks Kyohei! |
||
|
||
# Check the repeat | ||
if repeat < 1 or not isinstance(repeat, int): | ||
raise RuntimeError("The repeat should be int and larger than 0. Aborting...") | ||
|
||
_check_agents_online() | ||
_check_motor_on() | ||
_check_telescope_position(elevation_check=True, boresight_check=False) | ||
|
||
if _check_zenith: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
el_tag = ', wg_el90' | ||
else: | ||
el_tag = '' | ||
|
||
# Rotate for reference before insertion | ||
rotate(continuous=True, duration=10) | ||
|
||
# Bias step (the wire grid is off the window) | ||
bs_tag = f'biasstep, wg_time_constant, wg_ejected, hwp_2hz_{current_hwp_direction}' + el_tag | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A few comments on these tags:
|
||
run.smurf.bias_step(tag=bs_tag, concurrent=True) | ||
|
||
# Insert the wiregrid with streaming | ||
time.sleep(5) | ||
try: | ||
# Enable SMuRF streams | ||
stream_tag = f'wg_time_constant, wg_inserting, hwp_2hz_{current_hwp_direction}' + el_tag | ||
run.smurf.stream('on', tag=stream_tag, subtype='cal') | ||
# Insert the wiregrid | ||
insert() | ||
finally: | ||
# Stop SMuRF streams | ||
run.smurf.stream('off') | ||
time.sleep(5) | ||
|
||
for i in range(repeat): | ||
# Bias step (the wire grid is on the window) | ||
bs_tag = f'biasstep, wg_time_constant, wg_inserted, hwp_2hz_{current_hwp_direction}' + el_tag | ||
run.smurf.bias_step(tag=bs_tag, concurrent=True) | ||
|
||
stepwise_before = False | ||
stepwise_after = False | ||
if stepwise_first and i == 0: | ||
# Stepwise rotation before the first HWP speed change | ||
stepwise_before = True | ||
if stepwise_mid and i != 0: | ||
# Stepwise rotation between changing HWP speed | ||
stepwise_before = True | ||
if stepwise_last and i == repeat - 1: | ||
# Stepwise rotation after the last HWP speed change | ||
stepwise_after = True | ||
|
||
# Spin the HWP in the opposite direction of the initial direction with streaming and stepwise rotation | ||
current_hwp_direction = _reverse_hwp_with_wiregrid(current_hwp_direction, | ||
streaming=True, | ||
stepwise_before=stepwise_before, | ||
stepwise_after=stepwise_after) | ||
|
||
# Bias step (the wire grid is on the window) | ||
bs_tag = f'biasstep, wg_time_constant, wg_inserted, hwp_2hz_{current_hwp_direction}' + el_tag | ||
run.smurf.bias_step(tag=bs_tag, concurrent=True) | ||
|
||
# Eject the wiregrid with streaming | ||
time.sleep(5) | ||
try: | ||
# Enable SMuRF streams | ||
stream_tag = f'wg_time_constant, wg_ejecting, hwp_2hz_{current_hwp_direction}' + el_tag | ||
run.smurf.stream('on', tag=stream_tag, subtype='cal') | ||
# Eject the wiregrid | ||
eject() | ||
finally: | ||
# Stop SMuRF streams | ||
run.smurf.stream('off') | ||
time.sleep(5) | ||
|
||
# Bias step (the wire grid is off the window) | ||
bs_tag = f'biasstep, wg_time_constant, wg_ejected, hwp_2hz_{current_hwp_direction}' + el_tag | ||
run.smurf.bias_step(tag=bs_tag, concurrent=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this stays here for the moment...pre-commit is complaining about this particular line. Move the
+
down from line 211 to the start of line 212, like you have the rest of the lines starting with+
.