feat: batch id for QuantinuumConfig and HeliosConfig by chesterwringe · Pull Request #59 · quantinuum-dev/quantinuum-schemas

chesterwringe · 2026-04-16T12:26:54Z

No description provided.

…eError

…ntax checker, raise a warning

quantinuum-richard-morrison · 2026-04-16T13:29:07Z

+    HeliosConfig(system_name="Helios-1", attempt_batching=True)
+    QuantinuumConfig(device_name="H2-2", attempt_batching=True)
+
+    # if attempt batching is false, no warning is throw
+    HeliosConfig(system_name="Helios-1E", emulator_config=HeliosEmulatorConfig())
+    HeliosConfig(system_name="Helios-1SC")
+    QuantinuumConfig(device_name="H2-1E")
+    QuantinuumConfig(device_name="H1-Emulator")
+    QuantinuumConfig(device_name="H2-1SC")


I think we should actually assert that this code doesn't emit warnings. I nearly always run pytest --disable-warnings because our code emits... soo many warnings

quantinuum-richard-morrison · 2026-04-16T13:30:10Z

+    @model_validator(mode="after")
+    def check_batch_id_requires_attempt_batching(self) -> Self:
+        """Fails if batch_id is set and attempt_batching is False."""
+        if self.batch_id is not None and not self.attempt_batching:
+            raise ValueError(
+                "batch id can only be set if attempt_batching is set to True."
+            )
+        return self
+
+    @model_validator(mode="after")
+    def warn_if_backend_doesnt_support_batching(self) -> Self:
+        """Warns if attempt_batching is true for a backend that doesn't support batching"""
+
+        def is_hardware_device(config: HeliosConfig) -> bool:
+            """Helper function to identify hardware backends."""
+            if config.system_name.endswith("SC"):
+                return False
+            if config.emulator_config is not None:
+                return False
+            return True
+
+        if self.attempt_batching and not is_hardware_device(self):
+            warnings.warn(
+                "'QuantinuumConfig.attempt_batching' is only supported for hardware backends."
+                "Your job will be submitted as a non-batch job.",
+                RuntimeWarning,
+            )
+        return self
+


Can this repeated code go into a MixIn? DRY

If there's a way to do this for a full-model validator (one that takes the class as argument, which check_batch_id_requires_attempt_batching needs, as it checks two fields against each other), then my googling is failing me.

for warn_if_backend_doesnt_support_batching, as that is a field validation, then I could do something clever to avoid repeated code in theory, the problem being that is_hardware_device checks for different things for HeliosConfig and QuantinuumConfig.

If I've missed something and there is a way to do this, please let me know! Always keen to learn, in this case I just haven't found any way to

Ah, I hadn't spotted that is_hardware_device had different logic. I did a PR targeting this one: #60 - it moves the logic into one place but leaves the decision as to whether the target device supports batching up to the individual configs.

Normally I'd talk this through but we are working async. See what you think and merge if you approve

what does DRY mean here? I honestly thought you meant the code was, well, dry, but looking at your commit messages you use it again, so now I'm not so sure.

Your PR looks good - I think the bit I was missing was the idea of using the Protocol bit, as yes, I was running into static type-checker issues. TIL!

DRY = don't repeat yourself. I often see it used/use it as a verb "DRY up the code". The idea that there should only be one implementation of logic - especially business logic. Now the code expresses the fact that both QuantinuumConfig and HeliosConfig share the idea "if it ain't hardware, batching ain't supported". (but it also expresses that they have differing ideas about how to decide if their target is real hardware). Theoretically easier to maintain etc etc

…st_attempt_batching_against_simulators

quantinuum-richard-morrison

No major changes, just a suggested refactor and - while I think there are no bugs (famous last words) - I don't think the tests are testing quite what is intended

quantinuum-richard-morrison · 2026-04-16T19:14:19Z

