Skip to content

Conversation

@purva-thakre
Copy link
Collaborator

@purva-thakre purva-thakre commented May 30, 2025

To do:

@purva-thakre purva-thakre force-pushed the jupyter_sphinx_nbsphinx branch from 883955c to f781ddc Compare May 30, 2025 01:50
@nelimee nelimee added documentation Improvements or additions to documentation ci/cd Testing, review, release labels May 30, 2025
@purva-thakre purva-thakre force-pushed the jupyter_sphinx_nbsphinx branch 2 times, most recently from 6590673 to fa1ec70 Compare May 31, 2025 19:22
@purva-thakre
Copy link
Collaborator Author

purva-thakre commented May 31, 2025

@nelimee Do you happen to know if there is a time limit on the docs build? For some reason, a code block that produces an output locally is not working in the github-pages build.

Edit: It looks like this might be an nbsphinx issue, probably. I just tried a test .rst notebook with jupyter-sphinx code blocks in df6986c, and the execution is taking its time. But it fails in the end due to pymatching's output if we let the sphinx build fail for warnings.

image

If we don't let the sphinx build fail for warnings, it works!

https://tqec.github.io/tqec/pull/592/user_guide/cnot_test.html

image

Compare this to https://tqec.github.io/tqec/pull/592/gallery/cnot.html#Example-Circuit

@purva-thakre
Copy link
Collaborator Author

I still can't find the source for why the code block in the examples gallery is not executed, whereas the .rst test is executed successfully.

I cannot tell if nbgallery, nbsphinx, or myset_parser is the culprit. The latter two are added in conf.py, and the gallery page is built with the former.

tqec/docs/conf.py

Lines 45 to 50 in 8f2f79a

# A Markdown parser for Sphinx
# https://myst-parser.readthedocs.io/en/latest/index.html
"myst_parser",
# An extension allowing the inclusion of Jupyter notebooks.
# https://nbsphinx.readthedocs.io/en/0.9.3/
"nbsphinx",

Computation Gallery
===================
This section contains a collection of constructed logical computations and their simulation results.
.. nbgallery::

@nelimee
Copy link
Contributor

nelimee commented Jun 3, 2025

I still can't find the source for why the code block in the examples gallery is not executed, whereas the .rst test is executed successfully.

Isn't that because the notebook has some cells that are executed? The rule is the following:

  • if a notebook has no executed cell, it is run by Sphinx,
  • else, if at least one cell is marked as executed in the notebook, it is left as-is.

@purva-thakre
Copy link
Collaborator Author

else, if at least one cell is marked as executed in the notebook, it is left as-is.

Ooh. Thanks for this! I thought if at least one cell is unexecuted, then that one gets executed, but not the others that have an output.

@purva-thakre purva-thakre marked this pull request as ready for review June 24, 2025 18:24
@purva-thakre
Copy link
Collaborator Author

I think this PR is ready. I will take a quick look through the diff tomorrow before requesting a review.

@purva-thakre purva-thakre requested a review from nelimee June 25, 2025 00:38
Copy link
Contributor

@nelimee nelimee left a comment

Choose a reason for hiding this comment

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

This PR is fine for me, just change the cache key to make it valid across OSes and add the input enableCrossOsArchive to true to make that possible, and I will approve :)

Co-authored-by: Adrien Suau <12374487+nelimee@users.noreply.github.com>
@github-actions
Copy link

Code Coverage

Package Line Rate Complexity Health
src.tqec 100% 0
src.tqec.circuit 96% 0
src.tqec.circuit.schedule 99% 0
src.tqec.compile 93% 0
src.tqec.compile.blocks 95% 0
src.tqec.compile.blocks.layers 93% 0
src.tqec.compile.blocks.layers.atomic 96% 0
src.tqec.compile.blocks.layers.composed 98% 0
src.tqec.compile.detectors 91% 0
src.tqec.compile.observables 95% 0
src.tqec.compile.specs 91% 0
src.tqec.compile.specs.library 92% 0
src.tqec.compile.specs.library.generators 84% 0
src.tqec.compile.tree 79% 0
src.tqec.compile.tree.annotators 83% 0
src.tqec.computation 93% 0
src.tqec.gallery 100% 0
src.tqec.interop 87% 0
src.tqec.interop.collada 92% 0
src.tqec.interop.pyzx 81% 0
src.tqec.interop.pyzx.synthesis 91% 0
src.tqec.plaquette 84% 0
src.tqec.plaquette.compilation 100% 0
src.tqec.plaquette.compilation.passes 92% 0
src.tqec.plaquette.compilation.passes.transformer 98% 0
src.tqec.plaquette.library 98% 0
src.tqec.plaquette.rpng 80% 0
src.tqec.plaquette.rpng.translators 97% 0
src.tqec.post_processing 97% 0
src.tqec.post_processing.utils 96% 0
src.tqec.templates 92% 0
src.tqec.utils 94% 0
src.tqec.visualisation 100% 0
src.tqec.visualisation.computation 32% 0
src.tqec.visualisation.computation.plaquette 29% 0
Summary 89% (7059 / 7967) 0

@purva-thakre
Copy link
Collaborator Author

purva-thakre commented Jun 25, 2025

The new cache was saved after the build took approximately 50 mins. Retriggering the docs build to ensure the new cache is used.

image

Edit: The new cache was used by the retriggered build. As expected, it took approximately 8 mins. I think it might be fine to delete the previous cache that used Linux for the key.

Copy link
Contributor

@nelimee nelimee left a comment

Choose a reason for hiding this comment

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

LGTM, well done, that was a long run, but that is a very convenient feature :)

@purva-thakre purva-thakre merged commit 16627e5 into main Jun 25, 2025
7 checks passed
@purva-thakre purva-thakre deleted the jupyter_sphinx_nbsphinx branch June 25, 2025 16:45
@github-actions
Copy link

🪓 PR closed, deleted preview at https://tqec.github.io/tqec/pull/592/

@purva-thakre
Copy link
Collaborator Author

purva-thakre commented Jun 25, 2025

Merging this does not use the cache.

https://github.com/tqec/tqec/actions/runs/15882215387/job/44785294659#step:6:78

The cache exists but it can't be found on main

image

image

@nelimee
Copy link
Contributor

nelimee commented Jun 25, 2025

Merging this does not use the cache.

https://github.com/tqec/tqec/actions/runs/15882215387/job/44785294659#step:6:78

The cache exists but it can't be found on main

image

image

That's expected according to cache sharing rules I think. That should not be a problem for the next runs, as branches should be able to use the cache on main.

@purva-thakre
Copy link
Collaborator Author

purva-thakre commented Jun 25, 2025

It is strange to see two different caches of the same name. One is on main and the other one is specific to the PR.

Can you update your other PR and check if the main cache is used in your draft PR? I am worried a new cache might need to be generated for each PR.

https://github.com/tqec/tqec/actions/caches

@nelimee
Copy link
Contributor

nelimee commented Jun 25, 2025

It is strange to see two different caches of the same name. One is on main and the other one is specific to the PR.

Can you update your other PR and check if the main cache is used in your draft PR? I am worried a new cache might need to be generated for each PR.

https://github.com/tqec/tqec/actions/caches

Only tomorrow :/

According to https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/caching-dependencies-to-speed-up-workflows#restrictions-for-accessing-a-cache, cache on main can be used by other branches.

@nelimee
Copy link
Contributor

nelimee commented Jun 26, 2025

Cache hit in #562, everything seems to work correctly :)

@nelimee
Copy link
Contributor

nelimee commented Jun 26, 2025

It might be necessary to version the cache according to the current detector database version. In any case, we will need to make sure that the detector database is correctly handled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cd Testing, review, release documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants