Skip to content

Conversation

@ykyohei
Copy link
Contributor

@ykyohei ykyohei commented Apr 24, 2025

Add estimate of rotation direction to HWPSupervisor.HWPState

Description

Estimate the absolute rotation direction from pid_state and control_state and add estimated direction to HWPSupervisor.HWPState
Need to merge https://github.com/simonsobs/ocs-deployment-configs/pull/436 to have correct rotation direction estimate.

Motivation and Context

The real time absolute rotation direction estimate is necessary for wiregrid time constant measurement simonsobs/sorunlib#179

How Has This Been Tested?

Need to test

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@ykyohei
Copy link
Contributor Author

ykyohei commented May 1, 2025

Okay I was able to run some tests and confirmed this works nothing crashes, but I couldn't confirm the behavior when we run brake command. I needed to run some hwp agents in daq-dev to test and I got many timeous from pid controller at daq-dev.

When we run brake, ControlState transition to Brake, WaitForBrake and finally WaitForTragetFreq when rotation frequency is below 0.5 Hz. At the last state, the direction estimate can flip but I think this is fine.
Also the behavior of direction when we brake hwp is not critical for wiregrid measurement, so I think current state is enough to start review. If we need to test brake, I think we would like to run this supervisor agent in daq-satp3.
What do you think @BrianJKoopman ?

@BrianJKoopman
Copy link
Member

Okay I was able to run some tests and confirmed this works nothing crashes, but I couldn't confirm the behavior when we run brake command. I needed to run some hwp agents in daq-dev to test and I got many timeous from pid controller at daq-dev.

Glad you got the tests working! Maybe there's a network configuration issue on the pid controller? daq-dev is on a different subnet. We can debug offline sometime if you'd like.

When we run brake, ControlState transition to Brake, WaitForBrake and finally WaitForTragetFreq when rotation frequency is below 0.5 Hz. At the last state, the direction estimate can flip but I think this is fine. Also the behavior of direction when we brake hwp is not critical for wiregrid measurement, so I think current state is enough to start review. If we need to test brake, I think we would like to run this supervisor agent in daq-satp3. What do you think @BrianJKoopman ?

Yeah, it sounds like this is ready for review. Testing on daq-satp3 sounds fine to me.

@ykyohei ykyohei marked this pull request as ready for review May 2, 2025 18:42
@ykyohei
Copy link
Contributor Author

ykyohei commented May 2, 2025

I made this ready to review. Is it possible for me to run test on daq-satp3? I do not know how to do that.

@BrianJKoopman
Copy link
Member

I made this ready to review. Is it possible for me to run test on daq-satp3? I do not know how to do that.

This is possible, I'll describe the process once I complete the review.

@BrianJKoopman BrianJKoopman self-requested a review May 2, 2025 20:41
Copy link
Member

@BrianJKoopman BrianJKoopman left a comment

Choose a reason for hiding this comment

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

This looks good to me. I'll reach out on Slack to talk about testing on site. (The process will be pretty similar to the one I recently described here.)

@ykyohei
Copy link
Contributor Author

ykyohei commented May 7, 2025

I did additional test at daq-satp3 and I think this is working for Brake too.
This is the example of state in every 5 seconds. when we brake hwp.
When pid controller sends many commnds, it sometimes rerturns 0 frequency, and this is leading to direction = None.
The logic of update_spin_state when Brake state is preventing wrong direction estimate.

# is_spinning, direction, pid_target_freq, pid_current_freq, pid_direction, pid_last_updated, supervisor_control_state
True ccw 2.0 1.955 0 1746630689.597393 {'state_type': 'Idle', 'start_time': 1746630688.6138647, 'last_update_time': 1746630691.3516986}
True ccw 2.0 1.959 0 1746630694.7054486 {'state_type': 'Idle', 'start_time': 1746630688.6138647, 'last_update_time': 1746630696.3587086}
True ccw 2.0 1.964 0 1746630699.8136544 {'state_type': 'Idle', 'start_time': 1746630688.6138647, 'last_update_time': 1746630701.3673093}
True ccw 2.0 1.967 0 1746630704.9216154 {'state_type': 'Idle', 'start_time': 1746630688.6138647, 'last_update_time': 1746630707.376915}
True ccw 2.0 1.97 0 1746630710.0289612 {'state_type': 'Brake', 'start_time': 1746630709.2651365, 'last_update_time': 0, 'freq_tol': 0.05, 'freq_tol_duration': 10.0, 'brake_voltage': 10.0}
True ccw 2.0 1.97 0 1746630710.0289612 {'state_type': 'Brake', 'start_time': 1746630709.2651365, 'last_update_time': 0, 'freq_tol': 0.05, 'freq_tol_duration': 10.0, 'brake_voltage': 10.0}
False None 0.0 0.0 1 1746630718.7484915 {'state_type': 'Brake', 'start_time': 1746630709.2651365, 'last_update_time': 0, 'freq_tol': 0.05, 'freq_tol_duration': 10.0, 'brake_voltage': 10.0}
True None 0.0 1.931 1 1746630723.856315 {'state_type': 'Brake', 'start_time': 1746630709.2651365, 'last_update_time': 0, 'freq_tol': 0.05, 'freq_tol_duration': 10.0, 'brake_voltage': 10.0}
True None 0.0 1.912 1 1746630728.9643838 {'state_type': 'Brake', 'start_time': 1746630709.2651365, 'last_update_time': 0, 'freq_tol': 0.05, 'freq_tol_duration': 10.0, 'brake_voltage': 10.0}
True None 0.0 1.892 1 1746630734.071784 {'state_type': 'WaitForBrake', 'start_time': 1746630737.904163, 'last_update_time': 0, 'min_freq': 0.5, 'prev_freq': 1.9664684491384465}
True None 0.0 1.87 1 1746630739.1795752 {'state_type': 'WaitForBrake', 'start_time': 1746630737.904163, 'last_update_time': 0, 'min_freq': 0.5, 'prev_freq': 1.9664684491384465}

