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

Set up i24 serial to run on procserv #577

Merged
merged 19 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from 15 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 @@ -269,23 +269,8 @@ def main_extruder_plan(
elif parameters.detector_name == "eiger":
logger.info("Using Eiger detector")

logger.warning(
"""TEMPORARY HACK!
Running a Single image pilatus data collection to create directory."""
) # See https://github.com/DiamondLightSource/mx_bluesky/issues/45
num_shots = 1
sup.pilatus(
"quickshot-internaltrig",
[filepath, parameters.filename, num_shots, parameters.exposure_time_s],
)
logger.debug("Sleep 2s waiting for pilatus to arm")
sleep(2.5)
caput(pv.pilat_acquire, "0") # Disarm pilatus
sleep(0.5)
caput(pv.pilat_acquire, "1") # Arm pilatus
logger.debug("Pilatus data collection DONE")
sup.pilatus("return to normal", None)
logger.info("Pilatus back to normal. Single image pilatus data collection DONE")
logger.debug(f"Creating the directory for the collection in {filepath}.")
Path(filepath).mkdir(parents=True)

caput(pv.eiger_seqID, int(caget(pv.eiger_seqID)) + 1)
logger.info(f"Eiger quickshot setup: filepath {filepath}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3862,7 +3862,7 @@ buttonLabel "Stages"
numPvs 4
numDsps 1
displayFileName {
0 "/dls_sw/work/R3.14.12.3/ioc/ME14E/BL/data/ME14E-motors.edl"
0 "/dls_sw/work/R3.14.12.7/ioc/ME14E/BL/data/ME14E-motors.edl"
}
endObjectProperties

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@
)
from mx_bluesky.beamlines.i24.serial.write_nexus import call_nexgen

ABORTED = False

logger = logging.getLogger("I24ssx.fixed_target")

PMAC_MOVE_TIME = 0.008 # Move time between positions on chip ~ 7-8 ms
Copy link
Contributor

Choose a reason for hiding this comment

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

Will be good to have constants like this moved into more organised locations, but perhaps better to wait for me to finish my work on this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. In this case I was also half considering having this constant in dodal, but I don't have a strong opinion on this. Will make a note in the code to do this



def setup_logging():
# Log should now change name daily
Expand All @@ -85,7 +85,8 @@ def calculate_collection_timeout(parameters: FixedTargetParameters) -> float:
Returns:
The estimated collection time, in s.
"""
buffer = 30
buffer = PMAC_MOVE_TIME * parameters.total_num_images + 2
# buffer = 30
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to leave this comment in? It might be explain why we need a buffer for the timeout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, no, good spot

pump_setting = parameters.pump_repeat
collection_time = parameters.total_num_images * parameters.exposure_time_s
if pump_setting in [
Expand Down Expand Up @@ -474,24 +475,8 @@ def start_i24(
elif parameters.detector_name == "eiger":
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: There is some duplicated code between this file and i24ssx_Extruder_Coolect_py3v2.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that probably won't be fixed until we get around to #62 and we can finish tidying up the code...

logger.info("Using Eiger detector")

logger.warning(
"""TEMPORARY HACK!
Running a Single image pilatus data collection to create directory."""
)
num_imgs = 1
sup.pilatus(
"quickshot-internaltrig",
[filepath, filename, num_imgs, parameters.exposure_time_s],
)
logger.debug("Sleep 2s waiting for pilatus to arm")
sleep(2)
sleep(0.5)
caput(pv.pilat_acquire, "0") # Disarm pilatus
sleep(0.5)
caput(pv.pilat_acquire, "1") # Arm pilatus
logger.debug("Pilatus data collection DONE")
sup.pilatus("return to normal", None)
logger.info("Pilatus back to normal. Single image pilatus data collection DONE")
logger.debug(f"Creating the directory for the collection in {filepath}.")
Path(filepath).mkdir(parents=True)

logger.info(f"Triggered Eiger setup: filepath {filepath}")
logger.info(f"Triggered Eiger setup: filename {filename}")
Expand Down
29 changes: 23 additions & 6 deletions src/mx_bluesky/beamlines/i24/serial/run_extruder.sh
Original file line number Diff line number Diff line change
@@ -1,19 +1,36 @@
#!/bin/bash

NO_PROCESERV_TEST=false

case "$2" in
-t | --test)
echo "Will run serial in test mode without procserv."
NO_PROCESERV_TEST=true
;;
esac


# Get edm path from input
edm_path=$1

# Get the directory of this script
current=$( realpath "$( dirname "$0" )" )
if [[ $NO_PROCESERV_TEST == true ]]; then
echo "Start the blueapi sever"

# Get the directory of this script
current=$( realpath "$( dirname "$0" )" )

# Run script to start blueapi serve
. $current/start_blueapi.sh
# Run script to start blueapi serve
. $current/start_blueapi.sh
fi

# Open the edm screen for an extruder serial collection
echo "Starting extruder edm screen."
edm -x "${edm_path}/EX-gui/DiamondExtruder-I24-py3v1.edl"

echo "Edm screen closed, bye!"

pgrep blueapi | xargs kill
echo "Blueapi process killed"
if [[ $NO_PROCESERV_TEST == true ]]; then
# In this case blueapi server needs to be killed.
pgrep blueapi | xargs kill
echo "Blueapi process killed"
fi
28 changes: 22 additions & 6 deletions src/mx_bluesky/beamlines/i24/serial/run_fixed_target.sh
Original file line number Diff line number Diff line change
@@ -1,22 +1,38 @@
#!/bin/bash

NO_PROCESERV_TEST=false

case "$2" in
-t | --test)
echo "Will run serial in test mode without procserv."
NO_PROCESERV_TEST=true
;;
esac

# Get edm path from input
edm_path=$1

# Export env variable for the stages edm to work properly
export EDMDATAFILES="/dls_sw/prod/R3.14.12.3/support/motor/6-7-1dls14/motorApp/opi/edl"

# Get the directory of this script
current=$( realpath "$( dirname "$0" )" )
if [[ $NO_PROCESERV_TEST == true ]]; then
echo "Start the blueapi sever"

# Get the directory of this script
current=$( realpath "$( dirname "$0" )" )

# Run script to start blueapi serve
. $current/start_blueapi.sh
# Run script to start blueapi serve
. $current/start_blueapi.sh
fi

# Open the edm screen for a fixed target serial collection
echo "Starting fixed target edm screen."
edm -x "${edm_path}/FT-gui/DiamondChipI24-py3v1.edl"

echo "Edm screen closed, bye!"

pgrep blueapi | xargs kill
echo "Blueapi process killed"
if [[ $NO_PROCESERV_TEST == true ]]; then
# In this case blueapi server needs to be killed.
pgrep blueapi | xargs kill
echo "Blueapi process killed"
fi
20 changes: 18 additions & 2 deletions src/mx_bluesky/beamlines/i24/serial/run_serial.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import argparse
import logging
import subprocess
from os import environ
Expand All @@ -6,6 +7,13 @@
logger = logging.getLogger("I24ssx.run")


def _parse_input(expt: str):
Copy link
Contributor

Choose a reason for hiding this comment

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

If this file is the main entry point, it would be nice to be able to run this file directly, eg with a

if __name__ == "__main__":
   ...

Copy link
Contributor Author

@noemifrisina noemifrisina Oct 31, 2024

Choose a reason for hiding this comment

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

Ah no, the entry points are the two functions that are being run directly (see in pyproject.toml), which is why we don't need to use if name etc... it would actually make the code slightly more complicated.

parser = argparse.ArgumentParser(description=f"Run a {expt} collection.")
parser.add_argument("-t", "--test", action="store_true", help="Run in test mode.")
args = parser.parse_args()
return args

Check warning on line 14 in src/mx_bluesky/beamlines/i24/serial/run_serial.py

View check run for this annotation

Codecov / codecov/patch

src/mx_bluesky/beamlines/i24/serial/run_serial.py#L11-L14

Added lines #L11 - L14 were not covered by tests


def get_location(default: str = "dev") -> str:
return environ.get("BEAMLINE") or default

Expand All @@ -19,18 +27,26 @@


def run_extruder():
args = _parse_input("extruder")

Check warning on line 30 in src/mx_bluesky/beamlines/i24/serial/run_serial.py

View check run for this annotation

Codecov / codecov/patch

src/mx_bluesky/beamlines/i24/serial/run_serial.py#L30

Added line #L30 was not covered by tests
loc = get_location()
logger.debug(f"Running on {loc}.")
edm_path = get_edm_path()
filepath = _get_file_path()
test_mode = "--test" if args.test else ""

Check warning on line 35 in src/mx_bluesky/beamlines/i24/serial/run_serial.py

View check run for this annotation

Codecov / codecov/patch

src/mx_bluesky/beamlines/i24/serial/run_serial.py#L35

Added line #L35 was not covered by tests
logger.debug(f"Running {filepath}/run_extruder.sh")
subprocess.run(["sh", filepath / "run_extruder.sh", edm_path.as_posix()])
subprocess.run(

Check warning on line 37 in src/mx_bluesky/beamlines/i24/serial/run_serial.py

View check run for this annotation

Codecov / codecov/patch

src/mx_bluesky/beamlines/i24/serial/run_serial.py#L37

Added line #L37 was not covered by tests
["bash", filepath / "run_extruder.sh", edm_path.as_posix(), test_mode]
)


def run_fixed_target():
args = _parse_input("fixed target")

Check warning on line 43 in src/mx_bluesky/beamlines/i24/serial/run_serial.py

View check run for this annotation

Codecov / codecov/patch

src/mx_bluesky/beamlines/i24/serial/run_serial.py#L43

Added line #L43 was not covered by tests
loc = get_location()
logger.info(f"Running on {loc}.")
edm_path = get_edm_path()
filepath = _get_file_path()
test_mode = "--test" if args.test else ""

Check warning on line 48 in src/mx_bluesky/beamlines/i24/serial/run_serial.py

View check run for this annotation

Codecov / codecov/patch

src/mx_bluesky/beamlines/i24/serial/run_serial.py#L48

Added line #L48 was not covered by tests
logger.debug(f"Running {filepath}/run_fixed_target.sh")
subprocess.run(["sh", filepath / "run_fixed_target.sh", edm_path.as_posix()])
subprocess.run(

Check warning on line 50 in src/mx_bluesky/beamlines/i24/serial/run_serial.py

View check run for this annotation

Codecov / codecov/patch

src/mx_bluesky/beamlines/i24/serial/run_serial.py#L50

Added line #L50 was not covered by tests
["bash", filepath / "run_fixed_target.sh", edm_path.as_posix(), test_mode]
)
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,11 @@ async def test_laser_check(
"mx_bluesky.beamlines.i24.serial.extruder.i24ssx_Extruder_Collect_py3v2.setup_logging"
)
@patch("mx_bluesky.beamlines.i24.serial.extruder.i24ssx_Extruder_Collect_py3v2.bps.rd")
@patch(
"mx_bluesky.beamlines.i24.serial.extruder.i24ssx_Extruder_Collect_py3v2.Path.mkdir"
)
def test_run_extruder_quickshot_with_eiger(
fake_mkdir,
fake_read,
fake_log_setup,
mock_quickshot_plan,
Expand Down Expand Up @@ -180,9 +184,9 @@ def test_run_extruder_quickshot_with_eiger(
assert fake_dcid.generate_dcid.call_count == 1
assert fake_dcid.notify_start.call_count == 1
assert fake_sup.setup_beamline_for_collection_plan.call_count == 1
# Check temporary piilatus hack is in there
assert fake_sup.pilatus.call_count == 2
mock_quickshot_plan.assert_called_once()
assert fake_mkdir.call_count == 1
fake_mkdir.assert_called_once()


@patch("mx_bluesky.beamlines.i24.serial.extruder.i24ssx_Extruder_Collect_py3v2.sleep")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
PumpProbeSetting,
)
from mx_bluesky.beamlines.i24.serial.fixed_target.i24ssx_Chip_Collect_py3v1 import (
PMAC_MOVE_TIME,
calculate_collection_timeout,
datasetsizei24,
finish_i24,
get_chip_prog_values,
Expand All @@ -37,6 +39,27 @@ def fake_generator(value):
return value


def test_calculate_collection_timeout(dummy_params_without_pp):
dummy_params_without_pp.total_num_images = 400
expected_collection_time = (
dummy_params_without_pp.total_num_images
* dummy_params_without_pp.exposure_time_s
)
buffer = dummy_params_without_pp.total_num_images * PMAC_MOVE_TIME + 2
timeout = calculate_collection_timeout(dummy_params_without_pp)

assert timeout == expected_collection_time + buffer


def test_calculate_collection_timeout_for_eava(dummy_params_with_pp):
dummy_params_with_pp.total_num_images = 400
buffer = dummy_params_with_pp.total_num_images * PMAC_MOVE_TIME + 2
expected_pump_and_probe_time = 12.05
timeout = calculate_collection_timeout(dummy_params_with_pp)

assert timeout == expected_pump_and_probe_time + buffer


@patch("mx_bluesky.beamlines.i24.serial.fixed_target.i24ssx_Chip_Collect_py3v1.caput")
def test_datasetsizei24_for_one_block_and_two_exposures(
fake_caput, dummy_params_without_pp
Expand Down Expand Up @@ -134,7 +157,11 @@ def test_load_motion_program_data(
@patch("mx_bluesky.beamlines.i24.serial.fixed_target.i24ssx_Chip_Collect_py3v1.caget")
@patch("mx_bluesky.beamlines.i24.serial.fixed_target.i24ssx_Chip_Collect_py3v1.sup")
@patch("mx_bluesky.beamlines.i24.serial.fixed_target.i24ssx_Chip_Collect_py3v1.sleep")
@patch(
"mx_bluesky.beamlines.i24.serial.fixed_target.i24ssx_Chip_Collect_py3v1.Path.mkdir"
)
def test_start_i24_with_eiger(
fake_mkdir,
fake_sleep,
fake_sup,
fake_caget,
Expand Down Expand Up @@ -165,9 +192,9 @@ def test_start_i24_with_eiger(
assert fake_sup.eiger.call_count == 1
assert fake_sup.setup_beamline_for_collection_plan.call_count == 1
assert fake_sup.move_detector_stage_to_position_plan.call_count == 1
# Pilatus gets called for hack to create directory
assert fake_sup.pilatus.call_count == 2
assert fake_dcid.generate_dcid.call_count == 1
assert fake_mkdir.call_count == 1
fake_mkdir.assert_called_once()

shutter_call_list = [
call("Reset", wait=True, timeout=10.0),
Expand Down
19 changes: 19 additions & 0 deletions utility_scripts/procserv_ioc_start.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#!/bin/bash

# Sources the python environment and starts a blueapi server
# on a python IOC to run serial on I24 with procserv

# Get the directory of this script
current_directory=$( realpath "$( dirname "$0" )" )
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need protection against running this script when the ioc and/or when blueapi is already running?

Copy link
Contributor Author

@noemifrisina noemifrisina Oct 31, 2024

Choose a reason for hiding this comment

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

Not really... I mean, we could add something but blueAPI will not start and just raise and error if already running - on procserv or otherwise - because the port in the stomp configuration is already occupied.

Also, procserv right now can only run on one machine at a time on I24 because the control machine is still on RHEL7 so we've kind of already got two levels of "you shall not run"

# Get the directory with the venv
env_directory=$(dirname $current_directory)
# And where the blueapi config lives
config_directory="${env_directory}/src/mx_bluesky/beamlines/i24/serial"

# Source modules environment
source /dls_sw/etc/profile
# Activate virtual environment
source $env_directory/.venv/bin/activate

# Start blueapi server
blueapi -c "${config_directory}/blueapi_config.yaml" serve
Loading