PML-286 g2 noise#222
Conversation
…specfying everything
…o PML-286-g2-g2_dist
|
@codex review with a focus on scaling and memory usage |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55188a3099
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR adds support for simulating g2 (multi-photon emission) source noise in Merlin’s SLOS-based quantum pipeline by introducing a sectored (per photon-number) probability output and wiring it through measurement + sampling + docs/tests.
Changes:
- Introduces
NoisyG2SLOSComputeGraphand a newSectoredDistribution/SectorResultoutput type to represent probabilities across photon-number sectors. - Updates the measurement pipeline to accept a sampling process object (with a g2-aware sampling method) and blocks grouping when outputs are sectored.
- Adds extensive regression/unit tests and Sphinx API/docs entries for the new g2 functionality.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/pcvl_pytorch/test_noisy_g2_slos.py | New comprehensive test suite for g2 sectored outputs, normalization, gradients, and Perceval comparison. |
| tests/measurement/test_measurement_strategy.py | Updates tests to use the new sampler object interface instead of a sampling function. |
| tests/algorithms/test_noise_contract.py | Updates/extends noise contract tests to reflect g2 now being supported and adds regressions. |
| merlin/pcvl_pytorch/slos_torchscript.py | Routes source-noise graph building to NoisyG2SLOSComputeGraph when g2 is present. |
| merlin/pcvl_pytorch/noisy_slos.py | Implements NoisyG2SLOSComputeGraph and adds plumbing for shared SLOS subgraphs. |
| merlin/pcvl_pytorch/init.py | Exports NoisyG2SLOSComputeGraph from the pcvl_pytorch package. |
| merlin/measurement/strategies.py | Allows SectoredDistribution through distribution strategies; blocks grouping for g2. |
| merlin/measurement/process.py | Adds SamplingProcess.pcvl_sampler_g2 for sampling over sectored outputs. |
| merlin/core/sectored_distribution.py | Adds new core data classes representing sectored probability outputs. |
| merlin/core/process.py | Updates computation process to return SectoredDistribution for g2 noisy simulations and mix superpositions per sector. |
| merlin/core/init.py | Re-exports SectoredDistribution and SectorResult. |
| merlin/algorithms/layer.py | Extends QuantumLayer.forward typing/logic to pass sectored distributions through measurement. |
| merlin/algorithms/layer_utils.py | Removes “g2 not implemented” guard so g2 source noise is allowed. |
| merlin/algorithms/kernels.py | Explicitly rejects g2 noise for kernels (raises NotImplementedError). |
| docs/source/quantum_expert_area/noisy_simulations.rst | Documents new constraint: no grouping when g2>0 due to multiple photon sectors. |
| docs/source/api_reference/api/merlin.pcvl_pytorch.noisy_slos.rst | Adds API docs for NoisyG2SLOSComputeGraph. |
| docs/source/api_reference/api/merlin.core.sectored_distribution.rst | Adds API docs page for sectored distribution classes. |
| docs/source/api_reference/api/merlin.core.rst | Adds the new core module to the API reference index. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ben9871
left a comment
ben9871
left a comment
There was a problem hiding this comment.
Some issues with behaviour of G2 and accessibility of keys to be fixed before merging
| f"A grouping strategy cannot be applied to the output of a simulation with g2 noise. Indeed, since this noise creates input states with more photons than expected, multiple photon sectors are explored. The fock spaces explored are m={distribution.sectors[0].n_modes} modes and n_photons={min(distribution._photon_map.keys())} to 2*n_photons={max(distribution._photon_map.keys())} that all have different space dimensions. To still apply a grouping strategy, you can iterate over the :class:`~merlin.core.sectored_distribution.SectorResult`s of the :class:`~merlin.core.sectored_distribution.SectoredDistribution` and apply one grouping per sector." | ||
| ) | ||
| for sector_result in distribution.sectors: | ||
| sector_result.tensor = apply_photon_loss(sector_result.tensor) |
There was a problem hiding this comment.
This reuses the photon-loss and detector transforms built from the original n_photons Fock basis for all g2 sectors. Higher sectors have different basis-sizes, so post-measurement loss/detectors either crash or would map against the wrong basis.
| ) | ||
|
|
||
| # Creating the n_photons + num_photons_added sector | ||
| sector = SectorResult( |
There was a problem hiding this comment.
SectorResult.keys is never populated, so the output does not actually carry the sector basis metadata promised by the API. This is also why the measurement layer cannot safely build per-sector loss/detector transforms.
There was a problem hiding this comment.
Changed in next commit
| """Returns the total probability across the sectors.""" | ||
| total_prob = 0.0 | ||
| for sector in self.sectors: | ||
| total_prob += sector.tensor.sum().item() |
There was a problem hiding this comment.
total_probability() should return a tensor, not a Python float. The .item() calls detach the result from autograd, which works against the differentiable output contract.
There was a problem hiding this comment.
Changed in next commit
There was a problem hiding this comment.
Changed in next commit
Summary
Added the g2 noise simulations in the MerLin's quantum layer's pipeline. The simulations can now be .
Related Issue
PML-286
Type of change
Proposed changes
How to test / How to run
Performance considerations (optional)
Documentation