+
+        if self.attempt_batching and not is_hardware_device(self):
+            warnings.warn(
+                "'QuantinuumConfig.attempt_batching' is only supported for hardware backends."


We should warn "Batching is not supported for the targeted device/system" instead of "only supported for hardware backends". Today, these statements are equivalent but one day we might have a hardware device that doesn't support batching.

quantinuum-richard-morrison · 2026-04-16T19:14:44Z

+
+        if self.attempt_batching and not is_hardware_device(self):
+            warnings.warn(
+                "'HeliosConfig.attempt_batching' is only supported for hardware backends."


Same comment: "is not supported for this backend" or similar

quantinuum-richard-morrison · 2026-04-16T19:18:25Z

+
+def test_attempt_batching_against_simulators() -> None:
+    """Test warning if attempt_batching is True and the backend doesn't support batching."""
+    with pytest.warns(RuntimeWarning):


First up: thank you for writing unit tests. Looks like you might even have done TDD. NICE ONE

However, I think this test might need a refactor.

with pytest.warns(RuntimeWarning) ensures that the indented code below it does emit a RuntimeWarning. However

it's not specific: some other future or current RuntimeWarning might cause the test to accidentally pass

it's not specific to the lines of code below. If the first HeliosConfig emits the intended warning, but the subsequent lines of code don't, then the test would incorrectly pass even though we want every line to emit a warning.

Maybe break out into parameterized test and use with pytest.warns(RuntimeWarning, match="...")

quantinuum-richard-morrison · 2026-04-16T19:20:28Z

+        QuantinuumConfig(device_name="H1-Emulator", attempt_batching=True)
+        QuantinuumConfig(device_name="H2-1SC", attempt_batching=True)
+
+    with warnings.catch_warnings():


This doesn't throw an exception if a warning is emitted. It just stops them propagating out. Or something.

Anyways, I think for the test to actually check that warnings are not emitted, with warnings.catch_warnings(record=True) as captured_warnings and then assert not captured_warnings is necessary

for what it's worth, I did test this on a piece of code that does throw a warning, and it errored as expected. I intended to add a note about that as a self-review, but then fully forgot about it. I'll still do your suggestion if I can verify it works; It's a lot clearer imo.

I did test this on a piece of code that does throw a warning, and it errored as expected

That is weird. I get different behaviour

❯ python3 Python 3.13.5 (main, Jun 11 2025, 15:36:57) [Clang 16.0.0 (clang-1600.0.26.6)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> import warnings >>> with warnings.catch_warnings(): ... warnings.warn("something bad") ... <python-input-1>:2: UserWarning: something bad >>> # no exception. I wonder if warnings config is significant?

and

>>> with warnings.catch_warnings(record=True) as captured: ... warnings.warn("the end of the world is nigh") ... >>> assert not captured, len(captured) Traceback (most recent call last): File "<python-input-4>", line 1, in <module> assert not captured, len(captured) ^^^^^^^^^^^^ AssertionError: 1

hmm, wondering if I ran it with a catch warnings flag (which I was doing at points for dev purposes) and just didn't notice, in that case. In any case, code we know works > code that supposedly works!

quantinuum-richard-morrison · 2026-04-16T19:23:02Z

+        QuantinuumConfig(device_name="H2-1SC", attempt_batching=True)
+
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")


can remove this line

…s not (#60)

chesterwringe added 4 commits April 16, 2026 12:18

feat: add optional batch-id param to HeliosConfig and QuantinuumConfig

c22a653

chore: if batch_id is set but attempt_batching is False, raise a Valu…

1ef078e

…eError

chore: if attempt_batching is set and the device is an emulator or sy…

ded5631

…ntax checker, raise a warning

chore: validation that batch-id is a UUID

96aae86

chesterwringe force-pushed the feat/batch-id-for-backendconfigs branch from ec64a9a to 3e31166 Compare April 16, 2026 13:15

test: add test cases for validation

77d25f2

chesterwringe force-pushed the feat/batch-id-for-backendconfigs branch from 3e31166 to 77d25f2 Compare April 16, 2026 13:18

chesterwringe commented Apr 16, 2026

View reviewed changes

Comment thread quantinuum_schemas/models/backend_config.py

quantinuum-richard-morrison reviewed Apr 16, 2026

View reviewed changes

Comment thread quantinuum_schemas/models/backend_config.py Outdated

quantinuum-richard-morrison reviewed Apr 16, 2026

View reviewed changes

chesterwringe added 2 commits April 16, 2026 14:33

chore: just give it a UUID type instead of checking manually!

c57d03e

chore: assert that no warnings are called in the positive cases of te…

3dc789e

…st_attempt_batching_against_simulators

chesterwringe force-pushed the feat/batch-id-for-backendconfigs branch from 9490695 to 3dc789e Compare April 16, 2026 13:40

fix: typo

ccf0964

quantinuum-richard-morrison requested changes Apr 16, 2026

View reviewed changes

quantinuum-richard-morrison and others added 2 commits April 17, 2026 09:03

De-duplicating logic where it's shared; keeping it separate where it'…

74bec4e

…s not (#60)

chore: respond to comments (as best I can)

a3873e0

quantinuum-richard-morrison approved these changes Apr 17, 2026

View reviewed changes

chesterwringe merged commit 680e560 into main Apr 17, 2026
7 checks passed

chesterwringe deleted the feat/batch-id-for-backendconfigs branch April 17, 2026 08:42

Conversation

chesterwringe commented Apr 16, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

quantinuum-richard-morrison left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants