PML-284 noisy slos execution plumbing in quantum layer#205
PML-284 noisy slos execution plumbing in quantum layer#205LF-Vigneux wants to merge 15 commits intomerlinquantum:release/0.4from
Conversation
ben9871
left a comment
There was a problem hiding this comment.
The new tests/algorithms/test_noise_contract.py file is useful, but it mostly tests all noise fields together or source/circuit NotImplemented cases. These are cases which do not behave as expected in the current code
Missing tests worth adding:
- direct
noise_model=NoiseModel(brightness=0.3)applies photon survival probabilities, not[1.0, ...] - direct
noise_model=NoiseModel(transmittance=0.4)applies photon survival probabilities classify_noise_model(pcvl.NoiseModel())returns no active groupsclassify_noise_model(pcvl.NoiseModel(brightness=0.3))contains only post-measurement noiseQuantumLayer(..., noise_model=NoiseModel(brightness=0.3)).computation_process.noise_groupsreceives the groups after setup- NotImplemented message lists the group summary
- experiment and direct noise entry points use the same error type/message for unsupported measurement strategies
return_object=Truewith noise model fails
There was a problem hiding this comment.
changes to notebooks are not part of this PR, this is just ruff?
| @@ -671,12 +697,51 @@ def setup_noise_and_detectors( | |||
| "Compute amplitudes without detectors and apply a Partial DetectorTransform manually if needed." | |||
| ) | |||
|
|
|||
There was a problem hiding this comment.
PML-284 says brightness and transmittance are post-measurement noise and should remain in the existing photon-loss path via resolve_photon_loss(). That works when noise comes from experiment.noise, but not when it comes from the new direct noise_model= API.
The issue is that resolve_circuit() creates a new Experiment(circuit) for builder/circuit sources but does not attach the resolved noise_model to that experiment. Later, setup_noise_and_detectors() calls:
photon_survival_probs, empty_noise_model = resolve_photon_loss(
experiment, circuit.m
)
layer = ml.QuantumLayer(
n_photons=2,
input_size=4,
builder=circ,
noise_model=pcvl.NoiseModel(brightness=0.3),
)
print(layer._photon_survival_probs)
print(getattr(layer.experiment, "noise", None))Observed:
[1.0, 1.0, 1.0, 1.0]
None
Expected:
[0.3, 0.3, 0.3, 0.3]
NoiseModel(brightness=0.3) available to the photon-loss resolver, or equivalent direct plumbing.
There was a problem hiding this comment.
I changed the resolve_photon_loss inputs to be NoiseGroups and changed the order of computation in setup_noise_and_detectors
| ) | ||
| process_input_state = self.input_state | ||
|
|
||
| self.computation_process = ComputationProcessFactory.create( |
There was a problem hiding this comment.
The ticket explicitly asks for noise_groups to propagate through initialization so ComputationProcessFactory.create() can route to the correct process subclass later.
The PR adds noise_groups to InitializationContext and to ComputationProcessFactory.create(), but the actual QuantumLayer call site never passes it. So it never reads these
There was a problem hiding this comment.
Should be done now.
| # Source noise | ||
| if noise_model is None: | ||
| return None | ||
| source = { |
There was a problem hiding this comment.
The ticket says default/None values should be excluded because they represent "no noise on this axis."
Perceval's NoiseModel does not expose all defaults as None: for example an empty model reports values like brightness=1, indistinguishability=1, g2=0, g2_distinguishable=True, transmittance=1, phase_imprecision=0, and phase_error=0.
classify_noise_model(pcvl.NoiseModel())Observed:
NoiseGroups(
source={"indistinguishability": 1, "g2_distinguishable": True, "g2": 0},
circuit={"phase_imprecision": 0, "phase_error": 0},
post_measurement={"transmittance": 1, "brightness": 1},
)
This is not the intended classification. It also means NoiseModel(brightness=0.3) incorrectly has source and circuit groups, even though only post-measurement noise was requested.
| ) | ||
| noise_groups = None if noise_model is None else classify_noise_model(noise_model) | ||
| # Not implemented errors | ||
| if noise_groups is not None: |
There was a problem hiding this comment.
PML-284 asks for a clear construction-time NotImplementedError, after classification and validation, that identifies the noise groups found and states that the differentiable noisy execution path is not implemented.
Current code raises field-specific messages:
merlin/algorithms/layer_utils.py:708
if noise_groups is not None:
if noise_groups.source is not None:
...
raise NotImplementedError("The g2 error is not implemented yet")This does not list the classified groups and it only reports the first unsupported field encountered. It also depends on the current misclassification of identity defaults.
There was a problem hiding this comment.
In your test, you check for post measurement errors but they are already implemented...
| @@ -301,6 +315,7 @@ | |||
| measurement_strategy=measurement_strategy, | |||
| warnings=noise_and_detectors.detector_warnings, | |||
| return_object=return_object, | |||
There was a problem hiding this comment.
The ticket marks return_object=True with a noise model as out of scope. The current validation does not reject it. For a direct post-measurement-only noise model such as NoiseModel(brightness=0.3), construction succeeds.
Summary
Plumbing in the layer constructor to propagated the noise to the corresponding variables to be used in the noisy simulation.
The previous API still works but with a new noise model optional parameter in the QuantumLayer
Related Issue
PML-284
Type of change
Proposed changes
How to test / How to run
Documentation