Skip to content

Commit

Permalink
Remove verify_parallel_map script (#8962)
Browse files Browse the repository at this point in the history
In #7658 we disabled multiprocessing as part of unittest runs in CI
because the multiple layers of parallelism (subprocess from stestr,
multiprocessing from qiskit, and multithreading from rust in qiskit)
were triggering a latent bug in python's multiprocessing implementation
that was blocking CI. To counter the lost coverage from disabling
multiprocessing in that PR we added a script verify_parallel_map which
force enabled multiprocessing and validated transpile() run in parallel
executed correctly. However, in #8952 we introduced a model for
selectively enabling multiprocessing just for a single test method. This
should allow us to avoid the stochastic failure triggering the deadlock
in python's multiprocessing by overloading parallelism but still test in
isolation that parallel_map() works.

This commit builds on the test class introduced in #8952 and adds
identical test cases to what was previously in verify_parallel_map.py to
move that coverage into the unit test suite. Then the
verify_parallel_map script is removed and all callers are updated to
just run unit tests instead of also executing that script.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
mtreinish and mergify[bot] committed Nov 1, 2022
1 parent d56ef8c commit 16c2d31
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 79 deletions.
2 changes: 0 additions & 2 deletions .azure/test-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ jobs:
pip install -U "cplex" "qiskit-aer" "z3-solver" -c constraints.txt
mkdir -p /tmp/terra-tests
cp -r test /tmp/terra-tests/.
cp tools/verify_parallel_map.py /tmp/terra-tests/.
cp .stestr.conf /tmp/terra-tests/.
cp -r .stestr /tmp/terra-tests/. || :
sudo apt-get update
Expand All @@ -98,7 +97,6 @@ jobs:
export PYTHONHASHSEED=$(python -S -c "import random; print(random.randint(1, 4294967295))")
echo "PYTHONHASHSEED=$PYTHONHASHSEED"
stestr run
python ./verify_parallel_map.py
popd
env:
QISKIT_PARALLEL: FALSE
Expand Down
1 change: 0 additions & 1 deletion .azure/test-macos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ jobs:
export PYTHONHASHSEED=$(python -S -c "import random; print(random.randint(1, 4294967295))")
echo "PYTHONHASHSEED=$PYTHONHASHSEED"
stestr run
python ./tools/verify_parallel_map.py
env:
QISKIT_PARALLEL: FALSE
RUST_BACKTRACE: 1
Expand Down
1 change: 0 additions & 1 deletion .azure/test-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ jobs:
export PYTHONHASHSEED=$(python -S -c "import random; print(random.randint(1, 1024))")
echo "PYTHONHASHSEED=$PYTHONHASHSEED"
stestr run
python ./tools/verify_parallel_map.py
env:
LANG: 'C.UTF-8'
PYTHONIOENCODING: 'utf-8:backslashreplace'
Expand Down
14 changes: 1 addition & 13 deletions .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,29 +42,17 @@ jobs:
pip install -r requirements-dev.txt qiskit-aer
stestr run
./grcov . --binary-path ./target/debug/ -s . --llvm --parallel -t lcov --branch --ignore-not-existing --ignore "/*" -o ./rust_unittest.info
mkdir rust_lcov
mv ./rust_unittest.info rust_lcov/.
rm *profraw
env:
QISKIT_TEST_CAPTURE_STREAMS: 1
QISKIT_PARALLEL: FALSE
PYTHON: "coverage3 run --source qiskit --parallel-mode"
- name: Generate parallel_map test
run: |
set -e
coverage3 run --source qiskit --parallel-mode ./tools/verify_parallel_map.py
./grcov . --binary-path ./target/debug/ -s . --llvm --parallel -t lcov --branch --ignore-not-existing --ignore "/*" -o ./rust_parallel_map.info
mv rust_lcov/rust_unittest.info .
env:
QISKIT_TEST_CAPTURE_STREAMS: 1
QISKIT_PARALLEL: FALSE
PYTHON: "coverage3 run --source qiskit --parallel-mode"
- name: Convert to lcov and combine data
run: |
set -e
coverage3 combine
coveragepy-lcov --output_file_path python.info
lcov --add-tracefile python.info -a rust_unittest.info -a rust_parallel_map.info -o combined.info
lcov --add-tracefile python.info -a rust_unittest.info -o combined.info
lcov --remove combined.info "target/*" -o coveralls.info
- name: Coveralls
uses: coverallsapp/github-action@master
Expand Down
21 changes: 21 additions & 0 deletions test/python/compiler/test_transpiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -1833,3 +1833,24 @@ def test_parallel_with_target(self, opt_level):
self.assertIsInstance(res, list)
for circ in res:
self.assertIsInstance(circ, QuantumCircuit)

@data(0, 1, 2, 3)
def test_parallel_dispatch(self, opt_level):
"""Test that transpile in parallel works for all optimization levels."""
backend = FakeRueschlikon()
qr = QuantumRegister(16)
cr = ClassicalRegister(16)
qc = QuantumCircuit(qr, cr)
qc.h(qr[0])
for k in range(1, 15):
qc.cx(qr[0], qr[k])
qc.measure(qr, cr)
qlist = [qc for k in range(15)]
tqc = transpile(
qlist, backend=backend, optimization_level=opt_level, seed_transpiler=424242
)
result = backend.run(tqc, seed_simulator=4242424242, shots=1000).result()
counts = result.get_counts()
for count in counts:
self.assertTrue(math.isclose(count["0000000000000000"], 500, rel_tol=0.1))
self.assertTrue(math.isclose(count["0111111111111111"], 500, rel_tol=0.1))
60 changes: 0 additions & 60 deletions tools/verify_parallel_map.py

This file was deleted.

2 changes: 0 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ deps = setuptools_rust # This is work around for the bug of tox 3 (see #8606 fo
-r{toxinidir}/requirements-dev.txt
commands =
stestr run {posargs}
{toxinidir}/tools/verify_parallel_map.py

[testenv:lint]
envdir = .tox/lint
Expand Down Expand Up @@ -64,7 +63,6 @@ deps = -r{toxinidir}/requirements.txt
qiskit-aer
commands =
stestr run {posargs}
coverage3 run --source qiskit --parallel-mode {toxinidir}/tools/verify_parallel_map.py
coverage3 combine
coverage3 report

Expand Down

0 comments on commit 16c2d31

Please sign in to comment.