Skip to content

Add BVE QNN Experiment 1 docs page and notebook. - #294

Open
CyrilDeloince wants to merge 5 commits into
merlinquantum:mainfrom
CyrilDeloince:paper-bve-qnn-docs
Open

Add BVE QNN Experiment 1 docs page and notebook.#294
CyrilDeloince wants to merge 5 commits into
merlinquantum:mainfrom
CyrilDeloince:paper-bve-qnn-docs

Conversation

@CyrilDeloince

Copy link
Copy Markdown

Summary

Adds the MerLin docs page for the Experiment 1 photonic reproduction of Jaderberg et al. (Potential of quantum scientific machine learning applied to weather modelling, Phys. Rev. A 110, 052423).

  • docs/source/reproduced_papers/reproductions/bve_qnn.rst
  • wired into the Advanced Training toctree
  • notebook copy under docs/source/notebooks/reproduced_papers/bve_qnn.ipynb
  • page gallery JSON linking to the runnable package in merlinquantum/reproduced_papers

The package itself lives in the reproduced_papers PR; this one only adds the docs entry.

Document the MerLin dual-rail photonic reproduction of Jaderberg et al. weather QSciML Exp.1 under reproduced papers.
@CassNot CassNot added the documentation Improvements or additions to documentation label Aug 6, 2026

@CassNot CassNot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @CyrilDeloince !
Just a comment for the image of your gallery card, and maybe an additional link/gallery card to your notebook in the bve_qnn.rst

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use an image on your card ? (for instance the paper's figures for quantum)
Cf the other cards to see how to link the image, but it is just one line with the path in this json

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also link the merlin notebooks here (you can use a gallery card again as per the other files)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CassNot

CassNot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

@claude review

@claude

claude Bot commented Aug 11, 2026

Copy link
Copy Markdown

Claude finished @CassNot's task in 3m 59s —— View job


Review: BVE QNN docs page + notebook

  • Gather context (repo docs conventions, gallery extension, CI settings)
  • Review the new .rst page
  • Review the gallery JSON and toctree wiring
  • Review the notebook
  • Post review

Nice reproduction write-up — the page is well structured and the honest photonic-vs-neutral-atom framing is good. Review comments below, blocking items first (no code changed, as this was a review request).


🔴 Likely to fail the docs build

.github/workflows/docs.yml:49 builds with SPHINXOPTS="-W --keep-going -n", so any Sphinx warning fails CI. Two things here emit warnings:

1. The gallery card has no image — the card is dropped, not just image-less

docs/source/_data/galleries/reproduced_papers/bve_qnn_external_links.json:1-8

image is a required field in the gallery extension: docs/source/_ext/merlin_gallery.py:167-171 logs merlin-gallery: skipped card #1 ... because 'image' is missing and returns None, so the "Project Repository" gallery on your page renders as an empty grid — and the warning fails -W. (This is also @CassNot's review point, but worth knowing it's a hard requirement, not cosmetic.) Note the extension also warns if the image path doesn't exist under docs/source/, so the file must be committed (e.g. _static/reproduced_papers/bve_qnn/...png) — see hqpinn_external_links.json:5 for the one-line pattern.

Fix this →

2. The notebook has no title

docs/source/notebooks/reproduced_papers/bve_qnn.ipynb — the first markdown cell is a Colab caveat that starts mid-sentence ("on Google Colab, we sometimes needed to run the..."), with no # heading anywhere. It's added to a toctree at docs/source/examples/index.rst:214, and a titleless document in a toctree produces toctree contains reference to document ... that doesn't have a title-W failure, plus a nameless nav entry. Every other notebook opens with an H1 (cf. photonic_QGAN.ipynb). Suggest an H1 like # Weather modelling with a photonic dual-rail QNN (Jaderberg et al., Experiment 1), then the Colab note underneath as a proper sentence.

Fix this →

Worth re-checking the Docs Validation run on this PR — I only see the Claude Code runs as completed, so the docs build result isn't visible yet.


🟠 The page is unreachable from the index

docs/source/reproduced_papers/reproduced_papers.rst:85 adds reproductions/bve_qnn to a :hidden: toctree, but no card was added to _data/galleries/reproduced_papers/reproduced_papers_advanced_training.json. Since the toctree is hidden, the gallery cards are the navigation — every other page in that section (qllm, qssl, adversarial, QGAN, hqpinn, transfer learning) has one. Without it the page only exists via direct URL/search. Convention for tags in that file is ["<Author> et al.", "<year>", "<status>"], so: ["Jaderberg et al.", "2024", "Partial"].

Fix this →


🟠 The published notebook can't be run as-is

  • bve_qnn.ipynb cell 3 and the plotting cell load ../../data/bve_qnn/sem_supervised_dataset.npz → resolves to docs/source/data/bve_qnn/, which does not exist in this repo (no docs/source/data/ at all).
  • The evaluation cell loads models/qnn_exp1_merlin_dualrail_depth32_step5000.pt, relative to cwd — also not in the repo.

CI passes -D nbsphinx_execute=never, so this won't break the build, but a reader who clicks the notebook link gets a FileNotFoundError on the third cell. No other notebook under docs/source/notebooks/ uses a local np.load path. Either fetch the artifacts from the reproduced_papers repo by URL, or add an explicit prerequisites cell at the top stating where to get the .npz and .pt and what layout to place them in.


🟡 Notebook cleanups

  • ~45 lines of pip noise. %pip install merlinquantum and !pip install cartopy both have their full "Requirement already satisfied…" output committed. This is the only notebook in docs/source/notebooks/reproduced_papers/ with pip-install cells at all — either drop them (and mention cartopy as a prerequisite for the Mollweide figure) or at minimum clear those outputs.
  • qnn is rebound to an untrained model. The sanity-check cell does qnn = MerlinDualRailPaperQNN() after model = OutputScaledQNN(qnn=qnn, ...). model.qnn still points at the original object, so nothing breaks, but from then on the name qnn refers to a different, randomly-initialised circuit — and the later "qnn trainable parameters" prints are about that one. Renaming it qnn_check removes a real re-run footgun.
  • The training loop is dead code as committed. The checkpoint-load cell runs first and sets loss_history to length 5000, so start_step = len(loss_history) = 5000 and the loop body never executes (output is just starting from: 5000). Consider putting the loop before the checkpoint load, or gating it on a TRAIN_FROM_SCRATCH = False flag with a sentence saying it's intentionally skipped when a checkpoint is present.
  • Local filename leaked into prose: the RMSE section says the qubit baseline comes from running_exp1 (4).ipynb. Replace with a link/citation to the external Qadence reproduction you cite in the .rst.
  • Duplicated sentence + typo in the parameter-count markdown cell: "In linear optics there is no native two-qubit gate, so here the nearest-neighbour photonic mixers are made trainables. Indeed, in linear optics there is no native two-qubit gate, so here the nearest-neighbour photonic mixers are made trainable" — keep one, and trainablestrainable.
  • Also (v3, trainable entangling mixers) in the PPMCC cell reads as an internal version label; and one sentence ends without a period ("…of a real-world weather PDE").

🟡 bve_qnn.rst details

  • Dangling "Related Reproductions" (bve_qnn.rst:231-232): there is no QRNN or QLSTM reproduction page under docs/source/reproduced_papers/reproductions/, and HQPINN is plain text rather than a cross-ref. Suggest :doc:hqpinn`` and dropping (or re-pointing) the QRNN/QLSTM line.
  • Title spelling is internally inconsistent: the page title and abstract use "Modelling"/"modelling", the BibTeX at bve_qnn.rst:215 uses "weather modeling". Keep the BibTeX exactly as published and align the page title with it (or at least be consistent within the page).
  • bve_qnn.rst:68-72 documents python implementation.py --paper bve_qnn --config configs/example.json (matches the convention in hqnn-myth.rst/hqpinn.rst — good), but bve_qnn.rst:203 then calls lib/runner.py the "shared-runtime entrypoint". Slightly contradictory; describe it as the paper-side runner module invoked by the repo-root implementation.py.
  • @CassNot's second point still applies: Interactive Exploration (bve_qnn.rst:171) uses a bare :doc: link. A merlin-gallery card pointing at notebooks/reproduced_papers/bve_qnn would match the other pages — remember it needs an image too, or it gets silently skipped.
  • Nit: the results table gives the paper baseline MRE as a range (7.1–10.9%) under a "Median MRE" header — a footnote saying the range is across time snapshots/seeds would help readers compare with your single 14.85% figure.

✅ What looks good

  • Toctree wiring in both reproduced_papers.rst and examples/index.rst is in the right sections.
  • Paper metadata block follows the template (including the Reproducer email convention).
  • The dual-rail ↔ magnetisation mapping and the KLM/no-native-CNOT explanation are clearly argued, and the parameter-count reconciliation (654 paper + 350 mixers + 2 affine = 1006) is verified in-notebook with asserts — that's a nice touch.
  • Reporting a worse-than-paper result with a physical explanation rather than overclaiming is the right call for a reproduction page.

Happy to implement any of the above if you want — just say which.
· branch paper-bve-qnn-docs

@CassNot CassNot self-assigned this Aug 11, 2026
@CassNot CassNot added this to the reproduced_papers milestone Aug 11, 2026
@CassNot CassNot added the reproduced-papers Description of reproduced papers (found in https://github.com/merlinquantum/reproduced_papers) label Aug 11, 2026
@CassNot

CassNot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

I had to relaunch manually the CI Tests and Docs validation because there was an error on Github end on August 6th. The Docs validation fails and you should be able to see the logs that explain the errors you are getting. I am also pasting them below:

/home/runner/work/merlin/merlin/docs/source/reproduced_papers/reproductions/bve_qnn.rst:27: WARNING: merlin-gallery: skipped card #1 in 'bve_qnn_external_links.json' because 'image' is missing.
/home/runner/work/merlin/merlin/docs/source/reproduced_papers/reproductions/bve_qnn.rst:5: WARNING: Duplicate explicit target name: "reproduced_papers".

…anups

- Add gallery card image (bve_qnn.png) to fix missing-image CI warning.
- Add card to reproduced_papers_advanced_training.json (page was unreachable).
- Fix duplicate explicit target name in RST.
- Add H1 title and prerequisites cell to notebook (toctree warning).
- Clear pip outputs, fix qnn variable rebound, remove leaked filename.
- Fix dangling Related Reproductions, runner description, add MRE footnote.
@CyrilDeloince

Copy link
Copy Markdown
Author

Thanks @CassNot !

Both docs warnings should be fixed now, and Docs Validation is green on the latest commit.

  • gallery image was missing → added _static/reproduced_papers/bve_qnn.png and referenced it in bve_qnn_external_links.json
  • duplicate target "reproduced_papers" → the two links now have distinct labels (reproduced_papers repository vs reproduced_papers repository (notebook))

https://github.com/CyrilDeloince/merlin2/blob/paper-bve-qnn-docs/docs/source/_data/galleries/reproduced_papers/bve_qnn_external_links.json
https://github.com/CyrilDeloince/merlin2/blob/paper-bve-qnn-docs/docs/source/reproduced_papers/reproductions/bve_qnn.rst

@CassNot CassNot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there !
Just the test file to removed and this should be good to be merged

Comment thread test_bve_qnn_docs.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is not needed in this PR and can be removed (you can verify ruff formatting with ruff format merlin and ruff check merlin --fix and all tests pass so it should not be needed)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks !
Done

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 reproduced-papers Description of reproduced papers (found in https://github.com/merlinquantum/reproduced_papers)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants