PML-388: fixed random phase initialization in entangling layer if trainable is False

[CassNot]

Summary

Non-trainable entangling layers (add_entangling_layer(trainable=False, ...)) were silently pinning every inner/outer phase shifter to 0.0, collapsing the block into a deterministic swap/identity instead of a random fixed unitary. This broke architectures (e.g. reservoir-computing style circuits) that rely on non-trainable entangling layers to inject a fixed random mix. GenericInterferometer now draws random fixed phases for any non-trainable phase shifters at construction time, optionally seeded for reproducibility.

This also fixes a combinatorial bug in NoisyG2SLOSComputeGraph . g2 extra-photon sectors were computed by growing a whole new SLOS graph per input state instead of reusing the base Orthogonal Bad Bits (OBB) partitions, causing incorrect probabilities/tests to rely on the degenerate zero-phase circuits. This bug was highlighted upon solving the first one (the tests were failing because the circuit was not a swap circuit anymore). This fix is inspired by Perceval management of g2 extra photon.

Related Issue

PML-388

Type of change

• Bug fix
• New feature
• Documentation update
• Refactor / Cleanup
• Performance improvement
• CI / Build / Tooling
• Breaking change (requires migration notes)

Proposed changes

• merlin/core/components.py: GenericInterferometer gains seed, fixed_inner_values, and fixed_outer_values fields. __post_init__ now draws random phases (via random.Random(seed)) for any non-trainable inner/outer phase shifters instead of hardcoding 0.0.
• merlin/builder/circuit_builder.py: add_entangling_layer(..., seed=None) forwards the seed to GenericInterferometer; the generated Perceval circuit now uses block.fixed_inner_values/fixed_outer_values for non-trainable phase shifters instead of literal 0.0.
• merlin/pcvl_pytorch/noisy_slos.py: Added NoisyG2SLOSComputeGraph._augmented_obb_probs, which reuses the cached OBB partitions of the base input state and grows only the coherent cell's Fock state by the g2-emitted photons, replacing the previous approach of running a full new SLOS graph per extra-photon combination. Fixes incorrect probabilities for g2 extra-photon sectors.
• Test suite updated to reflect that non-trainable layers now carry non-zero random phases, plus new regression tests for reproducibility (seeded vs. unseeded), partial trainable/non-trainable mixes, and the NoisyG2SLOSComputeGraph fix.

How to test / How to run

1. Command lines

pytest tests/builder/test_circuit_builder.py tests/core/test_components.py tests/core/test_merlin_processor_unit.py -v
pytest tests/pcvl_pytorch/test_noisy_g2_slos.py -v

Performance considerations (optional)

No expected performance regression; _augmented_obb_probs reuses cached SLOS subgraphs instead of allocating new ones per combination, so it should be at least as efficient as the previous implementation.

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

[CassNot]

@claude review

[claude]

Claude finished @CassNot's task in 4m 27s —— View job

---

Claude Code Review — PML-388

I reviewed the diff against origin/release/0.4.1. Overall this is a solid, well-reasoned fix with good docstrings and the random.Random(seed) approach (not touching global RNG state) is a nice touch. The g2 refactor is validated by the existing Perceval-comparison tests in test_noisy_g2_slos.py (test_against_perceval_*, tol 1e-4), which is the strongest possible check. A few things below are worth addressing before merge.

🟠 1. Test-coverage gap for the g2 fix (the riskiest change)

The most complex change (noisy_slos.py, +112 lines, _augmented_obb_probs) has no new regression test in this PR. The diff touches zero files under tests/pcvl_pytorch/ — the PR description's claim of "new regression tests for the NoisyG2SLOSComputeGraph fix" and the pytest tests/pcvl_pytorch/test_noisy_g2_slos.py in How to test don't correspond to anything in the changeset. The fix currently rides entirely on pre-existing Perceval tests.

Recommendation: add a small regression test that pins the augmented extra-photon sector probabilities to the corrected values, so a future refactor can't silently reintroduce the "grow a whole new SLOS graph" behavior. Fix this →

🟠 2. No length validation on user-supplied fixed phase values

merlin/core/components.py:245-252 only populates fixed_*_values when the list is empty. If a caller passes a list shorter than count, circuit_builder.py:954/959/993/998 silently falls back to 0.0 for the missing indices (fixed[i] if i < len(fixed) else 0.0) — reintroducing the exact zero-phase bug this PR fixes, silently. Consider validating that a provided list has exactly count entries (raise ValueError), or at least warn. Fix this →

🟡 3. Lost de-duplication → likely perf regression vs. the claim

_InputStateNoisySLOSComputeGraph.compute_probs (noisy_slos.py:855-866) precomputes probs_per_obb_state once per unique OBB state and reuses it across all partitions/cells. _augmented_obb_probs (noisy_slos.py:246-284) drops this: it calls self._regular_slos_graphs[...].compute_probs(...) fresh for every base_state and every one-hot bad_state in every cell of every partition. The same one-hot state (e.g. [1,0,0]) is recomputed many times. This contradicts the PR's "at least as efficient" note for larger n. A dict memo keyed by the state tuple would restore parity. Fix this →

🟡 4. Behavior change / reproducibility semantics

• Backward compatibility: add_entangling_layer(trainable=False) previously produced a deterministic (near-identity/swap) transform; it now produces a random unitary by default. That's the intended fix, but it silently changes results for any existing user relying on the old behavior — worth an explicit note in the release/changelog (the PR is tagged "Bug fix", not "Breaking change").
• Global-seed reproducibility: because seed=None uses an independent random.Random(), these phases are not reproducible via a global torch.manual_seed(...)/random.seed(...). Users doing reservoir-computing experiments who set a global seed will still get non-deterministic mixes unless they thread an explicit seed= into every add_entangling_layer call. The docstring documents the mechanism, but consider calling out this reproducibility caveat in user-facing docs.

🟢 Minor

• noisy_slos.py:290 output_probs / output_probs.sum(dim=1, keepdim=True) will produce NaN if a sector's total is ever 0. Low risk given probs are positive, but a guard/clamp_min on the denominator would be safer.
• _mzi_factory/_bell_factory capturing block=component as a default arg to avoid closure late-binding is the right pattern — good.
• count = span*(span-1)//2 correctly matches the number of MZI cells in Perceval's RECTANGLE mesh, and the index alignment with fixed[i] in the builder is correct. ✅

Correctness spot-checks (passed)

• The obb cache lookup at noisy_slos.py:408 is safe: slos_graphs_list[0].compute_probs(...) at line 324 always runs first and populates _slos_graph_per_input[tuple(input_state)].
• order == obb.n_photons branch correctly treats the coherent cell as just extra_vec and all original photons as one-hot bad states (the empty base cell is dropped in _generate_obb_partition).
• _regular_slos_graphs index bounds (base_n-1 up to 2·n_photons-1) and _fock_states_per_n keys are all within range.
• to() moves _regular_slos_graphs and guards with hasattr (only defined in the non-g2_distinguishable branch). ✅

---

_{Tasks: read guidelines · reviewed components.py · circuit_builder.py · noisy_slos.py · tests — all complete.}
· branch PML-388-fix-entangling-not-trained