Skip to content
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 bending mode stress safety checks #79

Open
wants to merge 35 commits into
base: develop
Choose a base branch
from
Open

Conversation

gmegh
Copy link
Contributor

@gmegh gmegh commented Sep 9, 2024

No description provided.

@gmegh gmegh requested a review from tribeiro September 9, 2024 15:23
Copy link
Member

@tribeiro tribeiro left a comment

Choose a reason for hiding this comment

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

Here is an initial round of comments.

@@ -373,6 +386,26 @@ def get_dof_lv(self):

return self.ofc.lv_dof

def get_bending_mode_stresses(self):
Copy link
Member

Choose a reason for hiding this comment

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

Please, consider splitting this up into 2 methods, one for m2 and one for m1m3?

m1m3_bending_mode = BendingMode("M1M3", self.ofc.ofc_data)

m1m3_stresses = m1m3_bending_mode.stresses(
self.ofc.controller.aggregated_state[10:30]
Copy link
Member

Choose a reason for hiding this comment

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

I think these ranges (10-30) are set somewhere in the OFC configuration no?

self.ofc.controller.aggregated_state[10:30]
)
m2_stresses = m2_bending_mode.stresses(
self.ofc.controller.aggregated_state[30:]
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, I think these ranges are somewhere in ofc no?


self._logExecFunc()

stressesM1M3, stressesM2 = self.model.bending_mode_stresses()
Copy link
Member

Choose a reason for hiding this comment

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

Please use snake_case

stressesM1M3, stressesM2 = self.model.bending_mode_stresses()

# Calculate the total stress on the mirror
stressM1M3 = self.stress_scale_factor * np.sqrt(np.sum(np.square(stressesM1M3)))
Copy link
Member

Choose a reason for hiding this comment

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

snake_case

@@ -604,6 +618,70 @@ async def test_interruptWEP(self):
with self.assertRaises(salobj.AckError):
await run_wep_task

def test_stress_below_limit(self):
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't look like the next couple tests belongs here, or, at least, they need some work.

This file here contains tests for the CSC so we should make sure we create an instance of the CSC and we manipulate the CSC to test some behavior. This starts by adding

        async with self.make_csc(
            initial_state=salobj.State.STANDBY,
            config_dir=TEST_CONFIG_DIR,
            simulation_mode=0,
        ):
            ...

Then, you want to send a command to the CSC that would test the behavior you want. We now have the offsetDoF which probably can be used for what you want here.

it will look something like:

await self.remote.cmd_offsetDof.set_start(value=dof_aggr, timeout=STD_TIMEOUT)

Then you need to check the evens that where published as a result

dof = await self.assert_next_sample(self.remote.evt_degreeOfFreedom)
...

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.

2 participants