Test notebooks on pull request #483
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Should we skip these notebooks? Or somehow the specific code blocks? We want to be careful that CI doesn't burn through a bunch of compute resources. |
frankharkins
force-pushed
the
FH/notebook-ci
branch
from
fc3ec2e to
d6ebd9a
Compare
frankharkins
force-pushed
the
FH/notebook-ci
branch
from
c764a01 to
de85125
Compare
frankharkins
force-pushed
the
FH/notebook-ci
branch
from
3c127e1 to
9b58ed6
Compare
frankharkins
commented
Dec 13, 2023
Comment on lines
+35
to
+47
| - name: Check for notebooks that require LaTeX | ||
| id: latex-changed-files | ||
| uses: tj-actions/changed-files@af2816c65436325c50621100d67f6e853cd1b0f1 | ||
| with: | ||
| # Add your notebook to this list if it needs latex to run | ||
| files: | | ||
| docs/build/circuit-visualization.ipynb | ||
|
|
||
| - name: Install LaTeX dependencies | ||
| if: steps.latex-changed-files.outputs.any_changed == 'true' | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install texlive-pictures texlive-latex-extra poppler-utils |
There was a problem hiding this comment.
I've replaced the bash logic with this, should be easier to understand, and the step will now appear as "skipped" in the action log if it doesn't run.
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Eric-Arellano
approved these changes
Dec 13, 2023
arnaucasau
reviewed
Dec 14, 2023
There was a problem hiding this comment.
Great job Frank! I've been testing the script with different arguments and files, and besides a small thing (see below), all worked really well! I like a lot the way you wrote the test using actions to get the changed files, and good thinking adding the conditional to install the latex packages only when necessary!
Co-authored-by: Arnau Casau <47946624+arnaucasau@users.noreply.github.com>
Merged
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 18, 2023
This PR addresses a couple of edge cases I didn't consider in #483. - ~~Adds a token for `qiskit_ibm_provider`, and checks for trailing jobs as we already do for runtime.~~ Turns out provider and runtime share saved account info, and `QiskitRuntimeService.jobs` includes provider jobs, which means the script already handles this fine. - Don't run job-submitting notebooks without the `--submit-jobs` flag, even if passed explicitly. Currently, PR tests will fail if we edit these notebooks as the script will try to run them and they'll time out. To avoid having two lists of "notebooks that submit jobs" (one in the script and one in the action), I'm requiring the flag.
frankharkins
added a commit
to frankharkins/documentation
that referenced
this pull request
Jul 22, 2024
Adds a GitHub action to test notebook code examples on pull request. ### Handling notebooks that submit jobs We only want to submit jobs to IBM Quantum _very_ rarely (such as updating notebook outputs once per qiskit release), but runtime doesn't have a way of restricting the types of job you can send. In this PR, I've limited cell execution to 100s. This should be short enough to do most local tasks and to get backend information, but not nearly long enough for a job to run on the open provider (usually takes hours). If someone accidentally makes a PR with a notebook that submits jobs, the script will timeout and raise an error. On exit, the script cancels any jobs created since it started running. This means they won't take up any resources, and the author will be alerted when CI fails. --------- Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Co-authored-by: Arnau Casau <47946624+arnaucasau@users.noreply.github.com>
frankharkins
added a commit
to frankharkins/documentation
that referenced
this pull request
Jul 22, 2024
This PR addresses a couple of edge cases I didn't consider in Qiskit#483. - ~~Adds a token for `qiskit_ibm_provider`, and checks for trailing jobs as we already do for runtime.~~ Turns out provider and runtime share saved account info, and `QiskitRuntimeService.jobs` includes provider jobs, which means the script already handles this fine. - Don't run job-submitting notebooks without the `--submit-jobs` flag, even if passed explicitly. Currently, PR tests will fail if we edit these notebooks as the script will try to run them and they'll time out. To avoid having two lists of "notebooks that submit jobs" (one in the script and one in the action), I'm requiring the flag.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a GitHub action to test notebook code examples on pull request.
Handling notebooks that submit jobs
We only want to submit jobs to IBM Quantum very rarely (such as updating notebook outputs once per qiskit release), but runtime doesn't have a way of restricting the types of job you can send.
In this PR, I've limited cell execution to 100s. This should be short enough to do most local tasks and to get backend information, but not nearly long enough for a job to run on the open provider (usually takes hours).
If someone accidentally makes a PR with a notebook that submits jobs, the script will timeout and raise an error. On exit, the script cancels any jobs created since it started running. This means they won't take up any resources, and the author will be alerted when CI fails.