Skip to content

Commit

Permalink
feat(test-data-generation): add ability to generate invalid load stat…
Browse files Browse the repository at this point in the history
…ements (#15153)

# Overview

This PR aims to add the ability to call `protocol_context.load_*`
methods in a way that generates invalid protocols. There is also a whole
lot of refactoring that went into this PR to enable these invalid calls.
I removed as much as I could to go into a different PR.

## Features 

2 main features are exposed: `explict_loads` and
`allow_overlapping_loads`.

## `explicit_loads`

An explicit load is a `protocol_context.load_*` statement specified
outside of the internal load statement generation logic from a Deck
Configuration. These are stored as a dictionary
which takes the slot for the key and the load for the value. Also note,
that explicit loads have access to column 4 on staging areas

```python
explict_loads = {
    "a3": <a_heater_shaker_load_statement>,
    "c4": <a_trash_bin_load_statement>,
}
```

### `allow_overlapping_loads` 

When defining a protocol, it is invalid to say, "Load the Heater-Shaker
in slot C1 and also load a Trash Bin into slot C1`. The
`allow_overlapping_loads` switch, allows a test writer to override this
default behavior and specify the loading of multiple things into the
same slot.

### How to use these new features

There are 2 main ways that you can use `explict_loads` and
`allow_overlapping_loads` together: `explict_loads` defined and
`allow_overlapping_loads` set to False and `explict_loads` defined and
`allow_overlapping_loads` set to True.

When you have `allow_overlapping_loads` set to False, if you already
have a fixture defined in the slot that the explicit load is acting on,
the explicit load **REPLACES** the existing fixture.

When you have `allow_overlapping_loads` set to True, if you already have
a fixture defined in the slot that the explicit load is acting on, the
explicit load is **APPENDED AFTER** the existing fixture.

### Usage

- See
[python_protocol_generation_strategies.py](https://github.com/Opentrons/opentrons/blob/39a6849787081c26669c13951abb6cce82b96293/test-data-generation/src/test_data_generation/python_protocol_generation/strategy/python_protocol_generation_strategies.py
)

# Changelog

## Logic

The implementation of explict_loads exists mainly in, ast_helpers.py and
load_phase.py.

`ast_helpers.py` now has 1-to-1 instantation methods for each of the
utilized `protocol_content.load_*` methods.

`load_phase.py` 
- To remove all isinstance logic, all the helper methods now return
lists of assignment statements.
- Add logic for either overriding or appending

## Refactors and supporting content

- Rename `PossibleSlotContents` to `DeckConfigurationFixtures` - now
that explicit loads define things in slots, but don't use this object,
rename it to make it clearer
- Also change the list-generating properties to classmethods, shouldn't
have to have an object instantiated to get access to those lists
- Combine all the different datashapes and constants into singular
files. I was fighting so many circular imports
- For constants, switch nearly everything over to `typing.Literal`
instead of `enum.Enum`- cleans up calling logic
- Define ProtocolConfiguration dataclass to hold all these different
settings instead of passing 5 arguments to PythonProtocolGenerator
- Note that all the changes in python_protocol_generator don't change
any logic. Just usages of classes
- Refactor test/utils.py into protocol_analysis_validator.py. Parsing
the output of failed tests sucked, and was making writing them a
nightmare. This fixes that.
- All pytest tests now use ProtocolConfiguration objects. No more
DeckConfiguration object test cases.
- Add `pytest-xdist`, these tests are CPU bound so running them in
parallel speeds up overall execution. Something to keep in mind as we
move forward

# Test Plan

- [x] Manually validate various generated protocols
- [x] Manually validate analysis response from the various protocols
- [x] Run ~600 tests per strategy and determine if any failures are due
to my generation code or are actual failures. **Resolved all the issues,
they were all me**
- [x] Run new protocols against analysis in app to check for any
weirdness

# Review requests

- Please take a look at [the load statement determination
logic](https://github.com/Opentrons/opentrons/pull/15153/files#diff-2835a2dd65821aa83efd7e004726e6b12ea60eea84e33a86ace6b1a6a74989edR165-R210)
- The logic in
[python_protocol_generation_strategies.py](https://github.com/Opentrons/opentrons/blob/161783ffd765d3e5069c60651de1ece3c330a5d0/test-data-generation/src/test_data_generation/python_protocol_generation/strategy/python_protocol_generation_strategies.py)
is quite dense. Any suggestions?

# Risk assessment

Low
  • Loading branch information
DerekMaggio authored May 14, 2024
1 parent 76360e5 commit 79f25a9
Show file tree
Hide file tree
Showing 19 changed files with 986 additions and 326 deletions.
12 changes: 12 additions & 0 deletions test-data-generation/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,26 @@ wheel:
debug-test:
$(pytest) ./tests \
-vvv \
--tb=long \
-s \
--hypothesis-show-statistics \
--hypothesis-explain \
--hypothesis-profile=dev

.PHONY: exploratory-test
exploratory-test:
$(pytest) ./tests \
--numprocesses=auto \
-s \
--hypothesis-show-statistics \
--hypothesis-explain \
--hypothesis-profile=exploratory

.PHONY: test
test:
$(pytest) ./tests \
--numprocesses=auto \
-s \
--hypothesis-show-statistics \
--hypothesis-explain \
--hypothesis-profile=ci
1 change: 1 addition & 0 deletions test-data-generation/Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ opentrons-shared-data = {file = "../shared-data/python", editable = true}
opentrons = { editable = true, path = "../api"}
test-data-generation = {file = ".", editable = true}
astor = "0.8.1"
pytest-xdist = "*"

[requires]
python_version = "3.10"
19 changes: 18 additions & 1 deletion test-data-generation/Pipfile.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

108 changes: 108 additions & 0 deletions test-data-generation/src/test_data_generation/constants.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
"""Constants and datashapes used in the protocol generation."""

import enum
import typing
import dataclasses

ECHO_ANALYSIS_RESULTS_ENV_VAR_NAME: typing.Final[str] = "ECHO_ANALYSIS_RESULTS"

ColumnName = typing.Literal["1", "2", "3"]
RowName = typing.Literal["a", "b", "c", "d"]
DeckConfigurationSlotName = typing.Literal[
"a1",
"a2",
"a3",
"b1",
"b2",
"b3",
"c1",
"c2",
"c3",
"d1",
"d2",
"d3",
]
AllSlotName = typing.Literal[
"a1",
"a2",
"a3",
"a4",
"b1",
"b2",
"b3",
"b4",
"c1",
"c2",
"c3",
"c4",
"d1",
"d2",
"d3",
"d4",
]

ProtocolContextMethod = typing.Literal[
"load_module",
"load_labware",
"load_instrument",
"load_waste_chute",
"load_trash_bin",
]

PROTOCOL_CONTEXT_VAR_NAME: typing.Final[str] = "protocol_context"


class PipetteNames(str, enum.Enum):
"""Names of the pipettes used in the protocol."""

SINGLE_CHANNEL = "flex_1channel_1000"
MULTI_CHANNEL = "flex_8channel_1000"
NINETY_SIX_CHANNEL = "flex_96channel_1000"


@dataclasses.dataclass
class ModuleInfo:
"""Information about a module."""

load_name: str
name_creation_function: typing.Callable[[str], str]

def variable_name(self, location: str) -> str:
"""Return the variable name for the module."""
return self.name_creation_function(location)


class Modules:
"""Module names used in the protocol."""

MAGNETIC_BLOCK_MODULE = ModuleInfo(
load_name="magneticBlockV1",
name_creation_function=lambda x: f"mag_block_module_{x}",
)
THERMOCYCLER_MODULE = ModuleInfo(
load_name="thermocyclerModuleV2",
name_creation_function=lambda x: "thermocycler_module",
)
TEMPERATURE_MODULE = ModuleInfo(
load_name="temperatureModuleV2",
name_creation_function=lambda x: f"temperature_module_{x}",
)
HEATER_SHAKER_MODULE = ModuleInfo(
load_name="heaterShakerModuleV1",
name_creation_function=lambda x: f"heater_shaker_module_{x}",
)

@classmethod
def modules(cls) -> typing.List[ModuleInfo]:
"""Get all the module info."""
return [
cls.MAGNETIC_BLOCK_MODULE,
cls.THERMOCYCLER_MODULE,
cls.TEMPERATURE_MODULE,
cls.HEATER_SHAKER_MODULE,
]

@classmethod
def get_module_info(cls, module_name: str) -> ModuleInfo:
"""Get the ModuleInfo for the given module name."""
return getattr(cls, module_name.upper()) # type: ignore
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
"""Data shapes for the deck configuration of a Flex."""

import enum
import dataclasses
import enum
import typing

ColumnName = typing.Literal["1", "2", "3"]
RowName = typing.Literal["a", "b", "c", "d"]
SlotName = typing.Literal[
"a1", "a2", "a3", "b1", "b2", "b3", "c1", "c2", "c3", "d1", "d2", "d3"
]
from test_data_generation.constants import (
ColumnName,
DeckConfigurationSlotName,
PipetteNames,
RowName,
)


class PossibleSlotContents(enum.Enum):
class DeckConfigurationFixtures(enum.Enum):
"""Possible contents of a slot on a Flex."""

# Implicitly defined fixtures
Expand All @@ -35,67 +36,67 @@ class PossibleSlotContents(enum.Enum):
@classmethod
def longest_string(cls) -> int:
"""Return the longest string representation of the slot content."""
length = max([len(e.name) for e in PossibleSlotContents])
length = max([len(e.name) for e in DeckConfigurationFixtures])
return length if length % 2 == 0 else length + 1

def __str__(self) -> str:
"""Return a string representation of the slot content."""
return f"{self.name.replace('_', ' ')}"

@classmethod
def all(cls) -> typing.List["PossibleSlotContents"]:
def all(cls) -> typing.List["DeckConfigurationFixtures"]:
"""Return all possible slot contents."""
return list(cls)

@property
def modules(self) -> typing.List["PossibleSlotContents"]:
@classmethod
def modules(cls) -> typing.List["DeckConfigurationFixtures"]:
"""Return the modules."""
return [
PossibleSlotContents.THERMOCYCLER_MODULE,
PossibleSlotContents.MAGNETIC_BLOCK_MODULE,
PossibleSlotContents.TEMPERATURE_MODULE,
PossibleSlotContents.HEATER_SHAKER_MODULE,
cls.THERMOCYCLER_MODULE,
cls.MAGNETIC_BLOCK_MODULE,
cls.TEMPERATURE_MODULE,
cls.HEATER_SHAKER_MODULE,
]

@property
def staging_areas(self) -> typing.List["PossibleSlotContents"]:
@classmethod
def staging_areas(cls) -> typing.List["DeckConfigurationFixtures"]:
"""Return the staging areas."""
return [
PossibleSlotContents.STAGING_AREA,
PossibleSlotContents.STAGING_AREA_WITH_WASTE_CHUTE,
PossibleSlotContents.STAGING_AREA_WITH_WASTE_CHUTE_NO_COVER,
PossibleSlotContents.STAGING_AREA_WITH_MAGNETIC_BLOCK,
cls.STAGING_AREA,
cls.STAGING_AREA_WITH_WASTE_CHUTE,
cls.STAGING_AREA_WITH_WASTE_CHUTE_NO_COVER,
cls.STAGING_AREA_WITH_MAGNETIC_BLOCK,
]

@property
def waste_chutes(self) -> typing.List["PossibleSlotContents"]:
@classmethod
def waste_chutes(cls) -> typing.List["DeckConfigurationFixtures"]:
"""Return the waste chutes."""
return [
PossibleSlotContents.WASTE_CHUTE,
PossibleSlotContents.WASTE_CHUTE_NO_COVER,
PossibleSlotContents.STAGING_AREA_WITH_WASTE_CHUTE,
PossibleSlotContents.STAGING_AREA_WITH_WASTE_CHUTE_NO_COVER,
cls.WASTE_CHUTE,
cls.WASTE_CHUTE_NO_COVER,
cls.STAGING_AREA_WITH_WASTE_CHUTE,
cls.STAGING_AREA_WITH_WASTE_CHUTE_NO_COVER,
]

def is_one_of(self, contents: typing.List["PossibleSlotContents"]) -> bool:
def is_one_of(self, contents: typing.List["DeckConfigurationFixtures"]) -> bool:
"""Return True if the slot contains one of the contents."""
return any([self is content for content in contents])
return any(self is content for content in contents)

def is_a_module(self) -> bool:
"""Return True if the slot contains a module."""
return self.is_one_of(self.modules)
return self.is_one_of(self.modules())

def is_module_or_trash_bin(self) -> bool:
"""Return True if the slot contains a module or trash bin."""
return self.is_one_of(self.modules + [PossibleSlotContents.TRASH_BIN])
return self.is_one_of(self.modules() + [DeckConfigurationFixtures.TRASH_BIN])

def is_a_staging_area(self) -> bool:
"""Return True if the slot contains a staging area."""
return self.is_one_of(self.staging_areas)
return self.is_one_of(self.staging_areas())

def is_a_waste_chute(self) -> bool:
"""Return True if the slot contains a waste chute."""
return self.is_one_of(self.waste_chutes)
return self.is_one_of(self.waste_chutes())


@dataclasses.dataclass
Expand All @@ -104,16 +105,16 @@ class Slot:

row: RowName
col: ColumnName
contents: PossibleSlotContents
contents: "DeckConfigurationFixtures"

def __str__(self) -> str:
"""Return a string representation of the slot."""
return f"{(self.row + self.col).center(self.contents.longest_string())}{self.contents}"

@property
def label(self) -> SlotName:
def label(self) -> DeckConfigurationSlotName:
"""Return the slot label."""
return typing.cast(SlotName, f"{self.row}{self.col}")
return typing.cast(DeckConfigurationSlotName, f"{self.row}{self.col}")

@property
def slot_label_string(self) -> str:
Expand Down Expand Up @@ -182,7 +183,7 @@ def slot_by_row(self, name: RowName) -> Slot:
"""Return the slot by name."""
return getattr(self, f"{name}") # type: ignore

def number_of(self, contents: PossibleSlotContents) -> int:
def number_of(self, contents: DeckConfigurationFixtures) -> int:
"""Return the number of slots with the contents."""
return len([True for slot in self.slots if slot.contents is contents])

Expand Down Expand Up @@ -213,8 +214,8 @@ class DeckConfiguration:
def __str__(self) -> str:
"""Return a string representation of the deck."""
string_list = []
dashed_line = "-" * (PossibleSlotContents.longest_string() * 3)
equal_line = "=" * (PossibleSlotContents.longest_string() * 3)
dashed_line = "-" * (DeckConfigurationFixtures.longest_string() * 3)
equal_line = "=" * (DeckConfigurationFixtures.longest_string() * 3)
for row in self.rows:
string_list.append(
" | ".join([slot.slot_label_string for slot in row.slots])
Expand All @@ -226,6 +227,10 @@ def __str__(self) -> str:

return f"\n{joined_string}\n\n{equal_line}"

def comment_string(self) -> str:
"""Return a string representation of the deck."""
return str(self).replace("\n", "\n# ") + "\n"

def __hash__(self) -> int:
"""Return the hash of the deck."""
return hash(tuple(slot.contents.value for slot in self.slots))
Expand Down Expand Up @@ -278,7 +283,7 @@ def slot_below(self, slot: Slot) -> typing.Optional[Slot]:
return None
return self.rows[row_index + 1].slot_by_col_number(slot.col)

def number_of(self, contents: PossibleSlotContents) -> int:
def number_of(self, contents: DeckConfigurationFixtures) -> int:
"""Return the number of slots with the contents."""
return len([True for slot in self.slots if slot.contents is contents])

Expand All @@ -297,3 +302,11 @@ def column_by_number(self, number: ColumnName) -> Column:
c=self.c.slot_by_col_number(number),
d=self.d.slot_by_col_number(number),
)


@dataclasses.dataclass
class PipetteConfiguration:
"""Configuration for a pipette."""

left: PipetteNames | None
right: PipetteNames | None
Loading

0 comments on commit 79f25a9

Please sign in to comment.