From 3bffb41fc0085b41b8c49fa3ecc1a67ae49a34af Mon Sep 17 00:00:00 2001 From: amitlissack Date: Wed, 6 Apr 2022 14:01:28 -0400 Subject: [PATCH] refactor(api): slightly faster simulation (#9821) * cache thermocycler presense to avoid linear search * cache check names * comments. more caching. * thermocycler_present attribute test and fix. * constants. * move lru_cache to ul_per_mm --- api/src/opentrons/hardware_control/pipette.py | 5 +++++ .../context/simulator/instrument_context.py | 2 +- api/src/opentrons/protocols/geometry/deck.py | 15 +++++++++++++ .../opentrons/protocols/geometry/planning.py | 7 +++--- .../protocols/geometry/test_geometry.py | 22 +++++++++++++++++++ 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/api/src/opentrons/hardware_control/pipette.py b/api/src/opentrons/hardware_control/pipette.py index 905c6257628..dabfa5bf678 100644 --- a/api/src/opentrons/hardware_control/pipette.py +++ b/api/src/opentrons/hardware_control/pipette.py @@ -1,5 +1,7 @@ from __future__ import annotations +import functools + """ Classes and functions for pipette state tracking """ from dataclasses import asdict, replace @@ -287,6 +289,9 @@ def remove_tip(self) -> None: def has_tip(self) -> bool: return self._has_tip + # Cache max is chosen somewhat arbitrarily. With a float is input we don't + # want this to unbounded. + @functools.lru_cache(maxsize=100) def ul_per_mm(self, ul: float, action: UlPerMmAction) -> float: sequence = self._config.ul_per_mm[action] return pipette_config.piecewise_volume_conversion(ul, sequence) diff --git a/api/src/opentrons/protocols/context/simulator/instrument_context.py b/api/src/opentrons/protocols/context/simulator/instrument_context.py index 72abbd9470b..a3b6b05bd88 100644 --- a/api/src/opentrons/protocols/context/simulator/instrument_context.py +++ b/api/src/opentrons/protocols/context/simulator/instrument_context.py @@ -4,9 +4,9 @@ from opentrons.hardware_control import NoTipAttachedError, TipAttachedError from opentrons.hardware_control.dev_types import PipetteDict from opentrons.hardware_control.types import HardwareAction +from opentrons.protocols.api_support.labware_like import LabwareLike from opentrons.protocols.api_support.types import APIVersion from opentrons.protocols.api_support.definitions import MAX_SUPPORTED_VERSION -from opentrons.protocols.api_support.labware_like import LabwareLike from opentrons.protocols.api_support.util import FlowRates, PlungerSpeeds, Clearances from opentrons.protocols.geometry import planning from opentrons.protocols.context.instrument import AbstractInstrument diff --git a/api/src/opentrons/protocols/geometry/deck.py b/api/src/opentrons/protocols/geometry/deck.py index 01f20711790..c21224d5dbd 100644 --- a/api/src/opentrons/protocols/geometry/deck.py +++ b/api/src/opentrons/protocols/geometry/deck.py @@ -1,3 +1,4 @@ +import functools import logging from collections import UserDict from dataclasses import dataclass @@ -57,6 +58,7 @@ def __init__(self, load_name: Optional[str] = None) -> None: load_name = deck_type() self._definition = load_deck(load_name, 2) self._load_fixtures() + self._thermocycler_present = False def _load_fixtures(self): for f in self._definition["locations"]["fixtures"]: @@ -67,6 +69,7 @@ def _load_fixtures(self): self.__setitem__(slot_name, loaded_f) @staticmethod + @functools.lru_cache(20) def _assure_int(key: object) -> int: if isinstance(key, str): return int(key) @@ -97,6 +100,10 @@ def __delitem__(self, key: types.DeckLocation) -> None: self.data[checked_key] = None if old: self.recalculate_high_z() + # Update the thermocycler present member + self._thermocycler_present = any( + isinstance(item, ThermocyclerGeometry) for item in self.data.values() + ) def __setitem__(self, key: types.DeckLocation, val: DeckItem) -> None: slot_key_int = self._check_name(key) @@ -125,6 +132,9 @@ def __setitem__(self, key: types.DeckLocation, val: DeckItem) -> None: ) self.data[slot_key_int] = val self._highest_z = max(val.highest_z, self._highest_z) + self._thermocycler_present = any( + isinstance(item, ThermocyclerGeometry) for item in self.data.values() + ) def __contains__(self, key: object) -> bool: try: @@ -295,3 +305,8 @@ def get_item_covered_slot_keys(sk, i): if item_slot_keys.issubset(covered_sks): colliding_items.setdefault(sk, []).append(i) return colliding_items + + @property + def thermocycler_present(self) -> bool: + """Is a thermocycler present on the deck.""" + return self._thermocycler_present diff --git a/api/src/opentrons/protocols/geometry/planning.py b/api/src/opentrons/protocols/geometry/planning.py index a47c628da91..10780fe299b 100644 --- a/api/src/opentrons/protocols/geometry/planning.py +++ b/api/src/opentrons/protocols/geometry/planning.py @@ -18,7 +18,6 @@ from opentrons.protocols.api_support.labware_like import LabwareLike from opentrons.protocols.geometry.deck import Deck from opentrons.protocols.geometry.module_geometry import ( - ThermocyclerGeometry, ModuleGeometry, ) @@ -34,7 +33,7 @@ def max_many(*args): return functools.reduce(max, args[1:], args[0]) -BAD_PAIRS = [ +BAD_PAIRS = { ("1", "12"), ("12", "1"), ("4", "12"), @@ -49,7 +48,7 @@ def max_many(*args): ("11", "4"), ("1", "11"), ("11", "1"), -] +} def should_dodge_thermocycler( @@ -61,7 +60,7 @@ def should_dodge_thermocycler( Returns True if we need to dodge, False otherwise """ - if any([isinstance(item, ThermocyclerGeometry) for item in deck.values()]): + if deck.thermocycler_present: transit = (from_loc.labware.first_parent(), to_loc.labware.first_parent()) # mypy doesn't like this because transit could be none, but it's # checked by value in BAD_PAIRS which has only strings diff --git a/api/tests/opentrons/protocols/geometry/test_geometry.py b/api/tests/opentrons/protocols/geometry/test_geometry.py index 7afb77b6110..2f6d2b4e2cf 100644 --- a/api/tests/opentrons/protocols/geometry/test_geometry.py +++ b/api/tests/opentrons/protocols/geometry/test_geometry.py @@ -471,3 +471,25 @@ def test_get_non_fixture_slots(): deck[4] = trough assert deck.get_non_fixture_slots() == [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11] + + +def test_thermocycler_present() -> None: + """It should change when thermocycler is added/removed""" + deck = Deck() + + # Empty deck. No thermocycler + assert not deck.thermocycler_present + + # Add a thermocycler + deck[7] = module_geometry.load_module( + module_geometry.ThermocyclerModuleModel.THERMOCYCLER_V1, deck.position_for(7) + ) + assert deck.thermocycler_present + + # Add another labware + deck[4] = labware.load(trough_name, deck.position_for(4)) + assert deck.thermocycler_present + + # Remove thermocycler + del deck[7] + assert not deck.thermocycler_present