-
-
Notifications
You must be signed in to change notification settings - Fork 20
Hybrid mapping #419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Hybrid mapping #419
Changes from all commits
129b60f
08889bd
e2ff3fe
1c32d15
78dc1aa
a3ba836
5935e6f
f71fb29
3c7592b
6db5c27
47841c5
b1ac8ce
5f8473c
7491ec0
3346842
6f7a73c
6c67349
2692b96
f4874e6
54eec91
845f7de
3418936
ae870cc
861bc62
fa989b6
405bd39
7b2f321
b67d0a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
|
||
import argparse | ||
import os | ||
import platform | ||
import shutil | ||
from typing import TYPE_CHECKING | ||
|
||
|
@@ -29,7 +30,7 @@ | |
|
||
# TODO(denialhaag): Add 3.14 when all dependencies support it | ||
# https://github.com/munich-quantum-toolkit/predictor/issues/420 | ||
PYTHON_ALL_VERSIONS = ["3.10", "3.11", "3.12", "3.13"] | ||
PYTHON_ALL_VERSIONS = ["3.10", "3.11", "3.12"] | ||
|
||
if os.environ.get("CI", None): | ||
nox.options.error_on_missing_interpreters = True | ||
|
@@ -66,7 +67,9 @@ def _run_tests( | |
"test", | ||
*install_args, | ||
"pytest", | ||
"-v", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No expert on this, but these flags seem like artefacts from debugging and apparently were not necessary before (are they now?). |
||
*pytest_run_args, | ||
"--log-cli-level=INFO", | ||
*session.posargs, | ||
"--cov-config=pyproject.toml", | ||
env=env, | ||
|
@@ -82,6 +85,9 @@ def tests(session: nox.Session) -> None: | |
@nox.session(reuse_venv=True, venv_backend="uv", python=PYTHON_ALL_VERSIONS) | ||
def minimums(session: nox.Session) -> None: | ||
"""Test the minimum versions of dependencies.""" | ||
if platform.system() == "Windows": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we know why this is suddenly too slow? Seems unfortunate to miss this test if it was running fine before (note, there have always been longer test durations with the Predictor, so this is potentially fine). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually the windows test runs fine and is not skipped...I will delete this part |
||
session.skip("Too slow on Windows — skipping minimums session.") | ||
return | ||
_run_tests( | ||
session, | ||
install_args=["--resolution=lowest-direct"], | ||
|
@@ -133,5 +139,7 @@ def docs(session: nox.Session) -> None: | |
"--frozen", | ||
"sphinx-autobuild" if serve else "sphinx-build", | ||
*shared_args, | ||
"-v", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. |
||
"--log-cli-level=INFO", | ||
env=env, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,7 @@ dependencies = [ | |
"mqt.bench>=2.0.0", | ||
"qiskit!=1.3.2", # 1.3.2 causes a Qiskit error when using the CommutativeInverseCancellation pass, see https://github.com/Qiskit/qiskit/issues/13742 | ||
"pytket>=1.29.0", # lowest version that supports the used pytket AutoRebase pass instead of auto_rebase | ||
"pytket_qiskit>=0.60.0", | ||
"pytket_qiskit>=0.71.0", | ||
"sb3_contrib>=2.0.0", | ||
"tqdm>=4.66.0", | ||
"rich>=12.6.0", | ||
|
@@ -44,8 +44,10 @@ dependencies = [ | |
"numpy>=1.24; python_version >= '3.11'", | ||
"numpy>=1.22", | ||
"numpy>=1.22,<2; sys_platform == 'darwin' and 'x86_64' in platform_machine and python_version < '3.13'", # Restrict numpy v2 for macOS x86 since it is not supported anymore since torch v2.3.0 | ||
"torch>=2.7.1,<2.8.0; sys_platform == 'darwin' and 'x86_64' in platform_machine and python_version < '3.13'", # Restrict torch v2.3.0 for macOS x86 since it is not supported anymore. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the reason for deleting this? |
||
"torch>=2.7.1,<2.8.0", | ||
"typing-extensions>=4.1", # for `assert_never` | ||
"qiskit-ibm-transpiler>=0.11.1", | ||
"qiskit-ibm-ai-local-transpiler>=0.3.2; python_version < '3.13'" | ||
] | ||
|
||
classifiers = [ | ||
|
@@ -164,7 +166,7 @@ implicit_reexport = true | |
# recent versions of `gym` are typed, but stable-baselines3 pins a very old version of gym. | ||
# qiskit is not yet marked as typed, but is typed mostly. | ||
# the other libraries do not have type stubs. | ||
module = ["qiskit.*", "joblib.*", "sklearn.*", "matplotlib.*", "gymnasium.*", "mqt.bench.*", "sb3_contrib.*", "bqskit.*", "qiskit_ibm_runtime.*", "networkx.*", "stable_baselines3.*"] | ||
module = ["qiskit.*", "joblib.*", "sklearn.*", "matplotlib.*", "gymnasium.*", "mqt.bench.*", "sb3_contrib.*", "bqskit.*", "qiskit_ibm_runtime.*", "networkx.*", "stable_baselines3.*","qiskit_ibm_transpiler.*"] | ||
ignore_missing_imports = true | ||
|
||
[tool.ruff] | ||
|
@@ -248,3 +250,8 @@ fom = "fom" | |
|
||
[tool.repo-review] | ||
ignore = ["GH200"] | ||
|
||
[tool.uv] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a short comment that this is to ensure compatibility with qiskit-ibm-ai-local-transpiler, and should be removed once they require less strict versions. Btw is this the only supported version? All our other dependencies work at least with a range of supported versions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately, they only support |
||
override-dependencies = [ | ||
"networkx==2.8.5", | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -191,7 +191,10 @@ def _compile_all_circuits_devicewise( | |
if (path_compiled_circuits / (target_filename + ".qasm")).exists(): | ||
continue | ||
try: | ||
res = timeout_watcher(rl_compile, [qc, device, self.figure_of_merit, rl_pred], timeout) | ||
if sys.platform == "win32": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does the |
||
res = rl_compile(qc, device, self.figure_of_merit, rl_pred) | ||
else: | ||
res = timeout_watcher(rl_compile, [qc, device, self.figure_of_merit, rl_pred], timeout) | ||
if isinstance(res, tuple): | ||
compiled_qc = res[0] | ||
with Path(path_compiled_circuits / (target_filename + ".qasm")).open("w", encoding="utf-8") as f: | ||
|
@@ -206,7 +209,6 @@ def compile_training_circuits( | |
path_uncompiled_circuits: Path | None = None, | ||
path_compiled_circuits: Path | None = None, | ||
timeout: int = 600, | ||
num_workers: int = -1, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had to remove multiprocessing because some passes (e.g., BQSKit) already spawn parallel processes internally, which causes errors when running them within another multiprocessing context. |
||
) -> None: | ||
"""Compiles all circuits in the given directory with the given timeout and saves them in the given directory. | ||
|
||
|
@@ -227,19 +229,24 @@ def compile_training_circuits( | |
with zipfile.ZipFile(str(path_zip), "r") as zip_ref: | ||
zip_ref.extractall(path_uncompiled_circuits) | ||
|
||
Parallel(n_jobs=num_workers, verbose=100)( | ||
delayed(self._compile_all_circuits_devicewise)( | ||
device, timeout, path_uncompiled_circuits, path_compiled_circuits, logger.level | ||
if sys.platform != "win32": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is Windows not able to do this in parallel anymore? |
||
Parallel(n_jobs=1, verbose=100)( | ||
delayed(self._compile_all_circuits_devicewise)( | ||
device, timeout, path_uncompiled_circuits, path_compiled_circuits, logger.level | ||
) | ||
for device in self.devices | ||
) | ||
for device in self.devices | ||
) | ||
else: | ||
for device in self.devices: | ||
self._compile_all_circuits_devicewise( | ||
device, timeout, path_uncompiled_circuits, path_compiled_circuits, logger.level | ||
) | ||
|
||
def generate_training_data( | ||
self, | ||
path_uncompiled_circuits: Path | None = None, | ||
path_compiled_circuits: Path | None = None, | ||
path_training_data: Path | None = None, | ||
num_workers: int = -1, | ||
) -> None: | ||
"""Creates and saves training data from all generated training samples. | ||
|
||
|
@@ -267,7 +274,7 @@ def generate_training_data( | |
names_list = [] | ||
scores_list = [] | ||
|
||
results = Parallel(n_jobs=num_workers, verbose=100)( | ||
results = Parallel(n_jobs=1, verbose=100)( | ||
delayed(self._generate_training_sample)( | ||
filename.name, | ||
path_uncompiled_circuits, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems rather unfortunate to lose Python
3.13
support to enable a single pass; it would be preferable to make the pass optional based on the available Python version (ideally with a short comment that mentions the current limitation and can be revisited in the future).