PML-256 : hardware aware model design by leithkarrai · Pull Request #275 · merlinquantum/merlin

leithkarrai · 2026-06-22T15:00:23Z

Summary

This PR allow user to have a clear documentation on how to design a hardware-aware model. this documentation is added to user guide.

Related Issue

Type of change

Proposed changes

How to test / How to run

Command lines

Block of code

Screenshots / Logs (optional)

Performance considerations (optional)

Documentation

User docs updated (Sphinx)
Examples / notebooks updated
Docstrings updated
Updated the API

Checklist

PR title includes Jira issue key (e.g., PML-126)
"Related Jira ticket" section includes the Jira issue key (no URL)
Code formatted (ruff format)
Lint passes (ruff)
Static typing passes (mypy) if applicable
Unit tests added/updated (pytest)
Tests pass locally (pytest)
Tests pass on GPU (pytest)
Test coverage not decreased significantly
Docs build locally if affected (sphinx)
With this command:
```
  > SPHINXOPTS="-W --keep-going -n" make -C docs clean html 
```
the docs are built without any warning or errors.
New public classes/methods/packages are added in the API following the methodology presented in other files.
Dependencies updated (if needed) and pinned appropriately
PR description explains what changed and how to validate it

…lity

LF-Vigneux

First requested changes

LF-Vigneux · 2026-06-22T18:41:25Z

+When designing models for physical hardware execution, selecting the appropriate computation space is critical.
+
+* **Recommended:** Use ``ComputationSpace.unbunched`` or ``ComputationSpace.DUAL_RAIL``.
+* **Avoid:** ``ComputationSpace.bunched`` and ``ComputationSpace.FOCK``. Physical photonic QPUs do not natively support bunched states or arbitrary Fock spaces due to hardware limitations.


Fock is bunched

You can say that the current hardware does not support the FOCK space since we do not have photon number resolving detectors. That is the main cause, you can say that it is mainly because of this and other limitations

avoid ComputationSpace.FOCK for hardware-targeted models unless explicitly justified, that is the main claim that the ticket showed

LF-Vigneux · 2026-06-22T18:46:25Z

+State Encoding
+--------------
+
+Photonic QPUs do not natively perform amplitude encoding. 


Again please detail that amplitude encoding is difficult because arbitrary dense state preparation is not hardware-realistic. The circuit can be very bug and not trivial to implement

LF-Vigneux · 2026-06-22T18:47:18Z

+  * ``mode_expectations`` (Average photon counts per mode)
+  * ``sampled counts/probabilities`` (Shot-based measurements)
+
+* **Unsupported:** Raw amplitudes or ``StateVector`` outputs cannot be directly retrieved from physical hardware.


Indeed, we only have shots, so the amplitudes can not be retrieved but the rest can be deduced

Just reiterate that we only have sots/samples

LF-Vigneux · 2026-06-22T18:51:47Z

+
+* **Supported:**
+
+  * ``probabilities`` (Full probability distribution)


it is derived from the the counts on the hardware, not from the amplitudes.

LF-Vigneux · 2026-06-22T19:02:22Z

+       input_size=4,                                      # Number of classical features
+       Experiment=ComputationSpace.UNBUNCHED,             # UNBUNCHED or DUAL_RAIL for hardware
+       builder=builder,
+       n_photons=3,                                       # Equivalent to input_state = [1, 1, 1, 0, 0, 0]


This is not True: [1,0,1,0,1,0]

LF-Vigneux · 2026-06-22T19:09:05Z

+* **Component Selection:** Favor native photonic components such as Mach-Zehnder Interferometers (MZI), beam splitters, and phase shifters, as they map directly to the QPU's physical architecture.
+* **Circuit Depth:** Avoid unnecessarily deep circuits. Large simulated circuits may scale poorly during hardware compilation (transpilation), leading to suboptimal or inefficient physical implementations on a constrained QPU.
+
+Recommended Architecture Pattern


Add all the steps from the Recommended model pattern section in the ticket

LF-Vigneux · 2026-06-22T19:09:10Z

From the ticket, you are missing:

Shot-efficiency guidance:

Unsupported or discouraged features for the first hardware-aware guide

Only missing shot efficiency guidance

LF-Vigneux · 2026-06-22T19:14:09Z

This is not as clear as it can get. Identify it clearly.

The page clearly separates:

hardware-oriented designs

simulation-only designs

discouraged-but-possible designs.

LF-Vigneux · 2026-06-22T19:14:50Z

@claude review

claude · 2026-06-22T19:15:11Z

Claude finished @LF-Vigneux's task in 1m 20s —— View job

Code Review: Hardware-Aware Model Design docs

I reviewed the two changed files against the actual MerLin API on origin/release/0.4.1. Below is my feedback, grouped by severity. The reviewer (@LF-Vigneux) already raised most of the content gaps, so I focused on verifying the technical/API correctness of the new page, which is where I found the most serious issues.