As mentioned before, direction estimate flip when control state transition to 'WaitForTargetFreq' at low frequency but I think this is fine,

True None 0.0 0.545 1 1746630989.793582 {'state_type': 'WaitForBrake', 'start_time': 1746630737.904163, 'last_update_time': 1746630978.880162, 'min_freq': 0.5, 'prev_freq': 1.9664684491384465}
True None 0.0 0.496 1 1746630994.804739 {'state_type': 'WaitForBrake', 'start_time': 1746630737.904163, 'last_update_time': 1746630978.880162, 'min_freq': 0.5, 'prev_freq': 1.9664684491384465}
True None 0.0 0.459 1 1746630999.8148139 {'state_type': 'WaitForBrake', 'start_time': 1746630737.904163, 'last_update_time': 1746630988.9021518, 'min_freq': 0.5, 'prev_freq': 1.9664684491384465}
True None 0.0 0.401 1 1746631004.8255637 {'state_type': 'WaitForBrake', 'start_time': 1746630737.904163, 'last_update_time': 1746630988.9021518, 'min_freq': 0.5, 'prev_freq': 1.9664684491384465}
True cw 0.0 0.345 1 1746631009.836347 {'state_type': 'WaitForTargetFreq', 'start_time': 1746631011.2548249, 'last_update_time': 1746631012.2561884, 'target_freq': 0, 'freq_tol': 0.05, 'freq_tol_duration': 30, 'freq_within_tol_start': None, 'max_duration': None, 'direction': '', '_pcu_enabled': False}
True cw 0.0 0.32 1 1746631014.9446914 {'state_type': 'WaitForTargetFreq', 'start_time': 1746631011.2548249, 'last_update_time': 1746631017.2650402, 'target_freq': 0, 'freq_tol': 0.05, 'freq_tol_duration': 30, 'freq_within_tol_start': None, 'max_duration': None, 'direction': '', '_pcu_enabled': False}

@BrianJKoopman
Copy link
Member

When pid controller sends many commnds, it sometimes rerturns 0 frequency, and this is leading to direction = None.

Is this a problem? I don't really think it is, since the time constant function proposed in sorunlib checks the HWP direction while it's rotating before commanding it stop and spin up in the opposite direction.

That said, it does seem like we should be able to accurately determine the direction even while braking if those occasional 0's are solved. For instance:

True ccw 2.0 1.97 0 1746630710.0289612 {'state_type': 'Brake', 'start_time': 1746630709.2651365, 'last_update_time': 0, 'freq_tol': 0.05, 'freq_tol_duration': 10.0, 'brake_voltage': 10.0}
False None 0.0 0.0 1 1746630718.7484915 {'state_type': 'Brake', 'start_time': 1746630709.2651365, 'last_update_time': 0, 'freq_tol': 0.05, 'freq_tol_duration': 10.0, 'brake_voltage': 10.0}
True None 0.0 1.931 1 1746630723.856315 {'state_type': 'Brake', 'start_time': 1746630709.2651365, 'last_update_time': 0, 'freq_tol': 0.05, 'freq_tol_duration': 10.0, 'brake_voltage': 10.0}

It's clear that in the third data point it's still spinning ccw.

Anyway, if you're happy with the performance now, I'm happy. And that'll enable the time constant PR to move forward. Shall we (squash and) merge?

@ykyohei
Copy link
Contributor Author

ykyohei commented May 7, 2025

Actually, I think this is same issue as this. #803
and satp3 pid controller version is v0.5.2 which is older than this. updating pid controller should fix this issue. I will make PR for version update.

@BrianJKoopman BrianJKoopman self-requested a review May 7, 2025 17:30
Copy link
Member

@BrianJKoopman BrianJKoopman left a comment

Choose a reason for hiding this comment

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

Alright, I think this is good to go now. When deploying on site we should make sure the forward direction configuration change happens at the same time.

@BrianJKoopman BrianJKoopman merged commit 6d974f8 into main May 7, 2025
5 checks passed
@BrianJKoopman BrianJKoopman deleted the hwp_direction branch May 7, 2025 17:33
@BrianJKoopman BrianJKoopman changed the title Add estimate of rotation direction to HWPSupervisor.HWPState hwp-supervisor: Add estimate of rotation direction to HWPState May 7, 2025
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