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

Tickets/DM-47619 Update implementation of the ignore feature in scripts. #169

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
10 changes: 10 additions & 0 deletions doc/news/DM-47619.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Update the implementation of the ignore feature in all scripts to use the ``RemoteGroup.disable_checks_for_components`` method.

Updated scripts:
- ``base_parameter_march.py``
- ``base_take_ptc_flats.py``
- ``maintel/tma/random_walk.py``
- ``maintel/tma/random_walk_and_take_image_gencam.py``
- ``maintel/tma/serpent_walk.py``
- ``base_take_twilight_flats.py``

17 changes: 4 additions & 13 deletions python/lsst/ts/externalscripts/base_parameter_march.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,19 +245,10 @@ async def configure(self, config: types.SimpleNamespace) -> None:
await self.configure_ocps()

if hasattr(config, "ignore"):
for comp in config.ignore:
if comp in self.tcs.components_attr:
self.log.debug(f"Ignoring TCS component {comp}.")
setattr(self.tcs.check, comp, False)
elif comp in self.camera.components_attr:
self.log.debug(f"Ignoring Camera component {comp}.")
setattr(self.camera.check, comp, False)
else:
self.log.warning(
f"Component {comp} not in CSC Groups. "
f"Must be one of {self.tcs.components_attr} or "
f"{self.camera.components_attr}. Ignoring."
)
self.log.debug("Ignoring TCS components.")
self.tcs.disable_checks_for_components(components=config.ignore)
self.log.debug("Ignoring Camera components.")
self.camera.disable_checks_for_components(components=config.ignore)

if hasattr(config, "step_sequence"):
self.step_sequence = config.step_sequence
Expand Down
14 changes: 3 additions & 11 deletions python/lsst/ts/externalscripts/base_take_ptc_flats.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ def get_schema(cls):
ignore:
description: >-
CSCs from the camera group to ignore in status check.
Name must match those in self.group.components.
Name must match those in self.camera.components or
self.camera.components_attr.
type: array
items:
type: string
Expand Down Expand Up @@ -165,16 +166,7 @@ async def configure(self, config: types.SimpleNamespace):
await self.configure_electrometer(config.electrometer_scan["index"])

if hasattr(config, "ignore"):
for comp in config.ignore:
if comp in self.camera.components_attr:
self.log.debug(f"Ignoring Camera component {comp}.")
setattr(self.camera.check, comp, False)
else:
self.log.warning(
f"Component {comp} not in CSC Group. "
f"Must be one of {self.camera.components_attr}. "
f"Ignoring."
)
self.camera.disable_checks_for_components(components=config.ignore)

# Handle interleave darks settings
if hasattr(config, "interleave_darks"):
Expand Down
15 changes: 2 additions & 13 deletions python/lsst/ts/externalscripts/base_take_twilight_flats.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,19 +291,8 @@ async def configure(self, config: types.SimpleNamespace):
self.configure_catalog()

if hasattr(config, "ignore"):
for comp in config.ignore:
if comp in self.camera.components_attr:
self.log.debug(f"Ignoring Camera component {comp}.")
setattr(self.camera.check, comp, False)
elif comp in ["mtdome", "mtdometrajectory"]:
self.log.debug(f"Ignoring dome component {comp}.")
setattr(self.tcs.check, comp, False)
else:
self.log.warning(
f"Component {comp} not in CSC Group. "
f"Must be one of {self.camera.components_attr}. "
f"Ignoring."
)
self.log.debug("Ignoring Camera components.")
self.camera.disable_checks_for_components(components=config.ignore)

self.config = config

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import warnings

import yaml
from lsst.ts.observatory.control import RemoteGroup
from lsst.ts.observatory.control.maintel.comcam import ComCam, ComCamUsages
from lsst.ts.observatory.control.maintel.mtcs import MTCS, MTCSUsages
from lsst.ts.observatory.control.utils import RotType
Expand Down Expand Up @@ -246,3 +247,17 @@ async def configure(self, config):
"""Take the sequence of twilight flats twilight flats."""
self.configure_client()
await super().configure(config)

if hasattr(config, "ignore"):
# Ignoring only specific MTCS components
allowed_ignore = ["mtdome", "mtdometrajectory"]
for comp in map(RemoteGroup._remote_name_to_attr_format, config.ignore):
Copy link
Member

Choose a reason for hiding this comment

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

This is using a "private" method from RemoteGroup. Not sure I understand what is going on here. Can you please clarify?

if comp in allowed_ignore:
self.log.debug(f"Ignoring dome component {comp}.")
setattr(self.mtcs.check, comp, False)
else:
self.log.warning(
f"Component {comp} not in MTCS or not allowed to ignore. "
f"Must be one of {allowed_ignore}. "
f"Ignoring."
)
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import functools

import yaml
from lsst.ts.observatory.control import RemoteGroup
from lsst.ts.observatory.control.maintel.lsstcam import LSSTCam, LSSTCamUsages
from lsst.ts.observatory.control.maintel.mtcs import MTCS
from lsst.ts.observatory.control.utils import RotType
Expand Down Expand Up @@ -223,3 +224,21 @@ async def slew_azel_and_setup_instrument(self, az, el):
az=az,
el=el,
)

async def configure(self, config):
"""Take the sequence of twilight flats twilight flats."""
await super().configure(config)

if hasattr(config, "ignore"):
# Ignoring only specific MTCS components
allowed_ignore = ["mtdome", "mtdometrajectory"]
for comp in map(RemoteGroup._remote_name_to_attr_format, config.ignore):
Copy link
Member

Choose a reason for hiding this comment

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

Again, this is using a "private" method from RemoteGroup. Not sure I understand what is going on here. Can you please, clarify?

if comp in allowed_ignore:
self.log.debug(f"Ignoring dome component {comp}.")
setattr(self.mtcs.check, comp, False)
else:
self.log.warning(
f"Component {comp} not in MTCS or not allowed to ignore. "
f"Must be one of {allowed_ignore}. "
f"Ignoring."
)
11 changes: 1 addition & 10 deletions python/lsst/ts/externalscripts/maintel/tma/random_walk.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ def get_schema(cls):
type: array
items:
type: string
default: []
rot_value:
description: Rotator position value. Actual meaning depends on rot_type.
type: number
Expand Down Expand Up @@ -209,15 +208,7 @@ async def configure(self, config):
self.config.rot_type = getattr(RotType, self.config.rot_type)

if hasattr(self.config, "ignore"):
for comp in self.config.ignore:
if comp not in self.tcs.components_attr:
self.log.warning(
f"Component {comp} not in CSC Group. "
f"Must be one of {self.tcs.components_attr}. Ignoring."
)
else:
self.log.debug(f"Ignoring component {comp}.")
setattr(self.tcs.check, comp, False)
self.tcs.disable_checks_for_components(components=config.ignore)

def set_metadata(self, metadata):
"""Set estimated duration of the script."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,15 +141,8 @@ async def configure(self, config):
self.config = config
self.config.rot_type = getattr(RotType, self.config.rot_type)

for comp in self.config.ignore:
if comp not in self.tcs.components_attr:
self.log.warning(
f"Component {comp} not in CSC Group. "
f"Must be one of {self.tcs.components_attr}. Ignoring."
)
else:
self.log.debug(f"Ignoring component {comp}.")
setattr(self.tcs.check, comp, False)
if hasattr(config, "ignore"):
self.tcs.disable_checks_for_components(components=config.ignore)

self.gencam_list = [
GenericCamera(
Expand Down Expand Up @@ -228,7 +221,6 @@ def get_schema(cls):
type: array
items:
type: string
default: []
rot_value:
description: Rotator position value. Actual meaning depends on rot_type.
type: number
Expand Down Expand Up @@ -331,7 +323,6 @@ def get_schema(cls):
- big_offset_radius
- track_for
- stop_when_done
- ignore
- rot_value
- rot_type
- az_wrap_strategy
Expand Down Expand Up @@ -360,7 +351,7 @@ async def move_dome(self, az: float):
New dome Azimuth position in degrees.
"""
# Return right away if ignoring the dome
if "mtdome" in self.config.ignore:
if "mtdome" in getattr(self.config, "ignore", []):
return

# The dome usually goes to a fault when the brakes engage.
Expand Down
11 changes: 1 addition & 10 deletions python/lsst/ts/externalscripts/maintel/tma/serpent_walk.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ def get_schema(cls):
type: array
items:
type: string
default: []
rot_value:
description: Rotator position value. Actual meaning depends on rot_type.
type: number
Expand Down Expand Up @@ -168,15 +167,7 @@ async def configure(self, config):
self.config.rot_type = getattr(RotType, self.config.rot_type)

if hasattr(self.config, "ignore"):
for comp in self.config.ignore:
if comp not in self.tcs.components_attr:
self.log.warning(
f"Component {comp} not in CSC Group. "
f"Must be one of {self.tcs.components_attr}. Ignoring."
)
else:
self.log.debug(f"Ignoring component {comp}.")
setattr(self.tcs.check, comp, False)
self.tcs.disable_checks_for_components(components=config.ignore)

def set_metadata(self, metadata):
"""Set estimated duration of the script."""
Expand Down
15 changes: 15 additions & 0 deletions tests/auxtel/test_latiss_take_twilight_flats.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def mock_camera(self):
self.script.latiss.assert_all_enabled = mock.AsyncMock()
self.script.latiss.take_focus = mock.AsyncMock(return_value=[1234])
self.script.latiss.take_acq = mock.AsyncMock(return_value=([32, 0]))
self.script.latiss.disable_checks_for_components = mock.Mock()

def mock_consdb(self):
"""Mock consdb and its methods."""
Expand Down Expand Up @@ -90,6 +91,20 @@ async def test_invalid_configuration(self):
with pytest.raises(salobj.ExpectedError):
await self.configure_script(**bad_config)

async def test_configure_ignore(self):
components = ["atspectrograph", "not_comp"]
config = {
"filter": "SDSSr_65mm",
"ignore": components,
}

async with self.make_script():
await self.configure_script(**config)

self.script.camera.disable_checks_for_components.assert_called_once_with(
components=components
)

@unittest.skipIf(
BEST_ISR_AVAILABLE is False,
f"Best_Effort_ISR is {BEST_ISR_AVAILABLE}."
Expand Down
14 changes: 7 additions & 7 deletions tests/maintel/test_maintel_parameter_march_comcam.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,13 @@ async def test_configure_ignore(self):
assert self.script.rotation_sequence == [10, 20, 30, 40, 50]
assert self.script.config.program == "BLOCK-TXXX"

# Verify that the ignored components are correctly set to False
assert not self.script.mtcs.check.mtm1m3
assert not self.script.mtcs.check.mtrotator
# Verify ignoring components
self.script.mtcs.disable_checks_for_components.assert_called_once_with(
components=config["ignore"]
)
self.script.camera.disable_checks_for_components.assert_called_once_with(
components=config["ignore"]
)

async def test_configure_dof_index(self):
config = {
Expand Down Expand Up @@ -202,10 +206,6 @@ async def test_configure_dof_index(self):
assert self.script.rotation_sequence == [10, 20, 30, 40, 50]
assert self.script.config.program == "BLOCK-TXXX"

# Verify that the ignored components are correctly set to False
assert not self.script.mtcs.check.mtm1m3
assert not self.script.mtcs.check.mtrotator

async def test_invalid_configuration(self):
bad_configs = [
{
Expand Down
14 changes: 7 additions & 7 deletions tests/maintel/test_maintel_parameter_march_lsstcam.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,13 @@ async def test_configure_ignore(self):
assert self.script.rotation_sequence == [10, 20, 30, 40, 50]
assert self.script.config.program == "BLOCK-TXXX"

# Verify that the ignored components are correctly set to False
assert not self.script.mtcs.check.mtm1m3
assert not self.script.mtcs.check.mtrotator
# Verify ignoring components
self.script.mtcs.disable_checks_for_components.assert_called_once_with(
components=config["ignore"]
)
self.script.camera.disable_checks_for_components.assert_called_once_with(
components=config["ignore"]
)

async def test_configure_dof_index(self):
config = {
Expand Down Expand Up @@ -203,10 +207,6 @@ async def test_configure_dof_index(self):
assert self.script.rotation_sequence == [10, 20, 30, 40, 50]
assert self.script.config.program == "BLOCK-TXXX"

# Verify that the ignored components are correctly set to False
assert not self.script.mtcs.check.mtm1m3
assert not self.script.mtcs.check.mtrotator

async def test_invalid_configuration(self):
bad_configs = [
{
Expand Down
13 changes: 13 additions & 0 deletions tests/maintel/test_take_ptc_flats_comcam.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,19 @@ async def test_invalid_electrometer_mode_config(self):
with pytest.raises(salobj.ExpectedError):
await self.configure_script(**bad_config)

async def test_configure_ignore(self):
config = {
"flats_exp_times": [7.25, 5.25, 0.75, 12.75],
"ignore": ["ccoods", "no_comp"],
}

async with self.make_script():
await self.configure_script(**config)

self.script.comcam.disable_checks_for_components.assert_called_once_with(
components=config["ignore"]
)

async def test_take_ptc_flats(self):
config = {
"filter": "r_03",
Expand Down
23 changes: 23 additions & 0 deletions tests/maintel/test_take_twilight_flats_comcam.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def mock_camera(self):
self.script.comcam.assert_all_enabled = mock.AsyncMock()
self.script.comcam.take_imgtype = mock.AsyncMock(return_value=[1234])
self.script.comcam.take_acq = mock.AsyncMock(return_value=([32, 0]))
self.script.comcam.disable_checks_for_components = mock.Mock()

def mock_consdb(self):
"""Mock consdb and its methods."""
Expand Down Expand Up @@ -82,6 +83,28 @@ async def test_invalid_configuration(self):
with pytest.raises(salobj.ExpectedError):
await self.configure_script(**bad_config)

async def test_configure_ignore(self):
components = [
"ccoods", # ComCam component
"mtdome", # MTCS component
"mtmount", # Not allowed to ignore MTCS component
"not_comp", # Neither MTCS nor ComCam component
]
config = {
"filter": "r_03",
"ignore": components,
}

async with self.make_script():
await self.configure_script(**config)

self.script.camera.disable_checks_for_components.assert_called_once_with(
components=components
)
assert not self.script.mtcs.check.mtdome
self.script.mtcs.check.mtmount.assert_not_called()
self.script.mtcs.check.not_comp.assert_not_called()

"""
#TODO: properly test this script
async def test_take_twilight_flats(self):
Expand Down
Loading
Loading