PML-320: shared simple builder for simple circuits through QuantumLayer and FeatureMap#223
PML-320: shared simple builder for simple circuits through QuantumLayer and FeatureMap#223CassNot wants to merge 10 commits into
Conversation
… and FeatureMap, deprecation of FidelityKernel.simple and n_modes input for input_size
There was a problem hiding this comment.
Pull request overview
This PR refactors the “simple” circuit construction path to reduce duplication by introducing a shared _build_simple_circuit helper used by both QuantumLayer.simple and FeatureMap.simple. It also introduces deprecations around FidelityKernel.simple and the n_modes parameter, while updating tests and docs to match.
Changes:
- Add
_build_simple_circuit(input_size, n_modes, angle_encoding_scale)inlayer_utils.pyand routeQuantumLayer.simple/FeatureMap.simplethrough it. - Deprecate
FidelityKernel.simpleand deprecate (but still accept)n_modesforFeatureMap.simpleandFidelityKernel.simple, including warning registry updates. - Update tests, warning filters, and user docs/notebook examples to use
FeatureMap.simple+FidelityKernel(...)directly; also adjust post-select validation to handleNone.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/algorithms/test_layer_utils.py | Adds unit tests for the new shared _build_simple_circuit helper and topology parity checks. |
| tests/algorithms/test_kernels.py | Updates kernel tests for new defaults and deprecation-warning behavior. |
| pytest.ini | Adds warning filters for new deprecations to keep test output stable during migration. |
| merlin/utils/deprecations.py | Extends deprecation registry to cover FeatureMap.simple.n_modes, FidelityKernel.simple, and FidelityKernel.simple.n_modes. |
| merlin/algorithms/layer.py | Replaces inlined “simple” circuit assembly with _build_simple_circuit. |
| merlin/algorithms/layer_utils.py | Adds _build_simple_circuit and fixes post-select None handling in vet_experiment. |
| merlin/algorithms/kernels.py | Switches FeatureMap.simple to _build_simple_circuit, deprecates FidelityKernel.simple, and hardens post-select checks. |
| docs/source/user_guide/kernels.rst | Updates guidance/examples away from FidelityKernel.simple. |
| docs/source/notebooks/Kernels.ipynb | Updates notebook narrative and code examples to use FeatureMap.simple + FidelityKernel. |
Comments suppressed due to low confidence (1)
merlin/algorithms/kernels.py:693
- The
n_modesargument is deprecated but still accepted; this docstring now saysValueErroris raised only wheninput_sizeis outside the supported range. The implementation below can still raiseValueErrorfor invalidn_modesvalues (e.g.,< 2or> 20) when callers pass the deprecated parameter, so the Raises section should mentionn_modesas well (or clarify that errors may occur when using deprecatedn_modes).
Raises
------
ValueError
If ``input_size`` is outside the supported range.
"""
# TODO: In release 0.5.x, remove n_modes handling and always use input_size + 1.
if n_modes is None:
n_modes = input_size + 1
if n_modes < 2:
raise ValueError(f"The number of modes must be at least 2, got {n_modes}")
if input_size > 19 or n_modes > 20:
raise ValueError(
"Input size too large for the simple layer construction. For large inputs (with larger size than 20), please use the CircuitBuilder. Here is a quick tutorial on how to use it: https://merlinquantum.ai/quickstart/first_quantum_layer.html#circuitbuilder-walkthrough"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # FeatureMap.simple deprecations | ||
| "FeatureMap.simple.n_modes": ( | ||
| "The number of modes is fixed to 'input_size + 1'. " | ||
| "Use CircuitBuilder directly if you need a different mode count.", | ||
| False, | ||
| None, | ||
| ), |
| # FidelityKernel.simple parameter-level deprecations | ||
| "FidelityKernel.simple.n_modes": ( | ||
| "The number of modes is fixed to 'input_size + 1'. " | ||
| "Use CircuitBuilder directly if you need a different mode count.", | ||
| False, | ||
| None, | ||
| ), |
| # Suppress the DeprecationWarning from FeatureMap.simple when n_modes is | ||
| # forwarded: FidelityKernel.simple already warned the caller via its own | ||
| # registry entry, so a second warning would be confusing. | ||
| with warnings.catch_warnings(): | ||
| warnings.simplefilter("ignore", DeprecationWarning) | ||
| feature_map = FeatureMap.simple( | ||
| input_size=input_size, | ||
| n_modes=n_modes, | ||
| dtype=dtype, | ||
| device=device, | ||
| angle_encoding_scale=angle_encoding_scale, | ||
| ) |
…t FidelityKernel.simple
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a22c78e649
ℹ️ 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".
| kernel = FidelityKernel( | ||
| feature_map=feature_map, | ||
| shots=0, # exact probabilities (no sampling) | ||
| computation_space=ComputationSpace.FOCK, # allow bunched outcomes if needed | ||
| ) |
There was a problem hiding this comment.
Provide the required input_state in kernel examples
This new FidelityKernel(feature_map=...) example omits input_state, but FidelityKernel.__init__ still requires input_state: list[int] with no default. Users copying this quickstart (and the same pattern in the scikit-learn example below) will hit a TypeError before computing any kernel; add the alternating input state for the FeatureMap.simple mode count or use a builder/helper that generates it.
Useful? React with 👍 / 👎.
…nel with simple featuremap stays simple
…input state when odd mode count, copy of input_state to avoid inconsistence
Summary
Introduces a single shared circuit-building helper
_build_simple_circuitso thatQuantumLayer.simpleandFeatureMap.simpleboth delegate to the same method instead of duplicating it.Also deprecates
FidelityKernel.simpleand then_modesparameter on bothFeatureMap.simpleandFidelityKernel.simpleA follow-up ticket will extend this deprecation to
KernelCircuitBuilder.build_fidelity_kernelto complete the removal of all circuit-building paths inFidelityKernel.EDIT:
input_stateinFidelityKernelcan now be inferred from the number of modes in the circuit or from an_photonsparameter (similar toQuantumLayer). This way, we can have aFidelityKernelwith aFeatureMap.simplethat stays simple, without having to precise the number of photons (it is not the case inQuantumLayer)Related Issue
PML-320
Type of change
Proposed changes
layer_utils.py:_build_simple_circuit(input_size, n_modes, angle_encoding_scale): the single source of truth for the LI_simple → angle_encoding → RI_simple topology.QuantumLayer.simplenow delegates to_build_simple_circuit.kernels.py:FeatureMap.simplenow delegates to_build_simple_circuit.FeatureMap.simple: n_modes parameter deprecated; mode count is now alwaysinput_size + 1+ Marked for removal in 0.5.FidelityKernel.simple: deprecated in favour ofFeatureMap.simple+FidelityKernel; duplicate DeprecationWarning suppressed when forwarding to FeatureMap.simple. Marked for removal in 0.5.input_stateis inferred fromn_photonsif given, otherwise from the nb of modesdeprecations.py: extendedsanitize_parametersto cover the new deprecated parameters.test_layer_utils.py: new test file covering_build_simple_circuit.test_kernels.py,test_kernels_deprecation.py: updated to assert new deprecation warnings and correct behaviour.Kernels.ipynb,binary_classification.ipynb,kernels.rst: updated to use FeatureMap.simple directly and not FidelityKernel.simple (as it will be deprecated)How to test / How to run
Documentation
Checklist
SPHINXOPTS="-W --keep-going -n" make -C docs clean htmlthe docs are built without any warning or errors.