Uh oh!
There was an error while loading. Please reload this page.
Add debug logging for threading and parallelization dec… - #3
Conversation
…isions Adds debug logging across Rust and Python components to help diagnose threading behavior and parallelization decisions. Includes logging for: - Rust threading decisions in getenv_use_multiple_threads() - PassManager parallel execution choices and process counts - SABRE layout trial count configuration - parallel_map execution paths and results Co-Authored with the help of: Claude
There was a problem hiding this comment.
Pull Request Overview
Adds debug logging across Python and Rust components to help diagnose threading and parallelization decisions.
- Python: Injected
logger.debugstatements inshould_run_in_parallel,parallel_map, SABRE trial count, and PassManager execution paths. - Rust: Emits threading decision to stderr when
QISKIT_DEBUG_THREADINGis set.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| qiskit/utils/parallel.py | Added logging import, logger setup, and debug statements around parallel decisions and execution. |
| qiskit/transpiler/preset_passmanagers/builtin_plugins.py | Added debug logging in _get_trial_count for SABRE threading trials. |
| qiskit/passmanager/passmanager.py | Added debug logs in callback_func for serial vs parallel PassManager runs. |
| crates/accelerate/src/lib.rs | Added Rust debug printing under QISKIT_DEBUG_THREADING environment variable. |
Comments suppressed due to low confidence (1)
qiskit/utils/parallel.py:197
- The environment variable QISKIT_IN_PARALLEL is only reset in the KeyboardInterrupt branch, so other exceptions leave it set to TRUE. Consider moving the reset into a finally block to ensure it's always restored.
os.environ["QISKIT_IN_PARALLEL"] = "FALSE"
Uh oh!
There was an error while loading. Please reload this page.
| del callback | ||
| del kwargs | ||
| logger.debug("PassManager running %d programs in parallel with %d processes", |
There was a problem hiding this comment.
The serial execution branch assigns to out but lacks a return, so execution always falls through to the parallel path. Add a return statement inside the if-block to avoid unintended parallel runs.
Uh oh!
There was an error while loading. Please reload this page.
| // Log threading decision if debug logging is enabled | ||
| if env::var("QISKIT_DEBUG_THREADING").is_ok() { | ||
| eprintln!( |
There was a problem hiding this comment.
[nitpick] Using eprintln! for debug output is inconsistent with typical Rust logging practices. Consider using the log crate to emit debug-level messages and integrate with existing logging infrastructure.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Qiskit#15943) * fix(transpiler): TemplateOptimization drops circuit global_phase on substitution circuit_to_dagdependency did not copy global_phase when converting a template QuantumCircuit to DAGDependency, so template phases were lost before matching. TemplateSubstitution.run_dag_opt also constructed a new DAGDependency for the optimised output without inheriting the original circuit's global_phase, resetting it to zero whenever at least one substitution was applied. Fixed by copying global_phase in circuit_to_dagdependency and seeding dag_dep_opt.global_phase from the circuit DAG at construction time. Also added the per-match phase subtraction (Fix 3) for when templates with nonzero global_phase become accepted via the identity check fix in Qiskit#14538. FixesQiskit#14537. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(transpiler): TemplateOptimization drops circuit global_phase on substitution circuit_to_dagdependency did not copy global_phase when converting a template QuantumCircuit to DAGDependency, so template phases were lost before matching. TemplateSubstitution.run_dag_opt also constructed a new DAGDependency for the optimised output without inheriting the original circuit's global_phase, resetting it to zero whenever at least one substitution was applied. Additionally, when a template carries a nonzero global_phase phi_T (gate content implements e^{-i*phi_T} * I while the full operator is I), the per-match phase contribution was not being subtracted from the output circuit. FixesQiskit#14537. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: black reformat test_template_matching.py Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: use RST hyperlink for issue reference in release note The :issue: role is not available in this project's Sphinx config. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(transpiler): add combined circuit+template global_phase regression test Covers the case where both the circuit and the template carry a nonzero global_phase simultaneously, confirming that fixes#2 (circuit phase preserved across substitution) and #3 (per-match template phase subtracted) compose correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(transpiler): use assertEqual for Operator comparisons in template tests Replace assertTrue(Operator(x) == Operator(y)) with assertEqual(Operator(x), Operator(y)) so that assertion failures show the actual vs expected values rather than just "False is not True". Also shorten multi-line test docstrings to concise one-liners. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address review feedback on template optimization global phase fix - Simplify comment on global phase update in template_substitution.py - Split release note into per-bug files; add fix-circuit-to-dagdependency-global-phase-14537.yaml - Update tests: use issue reproducer (4-gate circuit) in test_template_nonzero_global_phase_applied_to_circuit, merge single/multiple match tests, drop modulo from assertAlmostEqual Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix test assertions and remove duplicate test - Fix test_circuit_and_template_both_have_nonzero_global_phase expected value from np.pi/4 to 7*np.pi/12 (pi/3 + pi/4) - Remove duplicate test_circuit_global_phase_preserved_with_multiple_template_matches - Update test_template_nonzero_global_phase_applied_to_circuit to use Operator equivalence instead of phase/count assertions since partial template match does occur Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Update releasenotes/notes/fix-circuit-to-dagdependency-global-phase-14537.yaml Co-authored-by: Julien Gacon <gaconju@gmail.com> * Update releasenotes/notes/fix-template-optimization-global-phase-14537.yaml Co-authored-by: Julien Gacon <gaconju@gmail.com> * Update test/python/transpiler/test_template_matching.py Co-authored-by: Julien Gacon <gaconju@gmail.com> * Update test/python/transpiler/test_template_matching.py Actually I removed all reference to "Regression test" in the test descriptions based on this comment Qiskit#15943 (comment) Co-authored-by: Julien Gacon <gaconju@gmail.com> * Remove 'Regression test' from docstring per review feedback Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Julien Gacon <gaconju@gmail.com>
…isions
Adds debug logging across Rust and Python components to help diagnose threading behavior and parallelization decisions. Includes logging for:
Co-Authored with the help of: Claude
Summary
Details and comments