🔴 Incorrect API references (will mislead users / won't run)

ComputationSpace.bunched / ComputationSpace.BUNCHED does not exist (hardware_design_checklist.rst:12).
The enum in merlin/core/computation_space.py only defines three members:
```
FOCK = "fock"
UNBUNCHED = "unbunched"
DUAL_RAIL = "dual_rail"
```
There is no BUNCHED space. Bunching is a property of the FOCK space (it allows multiple photons per mode), so listing ComputationSpace.bunched as a separate space to avoid is factually wrong. This matches @LF-Vigneux's "Fock is bunched" note: the single, correct claim is avoid ComputationSpace.FOCK for hardware-targeted models (no photon-number-resolving detectors on current hardware), unless explicitly justified.
Enum member casing is wrong (hardware_design_checklist.rst:11-12).
The doc writes ComputationSpace.unbunched and ComputationSpace.bunched, but the enum members are upper-case (ComputationSpace.UNBUNCHED, ComputationSpace.DUAL_RAIL, ComputationSpace.FOCK). The lower-case strings are only the .values. As written, ComputationSpace.unbunched is not valid Python.
The code example uses a non-existent constructor argument (hardware_design_checklist.rst:87).
```
core = QuantumLayer(
    Experiment=ComputationSpace.UNBUNCHED,   # ❌ no such parameter
    ...
)
```
QuantumLayer.__init__ (merlin/algorithms/layer.py:109) has no Experiment parameter. The computation space is passed via computation_space=, and there is a separate experiment= (lower-case) parameter that expects a pcvl.Experiment, not a ComputationSpace. As written this raises TypeError.
The example is missing all of its imports and uses a broken relative import (hardware_design_checklist.rst:70-98).
- from ..builder import CircuitBuilder is a package-internal relative import that will not work in user code. It should be an absolute import, e.g. from merlin.builder.circuit_builder import CircuitBuilder.
- QuantumLayer, ComputationSpace, MeasurementStrategy, and LexGrouping are used but never imported. Please make the snippet copy-paste runnable (the PR checklist claims docs build clean with -W, but a non-runnable example undercuts the page's purpose).
Please verify add_entangling_layer, add_angle_encoding, add_rotations, add_superpositions signatures against merlin/builder/circuit_builder.py — I confirmed add_angle_encoding, add_entangling_layer, and add_superpositions exist, but the example should be actually executed to confirm it runs end-to-end.

🟠 Content gaps (echoing the existing review thread — still open)

These were flagged inline by @LF-Vigneux and are not yet addressed:

State encoding (:17): explain why amplitude encoding is hard — arbitrary dense state preparation is not hardware-realistic; the prep circuit is large and non-trivial. Add the rule "avoid arbitrary superposition inputs unless there is a real state-preparation path."
Output formats (:29, :33): clarify that hardware only yields shots/counts — amplitudes cannot be retrieved, and probabilities/mode_expectations are derived from the counts, not from amplitudes.
Input-state constraints (:47): post-selection loss happens because invalid states fall outside the computation space → information loss (rather than just "discard invalid states").
Circuit design (:53, :55): also mention permutations as native components, and add "avoid designs that create excessive bunching when using UNBUNCHED."
Recommended pattern (:56): include all the steps from the ticket's "Recommended model pattern" section.
Still missing from the ticket: a shot-efficiency section and an unsupported/discouraged features section.
n_photons comment (:89): the inline comment is inaccurate for the dual-rail case ([1,0,1,0,1,0]) — make the input-state comment consistent with the chosen space.

🟠 Structure / clarity

index.rst: per the review, the Migration guide should be last, with the remaining entries in alphabetical order. Currently "Hardware aware design" is placed after "Migration guide" (index.rst:113-133). Note also it reuses migration.json for its gallery data (index.rst:125) — that looks copy-pasted and likely wrong.
Page organization: the page does not yet clearly separate hardware-oriented, simulation-only, and discouraged-but-possible designs. Consider explicit subsections so each design choice is unambiguously categorized.

🟡 Minor (RST)

Several section-underline rows should be checked for length ≥ title length to keep the -W build warning-free (e.g. the over-/under-line pairs around :7, :23, :36, :44).

Overall: the page is a good start and the structure is sensible, but the code example and the ComputationSpace references contain concrete API errors that would prevent it from running and would teach users incorrect APIs. I'd prioritize fixing items 1–4 above, then close out the still-open inline content comments.

I've only reviewed here — happy to implement any of these fixes if you reply with @claude implement ....
· PML-256-Hardware-aware-model-design

…son, bugfixes on toctree

LF-Vigneux

Updates

LF-Vigneux · 2026-06-25T16:10:37Z

+  * ``mode_expectations`` (Average photon counts per mode)
+  * ``sampled counts/probabilities`` (Shot-based measurements)
+
+* **Unsupported:** Raw amplitudes or ``StateVector`` outputs cannot be directly retrieved from physical hardware.


Just reiterate that we only have sots/samples

LF-Vigneux · 2026-06-25T16:28:16Z

Only missing shot efficiency guidance

leith karrai added 3 commits June 22, 2026 16:15

PML-256(Refactor) added docs to guide user through hardware compatibi…

14b9d48

…lity

added documentation to the toctree

5ea2579

PML(256) refactor -256(Refactor) file reformated with ruff

f37e537

LF-Vigneux requested changes Jun 22, 2026

View reviewed changes

LF-Vigneux self-assigned this Jun 22, 2026

leith karrai added 2 commits June 23, 2026 13:39

PR comments merely solved, still need to clarify shots description

20672f5

refactored rst files and thardware_aware rst file and corresponding j…

d274d6e

…son, bugfixes on toctree

LF-Vigneux requested changes Jun 25, 2026

View reviewed changes


		* Supported:

		* ``probabilities`` (Full probability distribution)

Uh oh!

Conversation

leithkarrai commented Jun 22, 2026

Summary

Related Issue

Type of change

Proposed changes

How to test / How to run

Screenshots / Logs (optional)

Performance considerations (optional)

Documentation

Checklist

Uh oh!

LF-Vigneux 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!

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!

Choose a reason for hiding this comment

Uh oh!

LF-Vigneux commented Jun 22, 2026

Uh oh!

claude Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review: Hardware-Aware Model Design docs

🔴 Incorrect API references (will mislead users / won't run)

🟠 Content gaps (echoing the existing review thread — still open)

🟠 Structure / clarity

🟡 Minor (RST)

Uh oh!

LF-Vigneux 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

claude Bot commented Jun 22, 2026 •

edited

Loading