From 3f4c4487f3bab777fe96d9c5c75579a66fb0f978 Mon Sep 17 00:00:00 2001 From: Frank Harkins Date: Thu, 5 Dec 2024 21:25:33 +0000 Subject: [PATCH 01/13] (rebase) Redesign nb-tester interface --- scripts/ci/check-all-notebooks-are-tested.py | 4 +- scripts/config/notebook-testing.toml | 97 ++--- scripts/nb-tester/pyproject.toml | 1 - .../qiskit_docs_notebook_tester/__init__.py | 4 +- .../qiskit_docs_notebook_tester/config.py | 347 +++++++----------- .../nb-tester/test/test_get_notebook_jobs.py | 232 ++++++++++++ 6 files changed, 415 insertions(+), 270 deletions(-) create mode 100644 scripts/nb-tester/test/test_get_notebook_jobs.py diff --git a/scripts/ci/check-all-notebooks-are-tested.py b/scripts/ci/check-all-notebooks-are-tested.py index 3987c8471fe..e10e3a3f5de 100644 --- a/scripts/ci/check-all-notebooks-are-tested.py +++ b/scripts/ci/check-all-notebooks-are-tested.py @@ -23,8 +23,8 @@ config = tomllib.loads(config_path.read_text()) categorized_notebooks = set() -for group in config.values(): - for path in group: +for group in config["groups"].values(): + for path in group.get("notebooks", []): categorized_notebooks.add(Path(path)) uncategorized = [ diff --git a/scripts/config/notebook-testing.toml b/scripts/config/notebook-testing.toml index d6df1cc0067..c40dc7f74a4 100644 --- a/scripts/config/notebook-testing.toml +++ b/scripts/config/notebook-testing.toml @@ -1,68 +1,83 @@ -# For notebooks to be tested "normally" (no mocking) -notebooks_normal_test = [ - "docs/guides/construct-circuits.ipynb", +default-strategy="ci" + +[test-strategies] +ci = { timeout = 300 } + +# For notebooks to be tested in CI "normally" (no mocking) +[groups.normal] +test-strategies.ci={} +test-strategies.hardware={} +notebooks = [ + "docs/guides/build-noise-models.ipynb", "docs/guides/circuit-library.ipynb", - "docs/guides/visualize-circuits.ipynb", "docs/guides/classical-feedforward-and-control-flow.ipynb", - "docs/guides/operators-overview.ipynb", - "docs/guides/pulse.ipynb", - "docs/guides/save-circuits.ipynb", - "docs/guides/save-jobs.ipynb", - "docs/guides/dynamic-circuits-considerations.ipynb", - "docs/guides/get-qpu-information.ipynb", - "docs/guides/visualize-results.ipynb", "docs/guides/common-parameters.ipynb", + "docs/guides/construct-circuits.ipynb", "docs/guides/create-transpiler-plugin.ipynb", "docs/guides/custom-backend.ipynb", "docs/guides/custom-transpiler-pass.ipynb", "docs/guides/defaults-and-configuration-options.ipynb", + "docs/guides/dynamic-circuits-considerations.ipynb", "docs/guides/dynamical-decoupling-pass-manager.ipynb", - "docs/guides/represent-quantum-computers.ipynb", - "docs/guides/set-optimization.ipynb", - "docs/guides/transpile-with-pass-managers.ipynb", - "docs/guides/transpiler-plugins.ipynb", - "docs/guides/transpiler-stages.ipynb", - "docs/guides/build-noise-models.ipynb", + "docs/guides/error-mitigation-and-suppression-techniques.ipynb", + "docs/guides/get-qpu-information.ipynb", "docs/guides/local-testing-mode.ipynb", - "docs/guides/plot-quantum-states.ipynb", - "docs/guides/simulate-with-qiskit-aer.ipynb", - "docs/guides/simulate-stabilizer-circuits.ipynb", "docs/guides/operator-class.ipynb", - "docs/guides/error-mitigation-and-suppression-techniques.ipynb", + "docs/guides/operators-overview.ipynb", + "docs/guides/plot-quantum-states.ipynb", + "docs/guides/pulse.ipynb", + "docs/guides/qiskit-addons-aqc-get-started.ipynb", + "docs/guides/represent-quantum-computers.ipynb", + "docs/guides/save-circuits.ipynb", + "docs/guides/save-jobs.ipynb", "docs/guides/serverless-first-program.ipynb", "docs/guides/serverless-manage-resources.ipynb", "docs/guides/serverless-run-first-workload.ipynb", + "docs/guides/set-optimization.ipynb", + "docs/guides/simulate-stabilizer-circuits.ipynb", + "docs/guides/simulate-with-qiskit-aer.ipynb", "docs/guides/specify-observables-pauli.ipynb", - "docs/guides/qiskit-addons-aqc-get-started.ipynb", -] - -# Don't test the following notebooks (this section can include glob patterns) -notebooks_exclude = [ - "docs/guides/qedma-qesem.ipynb", - "docs/guides/q-ctrl-optimization-solver.ipynb", - "docs/guides/q-ctrl-performance-management.ipynb", - "docs/guides/algorithmiq-tem.ipynb", - "docs/guides/functions.ipynb", - "docs/guides/qunasys-quri-chemistry.ipynb", - "docs/guides/multiverse-computing-singularity.ipynb", - "docs/guides/ibm-circuit-function.ipynb", - "docs/guides/qiskit-addons-sqd-get-started.ipynb", - "docs/guides/fractional-gates.ipynb", + "docs/guides/transpile-with-pass-managers.ipynb", + "docs/guides/transpiler-plugins.ipynb", + "docs/guides/transpiler-stages.ipynb", + "docs/guides/visualize-circuits.ipynb", + "docs/guides/visualize-results.ipynb", ] -# The following notebooks submit jobs that can be mocked with a 5Q simulator -notebooks_that_submit_jobs = [ - "docs/guides/primitive-input-output.ipynb", +# The following notebooks submit jobs that can be mocked with a simulator +[groups.mockable] +test-strategies.ci={ provider="qiskit-fake-provider", generic_backend_kwargs={ num_qubits=6 } } +test-strategies.hardware={} +notebooks = [ "docs/guides/debug-qiskit-runtime-jobs.ipynb", + "docs/guides/primitive-input-output.ipynb", ] -# The following notebooks submit jobs that are too big to mock with a 5Q simulator (or use functions that aren't supported on sims) +# The following notebooks submit jobs that are too big to mock with a simulator (or use functions that aren't supported on sims) # A job is "too big" if a cell can't run in under 5 mins, or we run out of # memory on a reasonable device. -notebooks_no_mock = [ +[groups.cron-job-only] +test-strategies.hardware={} +notebooks = [ "docs/guides/get-started-with-primitives.ipynb", "docs/guides/hello-world.ipynb", "docs/guides/noise-learning.ipynb", "docs/guides/qiskit-addons-obp-get-started.ipynb", "docs/guides/primitives-examples.ipynb", ] + +# Don't ever test the following notebooks +[groups.exclude] +notebooks = [ + "docs/guides/algorithmiq-tem.ipynb", + "docs/guides/fractional-gates.ipynb", + "docs/guides/functions.ipynb", + "docs/guides/ibm-circuit-function.ipynb", + "docs/guides/multiverse-computing-singularity.ipynb", + "docs/guides/q-ctrl-optimization-solver.ipynb", + "docs/guides/q-ctrl-performance-management.ipynb", + "docs/guides/qedma-qesem.ipynb", + "docs/guides/qiskit-addons-sqd-get-started.ipynb", + "docs/guides/qunasys-quri-chemistry.ipynb", +] + diff --git a/scripts/nb-tester/pyproject.toml b/scripts/nb-tester/pyproject.toml index 8954cfffc27..eba7df209df 100644 --- a/scripts/nb-tester/pyproject.toml +++ b/scripts/nb-tester/pyproject.toml @@ -14,7 +14,6 @@ dependencies = [ "nbformat~=5.10.4", "ipykernel~=6.29.2", "squeaky==0.7.0", - "tomli==2.2.1", ] [[project.authors]] diff --git a/scripts/nb-tester/qiskit_docs_notebook_tester/__init__.py b/scripts/nb-tester/qiskit_docs_notebook_tester/__init__.py index 31c3d92a9b3..f608f9739ee 100644 --- a/scripts/nb-tester/qiskit_docs_notebook_tester/__init__.py +++ b/scripts/nb-tester/qiskit_docs_notebook_tester/__init__.py @@ -19,12 +19,12 @@ import platform from datetime import datetime -from .config import get_notebook_jobs, get_args +from .config import get_notebook_jobs, get_parser from .execute import execute_notebook, cancel_trailing_jobs async def _main() -> None: - args = get_args() + args = get_parser().parse_args() jobs = list(get_notebook_jobs(args)) if not jobs: diff --git a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py index df3bbe67efd..82b9070d772 100644 --- a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py +++ b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py @@ -17,7 +17,7 @@ from textwrap import dedent from dataclasses import dataclass from pathlib import Path -import tomli +import tomllib from typing import Iterator, Literal @@ -59,143 +59,146 @@ def get_notebook_jobs(args: argparse.Namespace) -> Iterator[NotebookJob]: """ config = Config.from_args(args) - for path in config.notebooks_to_execute(): - backend_patch = None - if config.should_patch(path): - # Implements a subset of options from QiskitRuntimeService, but in - # practice any option can easily be added here - backend_patch = generate_backend_patch( - backend_name=config.args.backend, - provider=config.args.provider, - runtime_service_kwargs = { - "channel": config.args.channel, - "token": config.args.token, - "url": config.args.url, - "name": config.args.name, - "instance": config.args.instance, - }, - generic_backend_kwargs = { - "num_qubits": config.args.num_qubits, - "control_flow": config.args.control_flow - } + for group in config.groups: + for path in group["notebooks"]: + if config.cli_filenames and not matches(path, config.cli_filenames): + continue + if config.test_strategy not in group.get("test-strategies", {}): + if matches(path, config.cli_filenames): + # User explicitly defined this path so we'll explain why we're skipping it + print( + f'ℹ️ Skipping {path} (no test strategy "{config.test_strategy}" defined for this notebook)' + ) + continue + + patch = config.get_patch_for_group(group) + + def should_write(config: Config, patch: str | None): + if patch: + return Result(False, "hardware was mocked") + if not config.write: + return Result(False, "--write arg not set") + return Result(True) + + write = should_write(config, patch) + + yield NotebookJob( + path=Path(path), + pre_execute_code=PRE_EXECUTE_CODE, + backend_patch=patch, + cell_timeout=config.cell_timeout, + write=write, ) - - write = Result(True) - if reason := config.should_skip_writing(path): - write = Result(False, reason) - - yield NotebookJob( - path=path, - pre_execute_code=PRE_EXECUTE_CODE, - backend_patch=backend_patch, - cell_timeout=-1 if config.args.submit_jobs else 300, - write=write, - ) @dataclass class Config: - args: argparse.Namespace - notebooks_normal_test: list[str] - notebooks_exclude: list[str] - notebooks_that_submit_jobs: list[str] - notebooks_no_mock: list[str] - - @property - def all_job_submitting_notebooks(self) -> list[str]: - return [*self.notebooks_that_submit_jobs, *self.notebooks_no_mock] - - @property - def all_notebooks_to_test(self) -> list[str]: - return [ - *self.notebooks_normal_test, - *self.notebooks_that_submit_jobs, - *self.notebooks_no_mock, - ] - - @property - def all_notebooks(self) -> list[str]: - return [ - *self.all_notebooks_to_test, - *self.notebooks_exclude, - ] + cli_filenames: list[Path] + cell_timeout: int + test_strategy: str + write: bool + groups: list[dict] @classmethod def from_args(cls, args: argparse.Namespace) -> Config: """ - Create config from args, including loading the globs from the TOML file + Create config from args, including loading the paths from the TOML file if needed. """ - path = Path(args.config_path) + if (args.config_path or args.test_strategy) and args.patch: + raise ValueError("Can't set --patch with --config-file or --test-strategy") + + if not args.config_path: + patch_info = parse_patch_arg(args.patch) + if patch_info != {}: + print("Patching all notebooks with:", patch_info) + return cls( + cli_filenames=args.filenames, + cell_timeout=args.cell_timeout, + test_strategy="custom", + write=args.write, + groups=[ + { + "name": "main", + "notebooks": args.filenames, + "test-strategies": {"custom": patch_info}, + } + ], + ) + try: - return cls(args=args, **tomli.loads(path.read_text())) + config_path = Path(args.config_path) + config_file = tomllib.loads(config_path.read_text()) except TypeError as err: raise ValueError( - f"Couldn't read config from {path}; check it exists and the" + f"Couldn't read config from {config_path}; check it exists and the" " entries are correct." ) from err - def notebooks_to_execute(self) -> Iterator[Path]: - """ - Yield notebooks to be executed, printing messages for any skipped files. - """ - paths = map(Path, self.args.filenames or self.all_notebooks_to_test) - for path in paths: - if path.suffix != ".ipynb": - print(f"ℹ️ Skipping {path}; file is not `.ipynb` format.") - continue - - if matches(path, self.notebooks_exclude): - print( - f"ℹ️ Skipping {path}; to run it, edit `notebooks-exclude` in {self.args.config_path}." - ) - continue - - if not self.args.submit_jobs and matches(path, self.notebooks_no_mock): - print( - f"ℹ️ Skipping {path} as it doesn't work with mock hardware; use the --submit-jobs flag to run it." - ) - continue - - if self.args.only_submit_jobs and not matches( - path, self.all_job_submitting_notebooks - ): - print( - f"ℹ️ Skipping {path} as it doesn't submit jobs and the --only-submit-jobs flag is set." - ) - continue - - yield path + groups = [ + {"name": key, **value} for key, value in config_file["groups"].items() + ] + test_strategy = args.test_strategy or config_file["default-strategy"] + cell_timeout = ( + args.cell_timeout or + config_file.get("test-strategies", {}) + .get(test_strategy, {}) + .get("timeout", None) + ) + return cls( + cli_filenames=args.filenames, + cell_timeout=cell_timeout, + test_strategy=test_strategy, + write=args.write, + groups=groups, + ) - def should_patch(self, path: Path) -> bool: - if self.args.submit_jobs: - return False - return matches(path, self.notebooks_that_submit_jobs) + def get_patch_for_group(self, group: dict) -> str | None: + patch_config = group["test-strategies"].get(self.test_strategy, {}) + if patch_config == {}: + return None + return generate_backend_patch( + backend_name=patch_config.get("backend", "fake_athens"), + provider=patch_config.get("provider", "qiskit-ibm-runtime"), + generic_backend_kwargs=patch_config.get("generic_backend_kwargs", None), + runtime_service_kwargs=patch_config.get("runtime_service_kwargs", None), + ) - def should_skip_writing(self, path: Path) -> bool | str: - """Returns False or string with reason for skipping""" - if not self.args.write: - return "--write arg not set" - if self.should_patch(path): - return "hardware was mocked" - return False +def matches(path: Path | str, glob_list: list[str]) -> bool: + return any(Path(path).match(glob) for glob in glob_list) -def matches(path: Path, glob_list: list[str]) -> bool: - return any(path.match(glob) for glob in glob_list) +def parse_patch_arg(patch: str | None) -> dict: + if not patch: + return {} + try: + patch_info = tomllib.loads(f"patch={patch}")["patch"] + except tomllib.TOMLDecodeError as err: + raise ValueError( + "Problem parsing your --patch argument; " + "Make sure it's of the form '{ string=\"value\", number=4, ... }'." + ) from err + return patch_info -def get_args() -> argparse.Namespace: +def get_parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser( prog="Notebook executor", - description="For testing notebooks and updating their outputs", + description=dedent( + """\ + For testing notebooks and updating their outputs. + + There are two ways to call this script. The simplest is to just provide a list of filenames and (optionally) a backend that we'll patch qiskit-ibm-runtime's least_busy method to always return. + If providing a patch, this will apply all the notebooks. + + The other option is to provide a config file, which lets you set different patches for each notebook. See the README for more information. + """ + ), ) parser.add_argument( "filenames", help=( - "Paths to notebooks. If not provided, the script will search for " - "notebooks in `docs/`. To exclude a notebook from this process, add it " - "to `notebooks-exclude` in the config file." + "Paths to notebooks. If not provided but --config-file is provided, the script will run all notebooks in the config file." ), nargs="*", ) @@ -206,130 +209,26 @@ def get_args() -> argparse.Namespace: help="Overwrite notebooks with the results of this script's execution.", ) parser.add_argument( - "--submit-jobs", - action="store_true", - help=( - "Run notebooks that submit jobs to IBM Quantum and wait indefinitely " - "for jobs to complete. Warning: this has a real cost because it uses " - "quantum resources! Only use this argument occasionally and intentionally." - ), + "--patch", + help='If not providing a config file, an optional TOML string with the patch information. For example, \'{ provider="qiskit-ibm-runtime", backend="test-eagle" }\'', ) parser.add_argument( - "--only-submit-jobs", - action="store_true", - help=( - "Same as --submit-jobs, but only runs notebooks that submit jobs. " - "Setting this option implicitly sets --submit-jobs." - ), + "--cell-timeout", + type=int, + help="Maximum number of seconds each cell can run for. This overrides the timeout in your config file if you're using one. For no timeout, set to -1 (This is also the default with no config file).", ) parser.add_argument( "--config-path", - help="Path to a TOML file containing the globs for detecting and sorting notebooks", + help="Path to a TOML file listing the notebooks to execute and the test strategies for each notebook.", ) parser.add_argument( - "--provider", - action="store", - default="qiskit-fake-provider", - choices=["qiskit-ibm-runtime", "qiskit-fake-provider", "runtime-fake-provider"], + "--test-strategy", help=( - "Specify a provider to run notebook against." - ) - ) - parser.add_argument( - "--backend", - action="store", - default="fake_athens", - help=( - "Specify a backend to run the script against, such as 'fake_athens'" - "or 'athens'. Only relevant when `--provider` is " - "`qiskit-ibm-runtime` or `runtime-fake-provider`." - ) - ) - - generic_backend_options_group = parser.add_argument_group( - "qiskit-fake-backend options", - description=( - "These options change the behavior when --provider=qiskit-fake-backend, " - "and are passed as parameters directly to GenericBackendV2. " - "See https://docs.quantum.ibm.com/api/qiskit/qiskit.providers.fake_provider.GenericBackendV2 " - "for more details." - - ) - ) - generic_backend_options_group.add_argument( - "--num-qubits", - action="store", - type=int, - default=6, - help=( - "Specify the number of qubits for the qiskit generic backend to use" - ) - ) - generic_backend_options_group.add_argument( - "--control-flow", - action="store_true", - help=( - "Specify if the qiskit generic backend should enable control flow" - "directives on the target" - ) - ) - - runtime_options_group = parser.add_argument_group( - "qiskit-ibm-runtime options", - description=( - "These options change the behavior when --provider=qiskit-ibm-runtime, " - "and are passed as parameters directly to QiskitRuntimeService. " - "See https://docs.quantum.ibm.com/api/qiskit-ibm-runtime/qiskit_ibm_runtime.QiskitRuntimeService " - "for more details." - ) - ) - runtime_options_group.add_argument( - "--channel", - action="store", - default="ibm_quantum", - choices=["ibm_cloud", "ibm_quantum", "local"], - help=( - "Specify a channel for running the notebook against." - ) - ) - runtime_options_group.add_argument( - "--token", - action="store", - default=None, - help=( - 'IBM Cloud API key or IBM Quantum API token. Warning: for security,' - 'you should set this via an environment variable, e.g. ' - '`--token="${IQP_API_TOKEN}".' - ) - ) - runtime_options_group.add_argument( - "--url", - action="store", - default=None, - help=( - "The API URL to submit the notebook against." - ) - ) - runtime_options_group.add_argument( - "--name", - action="store", - default=None, - help=( - "Name of the qiskit account to load." - ) - ) - runtime_options_group.add_argument( - "--instance", - action="store", - default=None, - help=( - "The service instance to use." - ) + "If using a config file, the name of the testing strategy to use. " + "This affects which notebooks are run and how they're patched." + ), ) - args = parser.parse_args() - if args.only_submit_jobs: - args.submit_jobs = True - return args + return parser def generate_backend_patch( diff --git a/scripts/nb-tester/test/test_get_notebook_jobs.py b/scripts/nb-tester/test/test_get_notebook_jobs.py new file mode 100644 index 00000000000..02ee9e90f73 --- /dev/null +++ b/scripts/nb-tester/test/test_get_notebook_jobs.py @@ -0,0 +1,232 @@ +from pathlib import Path +from textwrap import dedent +from unittest import mock + +from qiskit_docs_notebook_tester.config import ( + get_notebook_jobs, + NotebookJob, + Result, + get_parser, + PRE_EXECUTE_CODE, +) + +parser = get_parser() + +QISKIT_PROVIDER_PATCH = """ +from qiskit_ibm_runtime import QiskitRuntimeService +import warnings + +warnings.filterwarnings("ignore", message="Options {.*} have no effect in local testing mode.") +warnings.filterwarnings("ignore", message="Session is not supported in local testing mode or when using a simulator.") + +from qiskit.providers.fake_provider import GenericBackendV2 +def patched_least_busy(self, *args, **kwargs): + return GenericBackendV2() + +QiskitRuntimeService.least_busy = patched_least_busy +""" + + +def test_no_config_file(): + filenames = ["path/to/notebook.ipynb", "path/to/another.ipynb"] + args = parser.parse_args(filenames) + jobs = list(get_notebook_jobs(args)) + assert jobs == [ + NotebookJob( + path=Path(path), + pre_execute_code=PRE_EXECUTE_CODE, + backend_patch=None, + cell_timeout=None, + write=Result(False, "--write arg not set"), + ) + for path in filenames + ] + + +def test_no_config_file_no_filenames(): + args = parser.parse_args([]) + assert list(get_notebook_jobs(args)) == [] + + +def test_cli_patch(): + args = parser.parse_args( + [ + "path/to/notebook.ipynb", + "--write", + "--patch", + '{ provider="qiskit-ibm-runtime", backend="test-eagle" }', + ] + ) + expected_patch = dedent( + """ + from qiskit_ibm_runtime import QiskitRuntimeService + import warnings + + warnings.filterwarnings("ignore", message="Options {.*} have no effect in local testing mode.") + warnings.filterwarnings("ignore", message="Session is not supported in local testing mode or when using a simulator.") + + def patched_least_busy(self, *args, **kwargs): + service = QiskitRuntimeService() + return service.backend("test-eagle") + + QiskitRuntimeService.least_busy = patched_least_busy + """ + ) + jobs = list(get_notebook_jobs(args)) + # Separate assertions make it easier to see diff in failed tests + assert jobs[0].backend_patch == expected_patch + assert jobs[0].write.reason == "hardware was mocked" + assert jobs == [ + NotebookJob( + path=Path("path/to/notebook.ipynb"), + pre_execute_code=PRE_EXECUTE_CODE, + backend_patch=expected_patch, + cell_timeout=None, + write=Result(False, "hardware was mocked"), + ) + ] + + +def test_config_file(): + args = parser.parse_args(["--write", "--config-path", "path/to/config.toml"]) + config_file = dedent( + """ + default-strategy="ci" + test-strategies.ci = { timeout = 350 } + + [groups] + [groups.default] + test-strategies.ci = { provider="qiskit-fake-provider" } + notebooks = ["path/to/notebook.ipynb"] + + [groups.other] + notebooks = ["path/to/another.ipynb"] + """ + ) + # Patch all read operations to return the config file + with mock.patch("pathlib.Path.read_text", lambda _: config_file): + jobs = list(get_notebook_jobs(args)) + + # Separate assertions make it easier to see diff in failed tests + assert jobs[0].backend_patch == QISKIT_PROVIDER_PATCH + assert jobs[0].write.reason == "hardware was mocked" + assert jobs == [ + NotebookJob( + path=Path("path/to/notebook.ipynb"), + pre_execute_code=PRE_EXECUTE_CODE, + backend_patch=QISKIT_PROVIDER_PATCH, + cell_timeout=350, + write=Result(False, "hardware was mocked"), + ) + ] + + +def test_config_file_with_filenames(): + args = parser.parse_args( + ["path/to/notebook.ipynb", "--write", "--config-path", "path/to/config.toml"] + ) + config_file = dedent( + """ + default-strategy="main" + + [groups] + [groups.default] + test-strategies.main = {} + notebooks = [ + "path/to/notebook.ipynb", + "path/to/another.ipynb", + ] + """ + ) + # Patch all read operations to return the config file + with mock.patch("pathlib.Path.read_text", lambda _: config_file): + jobs = list(get_notebook_jobs(args)) + assert jobs == [ + NotebookJob( + path=Path("path/to/notebook.ipynb"), + pre_execute_code=PRE_EXECUTE_CODE, + backend_patch=None, + cell_timeout=None, + write=Result(True), + ) + ] + + +def test_config_strategy_selection(): + args = parser.parse_args( + [ + "--config-path", + "path/to/config.toml", + "--test-strategy", + "other", + ] + ) + config_file = dedent( + """ + [groups] + [groups.default] + test-strategies.main = {} + notebooks = [ + "path/to/notebook.ipynb", + "path/to/another.ipynb", + ] + [groups.other] + test-strategies.other = {} + notebooks = [ + "path/to/notebook-in-other-group.ipynb", + ] + """ + ) + # Patch all read operations to return the config file + with mock.patch("pathlib.Path.read_text", lambda _: config_file): + jobs = list(get_notebook_jobs(args)) + assert jobs == [ + NotebookJob( + path=Path("path/to/notebook-in-other-group.ipynb"), + pre_execute_code=PRE_EXECUTE_CODE, + backend_patch=None, + cell_timeout=None, + write=Result(False, "--write arg not set"), + ) + ] + + +def test_config_with_different_patches_per_notebook(): + args = parser.parse_args( + [ + "--config-path", + "path/to/config.toml", + "--test-strategy", + "default", + ] + ) + config_file = dedent( + """ + [groups] + [groups.no-patch] + test-strategies.default = {} + notebooks = ["path/to/notebook.ipynb"] + [groups.patch] + test-strategies.default = { provider="qiskit-fake-provider" } + notebooks = ["path/to/another.ipynb"] + """ + ) + # Patch all read operations to return the config file + with mock.patch("pathlib.Path.read_text", lambda _: config_file): + jobs = list(get_notebook_jobs(args)) + assert jobs == [ + NotebookJob( + path=Path("path/to/notebook.ipynb"), + pre_execute_code=PRE_EXECUTE_CODE, + backend_patch=None, + cell_timeout=None, + write=Result(False, "--write arg not set"), + ), + NotebookJob( + path=Path("path/to/another.ipynb"), + pre_execute_code=PRE_EXECUTE_CODE, + backend_patch=QISKIT_PROVIDER_PATCH, + cell_timeout=None, + write=Result(False, "hardware was mocked"), + ), + ] From 01dd245e28164867c5abb14fb27c91b4fcc4d0ca Mon Sep 17 00:00:00 2001 From: Frank Harkins Date: Thu, 5 Dec 2024 21:33:57 +0000 Subject: [PATCH 02/13] Review comments --- scripts/config/notebook-testing.toml | 12 ++++++------ .../qiskit_docs_notebook_tester/__init__.py | 1 - .../qiskit_docs_notebook_tester/config.py | 14 ++++++-------- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/scripts/config/notebook-testing.toml b/scripts/config/notebook-testing.toml index c40dc7f74a4..710492ffb47 100644 --- a/scripts/config/notebook-testing.toml +++ b/scripts/config/notebook-testing.toml @@ -1,12 +1,12 @@ -default-strategy="ci" +default-strategy = "ci" [test-strategies] ci = { timeout = 300 } # For notebooks to be tested in CI "normally" (no mocking) [groups.normal] -test-strategies.ci={} -test-strategies.hardware={} +test-strategies.ci = {} +test-strategies.hardware = {} notebooks = [ "docs/guides/build-noise-models.ipynb", "docs/guides/circuit-library.ipynb", @@ -46,8 +46,8 @@ notebooks = [ # The following notebooks submit jobs that can be mocked with a simulator [groups.mockable] -test-strategies.ci={ provider="qiskit-fake-provider", generic_backend_kwargs={ num_qubits=6 } } -test-strategies.hardware={} +test-strategies.ci = { provider="qiskit-fake-provider", generic_backend_kwargs={ num_qubits=6 } } +test-strategies.hardware = {} notebooks = [ "docs/guides/debug-qiskit-runtime-jobs.ipynb", "docs/guides/primitive-input-output.ipynb", @@ -57,7 +57,7 @@ notebooks = [ # A job is "too big" if a cell can't run in under 5 mins, or we run out of # memory on a reasonable device. [groups.cron-job-only] -test-strategies.hardware={} +test-strategies.hardware = {} notebooks = [ "docs/guides/get-started-with-primitives.ipynb", "docs/guides/hello-world.ipynb", diff --git a/scripts/nb-tester/qiskit_docs_notebook_tester/__init__.py b/scripts/nb-tester/qiskit_docs_notebook_tester/__init__.py index f608f9739ee..f29cbf98ec5 100644 --- a/scripts/nb-tester/qiskit_docs_notebook_tester/__init__.py +++ b/scripts/nb-tester/qiskit_docs_notebook_tester/__init__.py @@ -15,7 +15,6 @@ import asyncio import sys -from textwrap import dedent import platform from datetime import datetime diff --git a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py index 82b9070d772..91e59deaa82 100644 --- a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py +++ b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py @@ -73,14 +73,12 @@ def get_notebook_jobs(args: argparse.Namespace) -> Iterator[NotebookJob]: patch = config.get_patch_for_group(group) - def should_write(config: Config, patch: str | None): - if patch: - return Result(False, "hardware was mocked") - if not config.write: - return Result(False, "--write arg not set") - return Result(True) - - write = should_write(config, patch) + if patch: + write = Result(False, "hardware was mocked") + elif not config.write: + write = Result(False, "--write arg not set") + else: + write = Result(True) yield NotebookJob( path=Path(path), From 5f9310dafd4e34a29674fa10e35213542b89191b Mon Sep 17 00:00:00 2001 From: Frank Harkins Date: Thu, 5 Dec 2024 21:52:50 +0000 Subject: [PATCH 03/13] Fix cron job --- .github/workflows/notebook-test-cron.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/notebook-test-cron.yml b/.github/workflows/notebook-test-cron.yml index ce929d21d75..7e12f23278d 100644 --- a/.github/workflows/notebook-test-cron.yml +++ b/.github/workflows/notebook-test-cron.yml @@ -30,7 +30,7 @@ jobs: - name: Execute notebooks timeout-minutes: 350 - run: tox -- --write --only-submit-jobs + run: tox -- --write --test-strategy hardware - name: Detect changed notebooks id: changed-notebooks From 5530a5cdd1f823910c2e0fc5cd834ffb272ae865 Mon Sep 17 00:00:00 2001 From: Frank Harkins Date: Fri, 6 Dec 2024 08:18:24 +0000 Subject: [PATCH 04/13] Allow setting patches as files --- scripts/config/notebook-testing.toml | 2 +- .../qiskit_docs_notebook_tester/config.py | 129 +++++++----------- .../nb-tester/test/test_get_notebook_jobs.py | 43 ++++-- 3 files changed, 86 insertions(+), 88 deletions(-) diff --git a/scripts/config/notebook-testing.toml b/scripts/config/notebook-testing.toml index 710492ffb47..b978bf8944f 100644 --- a/scripts/config/notebook-testing.toml +++ b/scripts/config/notebook-testing.toml @@ -46,7 +46,7 @@ notebooks = [ # The following notebooks submit jobs that can be mocked with a simulator [groups.mockable] -test-strategies.ci = { provider="qiskit-fake-provider", generic_backend_kwargs={ num_qubits=6 } } +test-strategies.ci = { patch="qiskit-fake-provider", num_qubits=6 } test-strategies.hardware = {} notebooks = [ "docs/guides/debug-qiskit-runtime-jobs.ipynb", diff --git a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py index 91e59deaa82..e5590409a51 100644 --- a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py +++ b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py @@ -18,7 +18,7 @@ from dataclasses import dataclass from pathlib import Path import tomllib -from typing import Iterator, Literal +from typing import Iterator # We always run the following code in the kernel before running the notebook @@ -30,6 +30,45 @@ _set_mpl_loglevel("critical") """ +BUILT_IN_PATCHES = { + "qiskit-fake-provider": dedent(""" + import warnings + from qiskit.providers.fake_provider import GenericBackendV2 + from qiskit_ibm_runtime import QiskitRuntimeService + + warnings.filterwarnings("ignore", message="Options {{.*}} have no effect in local testing mode.") + warnings.filterwarnings("ignore", message="Session is not supported in local testing mode or when using a simulator.") + + def patched_least_busy(self, *args, **kwargs): + return GenericBackendV2(num_qubits={num_qubits}) + + QiskitRuntimeService.least_busy = patched_least_busy + """), + + "runtime-fake-provider": dedent(""" + import warnings + from qiskit_ibm_runtime import QiskitRuntimeService + + warnings.filterwarnings("ignore", message="Options {{.*}} have no effect in local testing mode.") + warnings.filterwarnings("ignore", message="Session is not supported in local testing mode or when using a simulator.") + + def patched_least_busy(self, *args, **kwargs): + provider = FakeProviderForBackendV2() + return provider.backend("{backend}") + + QiskitRuntimeService.least_busy = patched_least_busy + """), + + "qiskit-ibm-runtime": dedent(""" + from qiskit_ibm_runtime import QiskitRuntimeService + + def patched_least_busy(self, *args, **kwargs): + service = QiskitRuntimeService({qiskit_runtime_service_args}) + return service.backend("{backend}") + + QiskitRuntimeService.least_busy = patched_least_busy + """) +} @dataclass class Result: @@ -152,13 +191,19 @@ def from_args(cls, args: argparse.Namespace) -> Config: def get_patch_for_group(self, group: dict) -> str | None: patch_config = group["test-strategies"].get(self.test_strategy, {}) - if patch_config == {}: + + patch_name = patch_config.get("patch", None) + if patch_name is None: return None - return generate_backend_patch( - backend_name=patch_config.get("backend", "fake_athens"), - provider=patch_config.get("provider", "qiskit-ibm-runtime"), - generic_backend_kwargs=patch_config.get("generic_backend_kwargs", None), - runtime_service_kwargs=patch_config.get("runtime_service_kwargs", None), + if Path(patch_name).exists(): + return Path(patch_name).read_text().format(**patch_config) + if patch_name in BUILT_IN_PATCHES: + return BUILT_IN_PATCHES[patch_name].format(**patch_config) + + valid_patch_names = list(BUILT_IN_PATCHES.keys()) + raise ValueError( + f"Could not find patch \"{patch_name}\". " + f"Patch names must be one of {valid_patch_names} or a path to a file." ) @@ -227,73 +272,3 @@ def get_parser() -> argparse.ArgumentParser: ), ) return parser - - -def generate_backend_patch( - backend_name: str, - provider: Literal[ - "qiskit-ibm-runtime", "qiskit-fake-provider", "runtime-fake-provider" - ] = "qiskit-ibm-runtime", - generic_backend_kwargs: dict[str, any] = None, - runtime_service_kwargs: dict[str, any] = None, -) -> str: - """ - Generate code for fetching a custom backend to inject into a notebook. - """ - - def render_kwarg(arg: str, val: any): - if isinstance(val, str): - return f'{arg}="{val}"' - return f"{arg}={val}" - - def render_kwargs(kwargs: dict[str, any]): - return ", ".join(render_kwarg(arg, val) for arg, val in kwargs.items()) - - patch = dedent( - """ - from qiskit_ibm_runtime import QiskitRuntimeService - import warnings - - warnings.filterwarnings("ignore", message="Options {.*} have no effect in local testing mode.") - warnings.filterwarnings("ignore", message="Session is not supported in local testing mode or when using a simulator.") - """ - ) - - if provider == "qiskit-fake-provider": - qiskit_fake_provider_args = render_kwargs(generic_backend_kwargs or {}) - patch += dedent( - f""" - from qiskit.providers.fake_provider import GenericBackendV2 - def patched_least_busy(self, *args, **kwargs): - return GenericBackendV2({qiskit_fake_provider_args}) - """ - ) - - elif provider == "runtime-fake-provider": - patch += dedent( - f""" - from qiskit_ibm_runtime.fake_provider import FakeProviderForBackendV2 - - def patched_least_busy(self, *args, **kwargs): - provider = FakeProviderForBackendV2() - return provider.backend("{backend_name}") - """ - ) - - elif provider == "qiskit-ibm-runtime": - qiskit_runtime_service_args = render_kwargs(runtime_service_kwargs or {}) - - patch += dedent( - f""" - def patched_least_busy(self, *args, **kwargs): - service = QiskitRuntimeService({qiskit_runtime_service_args}) - return service.backend("{backend_name}") - """ - ) - - else: - raise ValueError(f'Please specify a valid provider. "{provider}" is invalid.') - - patch += "\nQiskitRuntimeService.least_busy = patched_least_busy\n" - - return patch diff --git a/scripts/nb-tester/test/test_get_notebook_jobs.py b/scripts/nb-tester/test/test_get_notebook_jobs.py index 02ee9e90f73..e54282d84ce 100644 --- a/scripts/nb-tester/test/test_get_notebook_jobs.py +++ b/scripts/nb-tester/test/test_get_notebook_jobs.py @@ -13,15 +13,15 @@ parser = get_parser() QISKIT_PROVIDER_PATCH = """ -from qiskit_ibm_runtime import QiskitRuntimeService import warnings +from qiskit.providers.fake_provider import GenericBackendV2 +from qiskit_ibm_runtime import QiskitRuntimeService warnings.filterwarnings("ignore", message="Options {.*} have no effect in local testing mode.") warnings.filterwarnings("ignore", message="Session is not supported in local testing mode or when using a simulator.") -from qiskit.providers.fake_provider import GenericBackendV2 def patched_least_busy(self, *args, **kwargs): - return GenericBackendV2() + return GenericBackendV2(num_qubits=5) QiskitRuntimeService.least_busy = patched_least_busy """ @@ -54,16 +54,12 @@ def test_cli_patch(): "path/to/notebook.ipynb", "--write", "--patch", - '{ provider="qiskit-ibm-runtime", backend="test-eagle" }', + '{ patch="qiskit-ibm-runtime", backend="test-eagle", qiskit_runtime_service_args="" }', ] ) expected_patch = dedent( """ from qiskit_ibm_runtime import QiskitRuntimeService - import warnings - - warnings.filterwarnings("ignore", message="Options {.*} have no effect in local testing mode.") - warnings.filterwarnings("ignore", message="Session is not supported in local testing mode or when using a simulator.") def patched_least_busy(self, *args, **kwargs): service = QiskitRuntimeService() @@ -96,7 +92,7 @@ def test_config_file(): [groups] [groups.default] - test-strategies.ci = { provider="qiskit-fake-provider" } + test-strategies.ci = { patch="qiskit-fake-provider", num_qubits=5 } notebooks = ["path/to/notebook.ipynb"] [groups.other] @@ -207,7 +203,7 @@ def test_config_with_different_patches_per_notebook(): test-strategies.default = {} notebooks = ["path/to/notebook.ipynb"] [groups.patch] - test-strategies.default = { provider="qiskit-fake-provider" } + test-strategies.default = { patch="qiskit-fake-provider", num_qubits=5 } notebooks = ["path/to/another.ipynb"] """ ) @@ -230,3 +226,30 @@ def test_config_with_different_patches_per_notebook(): write=Result(False, "hardware was mocked"), ), ] + +def test_patch_file(): + args = parser.parse_args( + [ + "path/to/notebook.ipynb", + "--patch", + '{ patch="path/to/file.txt", text="Hello, world!" }', + ] + ) + + patch_file = "print('{text}')" + + with mock.patch("pathlib.Path.exists", lambda _: True): + with mock.patch("pathlib.Path.read_text", lambda _: patch_file): + jobs = list(get_notebook_jobs(args)) + + assert jobs[0].backend_patch == "print('Hello, world!')" + assert jobs == [ + NotebookJob( + path=Path("path/to/notebook.ipynb"), + pre_execute_code=PRE_EXECUTE_CODE, + backend_patch="print('Hello, world!')", + cell_timeout=None, + write=Result(False, "hardware was mocked"), + ) + ] + From 2c25a1844c3787317c38912c63a111e94885468e Mon Sep 17 00:00:00 2001 From: Frank Harkins Date: Fri, 6 Dec 2024 10:00:58 +0000 Subject: [PATCH 05/13] Allow multiple patch groups through CLI --- .../qiskit_docs_notebook_tester/config.py | 44 ++++++++++------ .../nb-tester/test/test_get_notebook_jobs.py | 50 ++++++++++++++++++- 2 files changed, 78 insertions(+), 16 deletions(-) diff --git a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py index e5590409a51..777b3fb1b8f 100644 --- a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py +++ b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py @@ -145,21 +145,34 @@ def from_args(cls, args: argparse.Namespace) -> Config: raise ValueError("Can't set --patch with --config-file or --test-strategy") if not args.config_path: - patch_info = parse_patch_arg(args.patch) - if patch_info != {}: - print("Patching all notebooks with:", patch_info) + groups = [] + if args.filenames: + groups.append( + { + "name": "no-patch", + "notebooks": args.filenames, + "test-strategies": {"custom": {}}, + } + ) + for index, (patch_info, *filenames) in enumerate(args.patch or []): + groups.append( + { + "name": f"patch-{index}", + "notebooks": filenames, + "test-strategies": {"custom": parse_patch_arg(patch_info)}, + } + ) + + all_filenames = args.filenames + [ + filepath for group in groups + for filepath in group["notebooks"] + ] return cls( - cli_filenames=args.filenames, + cli_filenames=all_filenames, cell_timeout=args.cell_timeout, test_strategy="custom", write=args.write, - groups=[ - { - "name": "main", - "notebooks": args.filenames, - "test-strategies": {"custom": patch_info}, - } - ], + groups=groups, ) try: @@ -167,7 +180,7 @@ def from_args(cls, args: argparse.Namespace) -> Config: config_file = tomllib.loads(config_path.read_text()) except TypeError as err: raise ValueError( - f"Couldn't read config from {config_path}; check it exists and the" + f"Couldn't read config from {args.config_path}; check it exists and the" " entries are correct." ) from err @@ -202,7 +215,7 @@ def get_patch_for_group(self, group: dict) -> str | None: valid_patch_names = list(BUILT_IN_PATCHES.keys()) raise ValueError( - f"Could not find patch \"{patch_name}\". " + f'Could not find patch "{patch_name}". ' f"Patch names must be one of {valid_patch_names} or a path to a file." ) @@ -253,7 +266,10 @@ def get_parser() -> argparse.ArgumentParser: ) parser.add_argument( "--patch", - help='If not providing a config file, an optional TOML string with the patch information. For example, \'{ provider="qiskit-ibm-runtime", backend="test-eagle" }\'', + action="append", + nargs="+", + metavar=("patch", "filenames"), + help='If not providing a config file, an optional TOML string with the patch information, followed by a list of filenames to run with that patch. For example, \'{ provider="qiskit-ibm-runtime", backend="test-eagle" }\'', ) parser.add_argument( "--cell-timeout", diff --git a/scripts/nb-tester/test/test_get_notebook_jobs.py b/scripts/nb-tester/test/test_get_notebook_jobs.py index e54282d84ce..062dc5967dd 100644 --- a/scripts/nb-tester/test/test_get_notebook_jobs.py +++ b/scripts/nb-tester/test/test_get_notebook_jobs.py @@ -51,10 +51,10 @@ def test_no_config_file_no_filenames(): def test_cli_patch(): args = parser.parse_args( [ - "path/to/notebook.ipynb", "--write", "--patch", '{ patch="qiskit-ibm-runtime", backend="test-eagle", qiskit_runtime_service_args="" }', + "path/to/notebook.ipynb", ] ) expected_patch = dedent( @@ -230,9 +230,9 @@ def test_config_with_different_patches_per_notebook(): def test_patch_file(): args = parser.parse_args( [ - "path/to/notebook.ipynb", "--patch", '{ patch="path/to/file.txt", text="Hello, world!" }', + "path/to/notebook.ipynb", ] ) @@ -253,3 +253,49 @@ def test_patch_file(): ) ] +def test_patch_file_multiple_groups(): + args = parser.parse_args( + [ + "no-patch.ipynb", + "--patch", + '{ patch="qiskit-fake-provider", num_qubits=3 }', + "fake-provider-3q-1.ipynb", + "fake-provider-3q-2.ipynb", + "--patch", + '{ patch="qiskit-fake-provider", num_qubits=5 }', + "fake-provider-5q.ipynb", + ] + ) + + jobs = list(get_notebook_jobs(args)) + + assert jobs == [ + NotebookJob( + path=Path("no-patch.ipynb"), + pre_execute_code=PRE_EXECUTE_CODE, + backend_patch=None, + cell_timeout=None, + write=Result(False, "--write arg not set"), + ), + NotebookJob( + path=Path("fake-provider-3q-1.ipynb"), + pre_execute_code=PRE_EXECUTE_CODE, + backend_patch=QISKIT_PROVIDER_PATCH.replace("5", "3"), + cell_timeout=None, + write=Result(False, "hardware was mocked"), + ), + NotebookJob( + path=Path("fake-provider-3q-2.ipynb"), + pre_execute_code=PRE_EXECUTE_CODE, + backend_patch=QISKIT_PROVIDER_PATCH.replace("5", "3"), + cell_timeout=None, + write=Result(False, "hardware was mocked"), + ), + NotebookJob( + path=Path("fake-provider-5q.ipynb"), + pre_execute_code=PRE_EXECUTE_CODE, + backend_patch=QISKIT_PROVIDER_PATCH, + cell_timeout=None, + write=Result(False, "hardware was mocked"), + ), + ] From e5b81bb37122d39e5f9bf2919b942ed023f62218 Mon Sep 17 00:00:00 2001 From: Frank Harkins Date: Fri, 6 Dec 2024 13:34:26 +0000 Subject: [PATCH 06/13] Add README --- scripts/nb-tester/README.md | 171 ++++++++++++++++++++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 scripts/nb-tester/README.md diff --git a/scripts/nb-tester/README.md b/scripts/nb-tester/README.md new file mode 100644 index 00000000000..d339883ec0f --- /dev/null +++ b/scripts/nb-tester/README.md @@ -0,0 +1,171 @@ +# Qiskit documentation notebook tester + +A tool to execute notebooks for testing, with certain features useful for +notebooks that run jobs on IBM Quantum backends. + +## Basic usage + +To use this tool, just run the `test-docs-notebooks` command, followed by a +list of notebooks you'd like to execute. + +``` +test-docs-notebooks path/to/notebook.ipynb path/to/another.ipynb +``` + +* To write the executed notebooks to disk, include the `--write` argument. +* To set a time limit for each cell, include the `--cell-timeout` argument, + followed by the time in seconds. + +``` +test-docs-notebooks --write path/to/notebook.ipynb +``` + +## Patches + +A key feature of this tool is tricking notebooks into running jobs on a +specific backend so we can test them without waiting in a queue. We do this by +monkey-patching `QiskitRuntimeService.least_busy` to return the backend of our +choosing. This technique requires notebooks always use `least_busy` to select a +backend. + +A patch is just Python code that we run in the kernel before executing your +notebook's code. You can provide your own patch code, or use one of our +built-in patches. + +You specify patch information as a TOML dict string. This dict _must_ inculde a +`patch` key, which is either a filepath to your custom patch, or the name of +one of our built-in patches (choose from `qiskit-fake-provider`, +`qiskit-ibm-runtime`, or `qiskit-fake-provider`). + +```toml +{ patch="patch-name" } +``` + +To make them more flexible, the patch code can include variables inside curly +braces. If a patch includes variables, you must provide them as extra arguments +when specifying the patch. These arguments will be injected into your patch +code using `.format(**args)`. For example, here's a simplified version of our +built-in `qiskit-fake-provider` patch: + +```python +from qiskit.providers.fake_provider import GenericBackendV2 +from qiskit_ibm_runtime import QiskitRuntimeService +QiskitRuntimeService.least_busy = lambda *_: GenericBackendV2(num_qubits={num_qubits}) +``` + +Note the variable `{num_qubits}`. To use this patch, you include the values of +any variables as extra arguments in the TOML dict. + +```toml +{ patch="qiskit-fake-provider", num_qubits=3 } +``` + +There are two ways to provide patches: Through the CLI or through a config file. +You cannot use both at the same time. + +### Setting patches through the CLI + +Use the `--patch` argument to provide patch information, followed by a list of +filenames that the patch should apply to. Filenames passed before a `--patch` +argument are not patched. + +``` +test-docs-notebooks [filenames] --patch [patch-information] [filenames] +``` + +
Example usage + +Take the following command as an example. + +``` +test-docs-notebooks\ + notebook.ipynb\ + --patch\ + '{ patch="qiskit-fake-provider", num_qubits=6 }'\ + notebook-2.ipynb\ + notebook-3.ipynb\ + --patch\ + '{ patch="qiskit-ibm-runtime", backend="test-eagle", qiskit_runtime_service_args="" }'\ + notebook-4.ipynb +``` + +This will execute: + * `notebook.ipynb` with no patch + * `notebook-2.ipynb` and `notebook-3.ipynb` with `least_busy` patched to return a 6-qubit simulator + * `notebook-4.ipynb` with `least_busy` patched to return the `test-eagle` cloud backend + +
+ +### Setting patches through a config file + +You can also choose how to patch notebooks through a TOML file. This config +file contains groups of notebook paths, and each group includes information on +how to patch that group in different situations, known as "test strategies". + +```toml +# Example config file +default-strategy = "" + +[groups.] +test-strategies. = { patch="", } +notebooks = [ + +] +``` + +
Example usage + +For example, the following config file has two groups, each with one notebook, +and two test strategies: "main" and "mock". + +```toml +# config.toml +default-strategy = "main" + +[test-strategies] +mock = { timeout = 300 } + +[groups.no-mock] +test-strategies.main = {} +notebooks = [ + "notebook.ipynb", +] + +[groups.mock] +test-strategies.main = {} +test-strategies.mock = { patch="qiskit-fake-provider", num_qubits=6 } +notebooks = [ + "another-notebook.ipynb", +] +``` + +Here's a few different commands you could run: + +* ``` + test-docs-notebooks --config-path config.toml + ``` + + This will run the config file with its default strategy, which + is "main". This means both `notebook.ipynb` and `another-notebook.ipynb` will + run without patching, as their `test-strategies.main` has no `patch` arg. + +* ``` + test-docs-notebooks --config-path config.toml --test-strategy mock + ``` + + This runs the same config file but with test strategy "mock". This will skip + `notebook.ipynb`, as it does not have a "mock" strategy defined, and will run + `another-notebook.ipynb` with a 6-qubit simulator. The "mock" strategy also + has a timeout defined, so each cell will timeout after 300s. You can override + this with your own `--timeout` argument. + +* ``` + test-docs-notebooks notebook.ipynb --config-path config.toml + ``` + + You can also provide filenames when using a config file. When filenames are + set, the script will only run notebooks passed as the filepath arg. This + command will run `notebook.ipynb` but skip `another-notebook.ipynb` as it + wasn't passed as a filename arg. + +
From 9ae730d3f460ff01d2b97176a5836399ef1baf04 Mon Sep 17 00:00:00 2001 From: Frank Harkins Date: Fri, 6 Dec 2024 13:46:10 +0000 Subject: [PATCH 07/13] lint / tweak --- .../qiskit_docs_notebook_tester/config.py | 30 +++++++++++-------- .../nb-tester/test/test_get_notebook_jobs.py | 2 ++ 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py index 777b3fb1b8f..583c5f7aef2 100644 --- a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py +++ b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py @@ -31,7 +31,8 @@ """ BUILT_IN_PATCHES = { - "qiskit-fake-provider": dedent(""" + "qiskit-fake-provider": dedent( + """ import warnings from qiskit.providers.fake_provider import GenericBackendV2 from qiskit_ibm_runtime import QiskitRuntimeService @@ -43,9 +44,10 @@ def patched_least_busy(self, *args, **kwargs): return GenericBackendV2(num_qubits={num_qubits}) QiskitRuntimeService.least_busy = patched_least_busy - """), - - "runtime-fake-provider": dedent(""" + """ + ), + "runtime-fake-provider": dedent( + """ import warnings from qiskit_ibm_runtime import QiskitRuntimeService @@ -57,9 +59,10 @@ def patched_least_busy(self, *args, **kwargs): return provider.backend("{backend}") QiskitRuntimeService.least_busy = patched_least_busy - """), - - "qiskit-ibm-runtime": dedent(""" + """ + ), + "qiskit-ibm-runtime": dedent( + """ from qiskit_ibm_runtime import QiskitRuntimeService def patched_least_busy(self, *args, **kwargs): @@ -67,9 +70,11 @@ def patched_least_busy(self, *args, **kwargs): return service.backend("{backend}") QiskitRuntimeService.least_busy = patched_least_busy - """) + """ + ), } + @dataclass class Result: ok: bool @@ -163,12 +168,11 @@ def from_args(cls, args: argparse.Namespace) -> Config: } ) - all_filenames = args.filenames + [ - filepath for group in groups - for filepath in group["notebooks"] + cli_filenames = args.filenames + [ + filepath for group in groups for filepath in group["notebooks"] ] return cls( - cli_filenames=all_filenames, + cli_filenames=cli_filenames, cell_timeout=args.cell_timeout, test_strategy="custom", write=args.write, @@ -203,7 +207,7 @@ def from_args(cls, args: argparse.Namespace) -> Config: ) def get_patch_for_group(self, group: dict) -> str | None: - patch_config = group["test-strategies"].get(self.test_strategy, {}) + patch_config = group["test-strategies"][self.test_strategy] patch_name = patch_config.get("patch", None) if patch_name is None: diff --git a/scripts/nb-tester/test/test_get_notebook_jobs.py b/scripts/nb-tester/test/test_get_notebook_jobs.py index 062dc5967dd..dc8fadf0956 100644 --- a/scripts/nb-tester/test/test_get_notebook_jobs.py +++ b/scripts/nb-tester/test/test_get_notebook_jobs.py @@ -227,6 +227,7 @@ def test_config_with_different_patches_per_notebook(): ), ] + def test_patch_file(): args = parser.parse_args( [ @@ -253,6 +254,7 @@ def test_patch_file(): ) ] + def test_patch_file_multiple_groups(): args = parser.parse_args( [ From f292212f20e00b85e5b0a5bc3896242e6640bef6 Mon Sep 17 00:00:00 2001 From: Frank Harkins Date: Fri, 6 Dec 2024 13:54:10 +0000 Subject: [PATCH 08/13] Tweak README --- scripts/nb-tester/README.md | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/scripts/nb-tester/README.md b/scripts/nb-tester/README.md index d339883ec0f..8781ec1a026 100644 --- a/scripts/nb-tester/README.md +++ b/scripts/nb-tester/README.md @@ -16,10 +16,6 @@ test-docs-notebooks path/to/notebook.ipynb path/to/another.ipynb * To set a time limit for each cell, include the `--cell-timeout` argument, followed by the time in seconds. -``` -test-docs-notebooks --write path/to/notebook.ipynb -``` - ## Patches A key feature of this tool is tricking notebooks into running jobs on a @@ -37,7 +33,7 @@ You specify patch information as a TOML dict string. This dict _must_ inculde a one of our built-in patches (choose from `qiskit-fake-provider`, `qiskit-ibm-runtime`, or `qiskit-fake-provider`). -```toml +``` { patch="patch-name" } ``` @@ -48,22 +44,23 @@ code using `.format(**args)`. For example, here's a simplified version of our built-in `qiskit-fake-provider` patch: ```python +# Example patch file from qiskit.providers.fake_provider import GenericBackendV2 from qiskit_ibm_runtime import QiskitRuntimeService QiskitRuntimeService.least_busy = lambda *_: GenericBackendV2(num_qubits={num_qubits}) ``` -Note the variable `{num_qubits}`. To use this patch, you include the values of -any variables as extra arguments in the TOML dict. +Note the variable `{num_qubits}`. To use this patch, include the values of any +variables as extra arguments in the TOML dict. -```toml +``` { patch="qiskit-fake-provider", num_qubits=3 } ``` There are two ways to provide patches: Through the CLI or through a config file. You cannot use both at the same time. -### Setting patches through the CLI +### Set patches through the CLI Use the `--patch` argument to provide patch information, followed by a list of filenames that the patch should apply to. Filenames passed before a `--patch` @@ -96,13 +93,13 @@ This will execute: -### Setting patches through a config file +### Set patches through a config file You can also choose how to patch notebooks through a TOML file. This config file contains groups of notebook paths, and each group includes information on how to patch that group in different situations, known as "test strategies". -```toml +``` # Example config file default-strategy = "" @@ -125,13 +122,13 @@ default-strategy = "main" [test-strategies] mock = { timeout = 300 } -[groups.no-mock] +[groups.group1] test-strategies.main = {} notebooks = [ "notebook.ipynb", ] -[groups.mock] +[groups.group2] test-strategies.main = {} test-strategies.mock = { patch="qiskit-fake-provider", num_qubits=6 } notebooks = [ @@ -154,10 +151,10 @@ Here's a few different commands you could run: ``` This runs the same config file but with test strategy "mock". This will skip - `notebook.ipynb`, as it does not have a "mock" strategy defined, and will run - `another-notebook.ipynb` with a 6-qubit simulator. The "mock" strategy also - has a timeout defined, so each cell will timeout after 300s. You can override - this with your own `--timeout` argument. + `notebook.ipynb`, as its group does not have a "mock" strategy defined, and + will run `another-notebook.ipynb` with a 6-qubit simulator. The "mock" + strategy also has a timeout defined, so each cell will timeout after 300s. + You can override this with your own `--timeout` argument. * ``` test-docs-notebooks notebook.ipynb --config-path config.toml From 34a0eef03b55f8155809b9a0f4ad25a39faf7f82 Mon Sep 17 00:00:00 2001 From: Frank Harkins Date: Fri, 6 Dec 2024 14:00:27 +0000 Subject: [PATCH 09/13] Fix action --- .github/workflows/notebook-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/notebook-test.yml b/.github/workflows/notebook-test.yml index 839f97e62f2..acc5c85dc05 100644 --- a/.github/workflows/notebook-test.yml +++ b/.github/workflows/notebook-test.yml @@ -47,7 +47,7 @@ jobs: """.strip().split("\n") import os github_output = os.getenv("GITHUB_OUTPUT") - all_files = "${{ steps.all-changed.outputs.all_changed_files }}".split("\n") + all_files = """${{ steps.all-changed.outputs.all_changed_files }}""".split("\n") config_changed = any(path.startswith("scripts/") for path in all_files) extra_deps = config_changed or any(path in EXTRA_DEPS_NOTEBOOKS for path in all_files) From 938dea027450f1c606881928c14dd977d1fabf20 Mon Sep 17 00:00:00 2001 From: Frank Harkins Date: Mon, 9 Dec 2024 13:53:34 +0000 Subject: [PATCH 10/13] Move patches to files --- .../nb-tester/patches/qiskit-fake-provider | 11 ++++ scripts/nb-tester/patches/qiskit-ibm-runtime | 7 +++ .../nb-tester/patches/runtime-fake-provider | 11 ++++ .../qiskit_docs_notebook_tester/config.py | 55 +++---------------- .../nb-tester/test/test_get_notebook_jobs.py | 33 ++++++++--- 5 files changed, 63 insertions(+), 54 deletions(-) create mode 100644 scripts/nb-tester/patches/qiskit-fake-provider create mode 100644 scripts/nb-tester/patches/qiskit-ibm-runtime create mode 100644 scripts/nb-tester/patches/runtime-fake-provider diff --git a/scripts/nb-tester/patches/qiskit-fake-provider b/scripts/nb-tester/patches/qiskit-fake-provider new file mode 100644 index 00000000000..7b9bd628688 --- /dev/null +++ b/scripts/nb-tester/patches/qiskit-fake-provider @@ -0,0 +1,11 @@ +import warnings +from qiskit.providers.fake_provider import GenericBackendV2 +from qiskit_ibm_runtime import QiskitRuntimeService + +warnings.filterwarnings("ignore", message="Options {{.*}} have no effect in local testing mode.") +warnings.filterwarnings("ignore", message="Session is not supported in local testing mode or when using a simulator.") + +def patched_least_busy(self, *args, **kwargs): + return GenericBackendV2(num_qubits={num_qubits}) + +QiskitRuntimeService.least_busy = patched_least_busy diff --git a/scripts/nb-tester/patches/qiskit-ibm-runtime b/scripts/nb-tester/patches/qiskit-ibm-runtime new file mode 100644 index 00000000000..a5c12841807 --- /dev/null +++ b/scripts/nb-tester/patches/qiskit-ibm-runtime @@ -0,0 +1,7 @@ +from qiskit_ibm_runtime import QiskitRuntimeService + +def patched_least_busy(self, *args, **kwargs): + service = QiskitRuntimeService({qiskit_runtime_service_args}) + return service.backend("{backend}") + +QiskitRuntimeService.least_busy = patched_least_busy diff --git a/scripts/nb-tester/patches/runtime-fake-provider b/scripts/nb-tester/patches/runtime-fake-provider new file mode 100644 index 00000000000..1a24535eb33 --- /dev/null +++ b/scripts/nb-tester/patches/runtime-fake-provider @@ -0,0 +1,11 @@ +import warnings +from qiskit_ibm_runtime import QiskitRuntimeService + +warnings.filterwarnings("ignore", message="Options {{.*}} have no effect in local testing mode.") +warnings.filterwarnings("ignore", message="Session is not supported in local testing mode or when using a simulator.") + +def patched_least_busy(self, *args, **kwargs): + provider = FakeProviderForBackendV2() + return provider.backend("{backend}") + +QiskitRuntimeService.least_busy = patched_least_busy diff --git a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py index 583c5f7aef2..fb894370b8e 100644 --- a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py +++ b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py @@ -18,6 +18,7 @@ from dataclasses import dataclass from pathlib import Path import tomllib +import importlib from typing import Iterator @@ -30,50 +31,6 @@ _set_mpl_loglevel("critical") """ -BUILT_IN_PATCHES = { - "qiskit-fake-provider": dedent( - """ - import warnings - from qiskit.providers.fake_provider import GenericBackendV2 - from qiskit_ibm_runtime import QiskitRuntimeService - - warnings.filterwarnings("ignore", message="Options {{.*}} have no effect in local testing mode.") - warnings.filterwarnings("ignore", message="Session is not supported in local testing mode or when using a simulator.") - - def patched_least_busy(self, *args, **kwargs): - return GenericBackendV2(num_qubits={num_qubits}) - - QiskitRuntimeService.least_busy = patched_least_busy - """ - ), - "runtime-fake-provider": dedent( - """ - import warnings - from qiskit_ibm_runtime import QiskitRuntimeService - - warnings.filterwarnings("ignore", message="Options {{.*}} have no effect in local testing mode.") - warnings.filterwarnings("ignore", message="Session is not supported in local testing mode or when using a simulator.") - - def patched_least_busy(self, *args, **kwargs): - provider = FakeProviderForBackendV2() - return provider.backend("{backend}") - - QiskitRuntimeService.least_busy = patched_least_busy - """ - ), - "qiskit-ibm-runtime": dedent( - """ - from qiskit_ibm_runtime import QiskitRuntimeService - - def patched_least_busy(self, *args, **kwargs): - service = QiskitRuntimeService({qiskit_runtime_service_args}) - return service.backend("{backend}") - - QiskitRuntimeService.least_busy = patched_least_busy - """ - ), -} - @dataclass class Result: @@ -212,12 +169,16 @@ def get_patch_for_group(self, group: dict) -> str | None: patch_name = patch_config.get("patch", None) if patch_name is None: return None + + built_in_patch_dir = importlib.resources.files("patches") + built_in_patch = built_in_patch_dir / patch_name + if built_in_patch.exists(): + return built_in_patch.read_text().format(**patch_config) + if Path(patch_name).exists(): return Path(patch_name).read_text().format(**patch_config) - if patch_name in BUILT_IN_PATCHES: - return BUILT_IN_PATCHES[patch_name].format(**patch_config) - valid_patch_names = list(BUILT_IN_PATCHES.keys()) + valid_patch_names = list(path.name for path in built_in_patch_dir.iterdir()) raise ValueError( f'Could not find patch "{patch_name}". ' f"Patch names must be one of {valid_patch_names} or a path to a file." diff --git a/scripts/nb-tester/test/test_get_notebook_jobs.py b/scripts/nb-tester/test/test_get_notebook_jobs.py index dc8fadf0956..2b98ab732f6 100644 --- a/scripts/nb-tester/test/test_get_notebook_jobs.py +++ b/scripts/nb-tester/test/test_get_notebook_jobs.py @@ -12,7 +12,7 @@ parser = get_parser() -QISKIT_PROVIDER_PATCH = """ +QISKIT_PROVIDER_PATCH = """\ import warnings from qiskit.providers.fake_provider import GenericBackendV2 from qiskit_ibm_runtime import QiskitRuntimeService @@ -57,8 +57,9 @@ def test_cli_patch(): "path/to/notebook.ipynb", ] ) - expected_patch = dedent( - """ + expected_patch = ( + dedent( + """ from qiskit_ibm_runtime import QiskitRuntimeService def patched_least_busy(self, *args, **kwargs): @@ -67,6 +68,8 @@ def patched_least_busy(self, *args, **kwargs): QiskitRuntimeService.least_busy = patched_least_busy """ + ).strip() + + "\n" ) jobs = list(get_notebook_jobs(args)) # Separate assertions make it easier to see diff in failed tests @@ -99,8 +102,16 @@ def test_config_file(): notebooks = ["path/to/another.ipynb"] """ ) - # Patch all read operations to return the config file - with mock.patch("pathlib.Path.read_text", lambda _: config_file): + + # Patch read operation to return the config file if file name is "config.toml" + original_read_text = Path.read_text + + def patched_read_text(self): + if self.name == "config.toml": + return config_file + return original_read_text(self) + + with mock.patch("pathlib.Path.read_text", patched_read_text): jobs = list(get_notebook_jobs(args)) # Separate assertions make it easier to see diff in failed tests @@ -207,8 +218,16 @@ def test_config_with_different_patches_per_notebook(): notebooks = ["path/to/another.ipynb"] """ ) - # Patch all read operations to return the config file - with mock.patch("pathlib.Path.read_text", lambda _: config_file): + + # Patch read operation to return the config file if file name is "config.toml" + original_read_text = Path.read_text + + def patched_read_text(self): + if self.name == "config.toml": + return config_file + return original_read_text(self) + + with mock.patch("pathlib.Path.read_text", patched_read_text): jobs = list(get_notebook_jobs(args)) assert jobs == [ NotebookJob( From bce4f675a2311a9a2c4738b0d01c8a9f7a6c9314 Mon Sep 17 00:00:00 2001 From: Frank Harkins Date: Mon, 9 Dec 2024 13:55:47 +0000 Subject: [PATCH 11/13] Fix rendering error in README Not sure what happened --- scripts/nb-tester/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/nb-tester/README.md b/scripts/nb-tester/README.md index 8781ec1a026..1c53c5e6b96 100644 --- a/scripts/nb-tester/README.md +++ b/scripts/nb-tester/README.md @@ -60,7 +60,7 @@ variables as extra arguments in the TOML dict. There are two ways to provide patches: Through the CLI or through a config file. You cannot use both at the same time. -### Set patches through the CLI +### Set patches through the CLI Use the `--patch` argument to provide patch information, followed by a list of filenames that the patch should apply to. Filenames passed before a `--patch` From b559c1b993bc6b923e613397f2f77c60101fecd8 Mon Sep 17 00:00:00 2001 From: Frank Harkins Date: Mon, 9 Dec 2024 13:58:40 +0000 Subject: [PATCH 12/13] Search local files for patch before builtins --- scripts/nb-tester/qiskit_docs_notebook_tester/config.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py index fb894370b8e..65ebbd90fa4 100644 --- a/scripts/nb-tester/qiskit_docs_notebook_tester/config.py +++ b/scripts/nb-tester/qiskit_docs_notebook_tester/config.py @@ -170,14 +170,14 @@ def get_patch_for_group(self, group: dict) -> str | None: if patch_name is None: return None + if Path(patch_name).exists(): + return Path(patch_name).read_text().format(**patch_config) + built_in_patch_dir = importlib.resources.files("patches") built_in_patch = built_in_patch_dir / patch_name if built_in_patch.exists(): return built_in_patch.read_text().format(**patch_config) - if Path(patch_name).exists(): - return Path(patch_name).read_text().format(**patch_config) - valid_patch_names = list(path.name for path in built_in_patch_dir.iterdir()) raise ValueError( f'Could not find patch "{patch_name}". ' From d982e13d7e4596cdaf6e6d719a8bc3dc1f5df116 Mon Sep 17 00:00:00 2001 From: Frank Harkins Date: Mon, 9 Dec 2024 15:55:32 +0000 Subject: [PATCH 13/13] Update scripts/nb-tester/README.md Co-authored-by: Arnau Casau <47946624+arnaucasau@users.noreply.github.com> --- scripts/nb-tester/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/nb-tester/README.md b/scripts/nb-tester/README.md index 1c53c5e6b96..0cec1aea331 100644 --- a/scripts/nb-tester/README.md +++ b/scripts/nb-tester/README.md @@ -28,7 +28,7 @@ A patch is just Python code that we run in the kernel before executing your notebook's code. You can provide your own patch code, or use one of our built-in patches. -You specify patch information as a TOML dict string. This dict _must_ inculde a +You can specify patch information as a TOML dict string. This dict _must_ include a `patch` key, which is either a filepath to your custom patch, or the name of one of our built-in patches (choose from `qiskit-fake-provider`, `qiskit-ibm-runtime`, or `qiskit-fake-provider`).