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

Only use config server if GDA didn't supply params #496

Merged
merged 13 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def flyscan_xray_centre(
parameters.features.update_self_from_server()
composite.eiger.set_detector_parameters(parameters.detector_params)
composite.zocalo.zocalo_environment = parameters.zocalo_environment
composite.zocalo.use_cpu_and_gpu = parameters.compare_cpu_and_gpu_results
composite.zocalo.use_cpu_and_gpu = parameters.features.compare_cpu_and_gpu_zocalo

feature_controlled = _get_feature_controlled(composite, parameters)

Expand Down
32 changes: 22 additions & 10 deletions src/mx_bluesky/hyperion/external_interaction/config_server.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from daq_config_server.client import ConfigServer
from pydantic import BaseModel
from pydantic import BaseModel, Field, model_validator

from mx_bluesky.hyperion.log import LOGGER
from mx_bluesky.hyperion.parameters.constants import CONST
Expand All @@ -17,19 +17,31 @@
class FeatureFlags(BaseModel):
# The default value will be used as the fallback when doing a best-effort fetch
# from the service
use_panda_for_gridscan: bool = False
use_gpu_for_gridscan: bool = False
set_stub_offsets: bool = False
use_panda_for_gridscan: bool = CONST.I03.USE_PANDA_FOR_GRIDSCAN
compare_cpu_and_gpu_zocalo: bool = CONST.I03.COMPARE_CPU_AND_GPU_ZOCALO
set_stub_offsets: bool = CONST.I03.SET_STUB_OFFSETS

# Feature values supplied at construction will override values from the config server
overriden_features: dict = Field(default_factory=dict, exclude=True)

@model_validator(mode="before")
@classmethod
def _get_flags(cls):
flags = config_server().best_effort_get_all_feature_flags()
return {f: flags[f] for f in flags if f in cls.__fields__.keys()}
def mark_overridden_features(cls, values):
assert isinstance(values, dict)
values["overriden_features"] = values.copy()
return values

@classmethod
def best_effort(cls):
return cls(**cls._get_flags())
def _get_flags(cls):
flags = config_server().best_effort_get_all_feature_flags()
return {f: flags[f] for f in flags if f in cls.model_fields.keys()}

def update_self_from_server(self):
"""Used to update the feature flags from the server during a plan. Where there are flags which were explicitly set from externally supplied parameters, these values will be used instead."""
for flag, value in self._get_flags().items():
setattr(self, flag, value)
updated_value = (

Check warning on line 42 in src/mx_bluesky/hyperion/external_interaction/config_server.py

View check run for this annotation

Codecov / codecov/patch

src/mx_bluesky/hyperion/external_interaction/config_server.py#L42

Added line #L42 was not covered by tests
value
if flag not in self.overriden_features.keys()
else self.overriden_features[flag]
)
setattr(self, flag, updated_value)

Check warning on line 47 in src/mx_bluesky/hyperion/external_interaction/config_server.py

View check run for this annotation

Codecov / codecov/patch

src/mx_bluesky/hyperion/external_interaction/config_server.py#L47

Added line #L47 was not covered by tests
1 change: 1 addition & 0 deletions src/mx_bluesky/hyperion/parameters/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ class I03Constants:
SHUTTER_TIME_S = 0.06
USE_PANDA_FOR_GRIDSCAN = False
THAWING_TIME = 20
SET_STUB_OFFSETS = False

# Turns on GPU processing for zocalo and logs a comparison between GPU and CPU-
# processed results. GPU results never used in analysis for now
Expand Down
6 changes: 1 addition & 5 deletions src/mx_bluesky/hyperion/parameters/gridscan.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,6 @@ class GridCommon(
panda_runup_distance_mm: float = Field(
default=CONST.HARDWARE.PANDA_FGS_RUN_UP_DEFAULT
)
use_panda: bool = Field(default=CONST.I03.USE_PANDA_FOR_GRIDSCAN)
compare_cpu_and_gpu_results: bool = Field(
default=CONST.I03.COMPARE_CPU_AND_GPU_ZOCALO
)
ispyb_experiment_type: IspybExperimentType = Field(
default=IspybExperimentType.GRIDSCAN_3D
)
Expand Down Expand Up @@ -73,7 +69,7 @@ def detector_params(self):
use_roi_mode=self.use_roi_mode,
det_dist_to_beam_converter_path=self.det_dist_to_beam_converter_path,
trigger_mode=self.trigger_mode,
enable_dev_shm=self.compare_cpu_and_gpu_results,
enable_dev_shm=self.features.compare_cpu_and_gpu_zocalo,
**optional_args,
)

Expand Down
6 changes: 2 additions & 4 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ def test_fgs_params():

@pytest.fixture
def test_panda_fgs_params(test_fgs_params: ThreeDGridScan):
test_fgs_params.use_panda = True
test_fgs_params.features.use_panda_for_gridscan = True
return test_fgs_params


Expand Down Expand Up @@ -940,9 +940,7 @@ def assert_events_and_data_in_order(

@pytest.fixture
def feature_flags():
return FeatureFlags(
**{field_name: False for field_name in FeatureFlags.model_fields.keys()}
)
return FeatureFlags()


def assert_none_matching(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
"transmission_frac": 1.0,
"omega_start_deg": 0,
"chi_start_deg": 30,
"use_panda": false
},
"multi_rotation_scan": {
"comment": "Hyperion Rotation Scan - ",
Expand Down
21 changes: 20 additions & 1 deletion tests/unit_tests/hyperion/parameters/test_parameter_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,32 @@ def test_selected_aperture_uses_default():
assert params.selected_aperture == ApertureValue.LARGE


def test_feature_flags_overriden_if_supplied(minimal_3d_gridscan_params):
test_params = ThreeDGridScan(**minimal_3d_gridscan_params)
assert test_params.features.use_panda_for_gridscan is False
assert test_params.features.compare_cpu_and_gpu_zocalo is False
minimal_3d_gridscan_params["features"] = {
"use_panda_for_gridscan": True,
"compare_cpu_and_gpu_zocalo": True,
}
test_params = ThreeDGridScan(**minimal_3d_gridscan_params)
assert test_params.features.compare_cpu_and_gpu_zocalo
assert test_params.features.use_panda_for_gridscan
# Config server shouldn't update values which were explicitly provided
test_params.features.update_self_from_server()
assert test_params.features.compare_cpu_and_gpu_zocalo
assert test_params.features.use_panda_for_gridscan


@patch("mx_bluesky.hyperion.parameters.gridscan.os")
def test_gpu_enabled_if_use_gpu_or_compare_gpu_enabled(_, minimal_3d_gridscan_params):
minimal_3d_gridscan_params["detector_distance_mm"] = 100

grid_scan = ThreeDGridScan(**minimal_3d_gridscan_params)
assert not grid_scan.detector_params.enable_dev_shm

minimal_3d_gridscan_params["compare_cpu_and_gpu_results"] = True
minimal_3d_gridscan_params["features"] = {
"compare_cpu_and_gpu_zocalo": True,
}
grid_scan = ThreeDGridScan(**minimal_3d_gridscan_params)
assert grid_scan.detector_params.enable_dev_shm
Loading