Skip to content

PML-250 : Image classifications with quantum layers#282

Draft
leithkarrai wants to merge 6 commits into
merlinquantum:release/0.4.1from
leithkarrai:PML-250-Image-Classifications-with-QuantumLayers
Draft

PML-250 : Image classifications with quantum layers#282
leithkarrai wants to merge 6 commits into
merlinquantum:release/0.4.1from
leithkarrai:PML-250-Image-Classifications-with-QuantumLayers

Conversation

@leithkarrai

Copy link
Copy Markdown

Summary

Related Issue

Type of change

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

Proposed changes

How to test / How to run

  1. 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

@leithkarrai
leithkarrai changed the base branch from main to release/0.4.1 July 9, 2026 07:47
@leithkarrai leithkarrai changed the title Pml 250 image classifications with quantum layers PML-250 : Image classifications with quantum layers Jul 9, 2026
@leithkarrai

Copy link
Copy Markdown
Author

@claude

@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown

Claude finished @leithkarrai's task in 1m 58s —— View job


Code review — MNIST_classifier_example.ipynb

  • Read repo guidelines (AGENTS.md)
  • Read the new notebook
  • Provide review feedback

Thanks @leithkarrai. I reviewed the single added file, docs/source/notebooks/MNIST_classifier_example.ipynb. Overall this is an early draft that needs work before it matches the PR title and the repo's contribution standards. Below is a prioritized review.


🔴 Blocking

1. The notebook doesn't do what the title says.
It's called "MNIST classifier example" / "Image classifications with quantum layers", but there is no MNIST, no data loading, no training loop, and no classification anywhere. The only non-empty cell defines a ScaleLayer and a QLayer module and instantiates QLayer(3, 3, 3). For this to be an example notebook it needs, at minimum: dataset loading + preprocessing (e.g. PCA like the other MNIST notebooks), a full model, a training loop, and an accuracy result. See docs/source/notebooks/QCNNClassifier-demo-MNIST.ipynb or ReservoirClassifier.ipynb as reference patterns.

2. Committed error output + leaked local path.
The saved notebook still contains a stale execution with a full traceback:

ValueError: Input size (3) must equal the number of input parameters generated by the circuit (0).

Two problems:

  • The traceback doesn't even match the current cell source — it references builder_second_interf.add_rotations(list(range(nb_modes+1))), which is not in the committed code. So the outputs are from an older version of the cell and are misleading.
  • It leaks a personal absolute path (and a different ticket number): /mnt/c/Users/Leïth.../PML-328/merlin/....

Please do a clean Restart & Run All so all cells execute without errors, and commit the notebook with fresh, consistent outputs (or cleared outputs, per your convention).

3. Not registered in any toctree / gallery.
MNIST_classifier_example.ipynb isn't referenced in any toctree or gallery JSON (I grepped docs/source). AGENTS.md states "Every file under docs/source must appear in at least one toctree", and the CI docs build uses SPHINXOPTS="-W --keep-going -n", which turns the "document isn't included in any toctree" warning into a build failure. Add it to the relevant index (e.g. docs/source/examples/index.rst, alongside the other MNIST notebooks) and a gallery entry if applicable. Fix this →


🟠 Correctness concerns

4. Likely dimensional mismatch when chaining the two quantum layers.

def forward(self, x):
    x = self.layer(x)          # QuantumLayer output = probability distribution over Fock states
    x = self.scaler(x)         # ScaleLayer(dim=nb_modes)
    x = self.second_interf(x)  # QuantumLayer(input_size=nb_modes)

self.layer returns a probability distribution whose size is the number of output Fock states (for n_photons=3, n_modes=3 that's 10, not 3), so self.scaler (dim=3) and self.second_interf (input_size=3) can't consume it. Please run the forward path end-to-end on a real input — I believe it will raise an input-size error just like the committed traceback. The dim parameter should be derived from the first layer's output size rather than passed independently.

5. ScaleLayer silently allows an invalid state.

if scale_type == "learned": ...
elif scale_type == "2pi": ...
elif scale_type == "pi": ...
elif scale_type == "1": ...

There's no else. An unknown scale_type produces a module with no self.scale, which then fails with an obscure AttributeError inside forward. AGENTS.md's failure policy asks to make invalid states raise clearly:

else:
    raise ValueError(f"Unknown scale_type: {scale_type!r}")

🟡 Style / maintainability

6. Remove leftover developer notes. The French fix-notes aren't intent comments and should go:

# 4. CORRECTION SYNTAXE : L'indentation de forward()
# 5. CORRECTION TYPO : self.layer(x) au lieu de self.second_inter(x)

7. Remove the empty cells. The notebook has 4 empty code cells before the model cell and 1 trailing empty cell.

8. Consistent super(). ScaleLayer uses super(ScaleLayer, self).__init__() while QLayer uses super().__init__(). Prefer the modern super().__init__() in both.

9. Missing docstrings & type hints. AGENTS.md requires NumPy-style docstrings and typed signatures for public classes/methods. If these classes are meant to be example code, at least add short docstrings; nb_photons, nb_modes, dim, and scale_type all deserve a one-line description.

10. Unused stored attributes. self.nb_modes / self.nb_photons are stored but never used after __init__.


Housekeeping

The PR description and checklist are empty. Please fill in the summary, the Jira key section, and tick the relevant boxes (ruff format/lint, docs build) so reviewers can validate.

I've left this as review feedback only and haven't changed any files. Happy to implement any of the above (e.g. add a real MNIST training flow, wire up the toctree, fix ScaleLayer validation) — just tell me which and I'll push to this branch.
· PML-250-Image-Classifications-with-QuantumLayers

@CassNot CassNot added the documentation Improvements or additions to documentation label Jul 14, 2026
@CassNot CassNot self-assigned this Jul 14, 2026
@CassNot CassNot added this to the v0.4.1 milestone Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants