qiskit/Cargo.toml

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

41 lines
735 B
TOML
Raw Normal View History

Implement multithreaded stochastic swap in rust (#7658) * Implement multithreaded stochastic swap in rust This commit is a rewrite of the core swap trials functionality in the StochasticSwap transpiler pass. Previously this core routine was written using Cython (see #1789) which had great performance, but that implementation was single threaded. The core of the stochastic swap algorithm by it's nature is well suited to be executed in parallel, it attempts a number of random trials and then picks the best result from all the trials and uses that for that layer. These trials can easily be run in parallel as there is no data dependency between the trials (there are shared inputs but read-only). As the algorithm generally scales exponentially the speed up from running the trials in parallel can offset this and improve the scaling of the pass. Running the pass in parallel was previously tried in #4781 using Python multiprocessing but the overhead of launching an additional process and serializing the input arrays for each trial was significantly larger than the speed gains. To run the algorithm efficiently in parallel multithreading is needed to leverage shared memory on shared inputs. This commit rewrites the cython routine using rust. This was done for two reasons. The first is that rust's safety guarantees make dealing with and writing parallel code much easier and safer. It's also multiplatform because the rust language supports native threading primatives in language. The second is while writing parallel cython code using open-mp there are limitations with it, mainly on windows. In practice it was also difficult to write and maintain parallel cython code as it has very strict requirements on python and c code interactions. It was much faster and easier to port it to rust and the performance for each iteration (outside of parallelism) is the same (in some cases marginally faster) in rust. The implementation here reuses the data structures that the previous cython implementation introduced (mainly flattening all the terra objects into 1d or 2d numpy arrays for efficient access from C). The speedups from this PR can be significant, calling transpile() on a 400 qubit (with a depth of 10) QV model circuit targetting a 409 heavy hex coupling map goes from ~200 seconds with the single threaded cython to ~60 seconds with this PR locally on a 32 core system, When transpiling a 1000 qubit (also with a depth of 10) QV model circuit targetting a 1081 qubit heavy hex coupling map goes from taking ~6500 seconds to ~720 seconds. The tradeoff with this PR is for local qiskit-terra development a rust compiler needs to be installed. This is made trivial using rustup (https://rustup.rs/), but it is an additional burden and one that we might not want to make. If so we can look at turning this PR into a separate repository/package that qiskit-terra can depend on. The tradeoff here is that we'll be adding friction to the api boundary between the pass and the core swap trials interface. But, it does ease the dependency on development for qiskit-terra. * Sanitize packaging to support future modules This commit fixes how we package the compiled rust module in qiskit-terra. As a single rust project only gives us a single compiled binary output we can't use the same scheme we did previously with cython with a separate dynamic lib file for each module. This shifts us to making the rust code build a `qiskit._accelerate` module and in that we have submodules for everything we need from compiled code. For this PR there is only one submodule, `stochastic_swap`, so for example the parallel swap_trials routine can be imported from `qiskit._accelerate.stochastic_swap.swap_trials`. In the future we can have additional submodules for other pieces of compiled code in qiskit. For example, the likely next candidate is the pauli expectation value cython module, which we'll likely port to rust and also make parallel (for sufficiently large number of qubits). In that case we'd add a new submodule for that functionality. * Adjust random normal distribution to use correct mean This commit corrects the use of the normal distribution to have the mean set to 1.0. Previously we were doing this out of band for each value by adding 1 to the random value which wasn't necessary because we could just generate it with a mean of 1.0. * Remove unecessary extra scope from locked read This commit removes an unecessary extra scope around the locked read for where we store the best solution. The scope was previously there to release the lock after we check if there is a solution or not. However this wasn't actually needed as we can just do the check inline and the lock will release after the condition block. * Remove unecessary explicit type from opt_edges variable * Fix indices typo in NLayout constructor Co-authored-by: Jake Lishman <jake@binhbar.com> * Remove explicit lifetime annotation from swap_trials Previously the swap_trials() function had an explicit lifetime annotation `'p` which wasn't necessary because the compiler can determine this on it's own. Normally when dealing with numpy views and a Python object (i.e. a GIL handle) we need a lifetime annotation to tell the rust compiler the numpy view and the python gil handle will have the same lifetime. But since swap_trials doesn't take a gil handle and operates purely in rust we don't need this lifetime and the rust compiler can deal with the lifetime of the numpy views on their own. * Use sum() instead of fold() * Fix lint and add rust style and lint checks to CI This commit fixes the python lint failures and also updates the ci configuration for the lint job to also run rust's style and lint enforcement. * Fix returned layout mapping from NLayout This commit fixes the output list from the `layout_mapping()` method of `NLayout`. Previously, it incorrectly would return the wrong indices it should be a list of virtual -> physical to qubit pairs. This commit corrects this error Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Tweak tox configuration to try and reliably build rust extension * Make swap_trials parallelization configurable This commit makes the parallelization of the swap_trials() configurable. This is dones in two ways, first a new argument parallel_threshold is added which takes an optional int which is the number of qubits to switch between a parallel and serial version. The second is that it takes into account the the state of the QISKIT_IN_PARALLEL environment variable. This variable is set to TRUE by parallel_map() when we're running in a multiprocessing context. In those cases also running stochastic swap in parallel will likely just cause too much load as we're potentially oversubscribing work to the number of available CPUs. So, if QISKIT_IN_PARALLEL is set to True we run swap_trials serially. * Revert "Make swap_trials parallelization configurable" This reverts commit 57790c84b03da10fd7296c57b38b54c5bccebf4c. That commit attempted to sovle some issues in test running, mainly around multiple parallel dispatch causing exceess load. But in practice it was broken and caused more issues than it fixed. We'll investigate and add control for the parallelization in a future commit separately after all the tests are passing so we have a good baseline. * Add docs to swap_trials() and remove unecessary num_gates arg * Fix race condition leading to non-deterministic behavior Previously, in the case of circuits that had multiple best possible depth == 1 solutions for a layer, there was a race condition in the fast exit path between the threads which could lead to a non-deterministic result even with a fixed seed. The output was always valid, but which result was dependent on which parallel thread with an ideal solution finished last and wrote to the locked best result last. This was causing weird non-deterministic test failures for some tests because of #1794 as the exact match result would change between runs. This could be a bigger issue because user expectations are that with a fixed seed set on the transpiler that the output circuit will be deterministically reproducible. To address this is issue this commit trades off some performance to ensure we're always returning a deterministic result in this case. This is accomplished by updating/checking if a depth==1 solution has been found in another trial thread we only act (so either exit early or update the already found depth == 1 solution) if that solution already found has a trial number that is less than this thread's trial number. This does limit the effectiveness of the fast exit, but in practice it should hopefully not effect the speed too much. As part of this commit some tests are updated because the new deterministic behavior is slightly different from the previous results from the cython serial implementation. I manually verified that the new output circuits are still valid (it also looks like the quality of the results in some of those cases improved, but this is strictly anecdotal and shouldn't be taken as a general trend with this PR). * Apply suggestions from code review Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Fix compiler errors in previous commit * Revert accidental commit of parallel reduction in compute_cost This was only a for local testing to prove it was a bad idea and was accidently included in the branch. We should not nest the parallel execution like this. * Eliminate short circuit for depth == 1 swap_trial() result This commit eliminates the short circuit fast return in swap_trial() when another trial thread has found an ideal solution. Trying to do this in a parallel context is tricky to make deterministic because in cases of >1 depth == 1 solutions there is an inherent race condition between the threads for writing out their depth == 1 result to the shared location. Different strategies were tried to make this reliably deterministic but there wa still a race condition. Since this was just a performance optimization to avoid doing unnecessary work this commit removes this step. Weighing improved performance against repeatability in the output of the compiler, the reproducible results are more important. After we've adopted a multithreaded stochastic swap we can investigate adding this back as a potential future optimization. * Add missing docstrings * Add section to contributing on installing form source * Make rust python classes pickleable * Add rust compiler install to linux wheel jobs * Try more tox changes to fix docs builds * Revert "Eliminate short circuit for depth == 1 swap_trial() result" This reverts commit c510764a770cb610661bdb3732337cd45ab587fd. The removal there was premature and we had a fix for the non-determinism in place, ignoring a typo which was preventing it from working. Co-Authored-By: Georgios Tsilimigkounakis <45130028+georgios-ts@users.noreply.github.com> * Fix submodule declaration and module attribute on rust classes * Fix rust lint * Fix docs job definition * Disable multiprocessing parallelism in unit tests This commit disables the multiprocessing based parallelism when running unittest jobs in CI. We historically have defaulted the use of multiprocessing in environments only where the "fork" start method is available because this has the best performance and has no caveats around how it is used by users (you don't need an `if __name__ == "__main__"` guard). However, the use of the "fork" method isn't always 100% reliable (see https://bugs.python.org/issue40379), which we saw on Python 3.9 #6188. In unittest CI (and tox) by default we use stestr which spawns (not using fork) parallel workers to run tests in parallel. With this PR this means in unittest we're now running multiple test runner subprocesses, which are executing parallel dispatched code using multiprocessing's fork start method, which is executing multithreaded rust code. This three layers of nesting is fairly reliably hanging as Python's fork doesn't seem to be able to handle this many layers of nested parallelism. There are 2 ways I've been able to fix this, the first is to change the start method used by `parallel_map()` to either "spawn" or "forkserver" either of these does not suffer from random hanging. However, doing this in the unittest context causes significant overhead and slows down test executing significantly. The other is to just disable the multiprocessing which fixes the hanging and doesn't impact runtime performance signifcantly (and might actually help in CI so we're not oversubscribing the limited resources. As I have not been able to reproduce `parallel_map()` hanging in a standalone context with multithreaded stochastic swap this commit opts for just disabling multiprocessing in CI and documenting the known issue in the release notes as this is the simpler solution. It's unlikely that users will nest parallel processes as it typically hurts performance (and parallel_map() actively guards against it), we only did it in testing previously because the tests which relied on it were a small portion of the test suite (roughly 65 tests) and typically did not have a significant impact on the total throughput of the test suite. * Fix typo in azure pipelines config * Remove unecessary extension compilation for image tests * Add test script to explicitly verify parallel dispatch In an earlier commit we disabled the use of parallel dispatch in parallel_map() to avoid a bug in cpython associated with their fork() based subprocess launch. Doing this works around the bug which was reliably triggered by running multiprocessing in parallel subprocesses. It also has the side benefit of providing a ~2x speed up for test suite execution in CI. However, this meant we lost our test coverage in CI for running parallel_map() with actual multiprocessing based parallel dispatch. To ensure we don't inadvertandtly regress this code path moving forward this commit adds a dedicated test script which runs a simple transpilation in parallel and verifies that everything works as expected with the default parallelism settings. * Avoid multi-threading when run in a multiprocessing context This commit adds a switch on running between a single threaded and a multithreaded variant of the swap_trials loop based on whether the QISKIT_IN_PARALLEL flag is set. If QISKIT_IN_PARALLEL is set to TRUE this means the `parallel_map()` function is running in the outer python context and we're running in multiprocessing already. This means we do not want to be running in multiple threads generally as that will lead to potential resource exhaustion by spawn n processes each potentially running with m threads where `n` is `min(num_phys_cpus, num_tasks)` and `m` is num_logical_cpus (although only `min(num_logical_cpus, num_trials)` will be active) which on the typical system there aren't enough cores to leverage both multiprocessing and multithreading. However, in case a user does have such an environment they can set the `QISKIT_FORCE_THREADS` env variable to `TRUE` which will use threading regardless of the status of `QISKIT_IN_PARALLEL`. * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> * Minor fixes from review comments This commits fixes some minor details found during code review. It expands the section on building from source to explain how to build a release optimized binary with editable mode, makes the QISKIT_PARALLEL env variable usage consistent across all jobs, and adds a missing shebang to the `install_rush.sh` script which is used to install rust in the manylinux container environment. * Simplify tox configuration In earlier commits the tox configuration was changed to try and fix the docs CI job by going to great effort to try and enforce that setuptools-rust was installed in all situations, even before it was actually needed. However, the problem with the docs ci job was unrelated to the tox configuration and this reverts the configuration to something that works with more versions of tox and setuptools-rust. * Add missing pieces of cargo configuration Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2022-03-01 05:49:54 +08:00
[package]
name = "qiskit-terra"
version = "0.22.0"
Implement multithreaded stochastic swap in rust (#7658) * Implement multithreaded stochastic swap in rust This commit is a rewrite of the core swap trials functionality in the StochasticSwap transpiler pass. Previously this core routine was written using Cython (see #1789) which had great performance, but that implementation was single threaded. The core of the stochastic swap algorithm by it's nature is well suited to be executed in parallel, it attempts a number of random trials and then picks the best result from all the trials and uses that for that layer. These trials can easily be run in parallel as there is no data dependency between the trials (there are shared inputs but read-only). As the algorithm generally scales exponentially the speed up from running the trials in parallel can offset this and improve the scaling of the pass. Running the pass in parallel was previously tried in #4781 using Python multiprocessing but the overhead of launching an additional process and serializing the input arrays for each trial was significantly larger than the speed gains. To run the algorithm efficiently in parallel multithreading is needed to leverage shared memory on shared inputs. This commit rewrites the cython routine using rust. This was done for two reasons. The first is that rust's safety guarantees make dealing with and writing parallel code much easier and safer. It's also multiplatform because the rust language supports native threading primatives in language. The second is while writing parallel cython code using open-mp there are limitations with it, mainly on windows. In practice it was also difficult to write and maintain parallel cython code as it has very strict requirements on python and c code interactions. It was much faster and easier to port it to rust and the performance for each iteration (outside of parallelism) is the same (in some cases marginally faster) in rust. The implementation here reuses the data structures that the previous cython implementation introduced (mainly flattening all the terra objects into 1d or 2d numpy arrays for efficient access from C). The speedups from this PR can be significant, calling transpile() on a 400 qubit (with a depth of 10) QV model circuit targetting a 409 heavy hex coupling map goes from ~200 seconds with the single threaded cython to ~60 seconds with this PR locally on a 32 core system, When transpiling a 1000 qubit (also with a depth of 10) QV model circuit targetting a 1081 qubit heavy hex coupling map goes from taking ~6500 seconds to ~720 seconds. The tradeoff with this PR is for local qiskit-terra development a rust compiler needs to be installed. This is made trivial using rustup (https://rustup.rs/), but it is an additional burden and one that we might not want to make. If so we can look at turning this PR into a separate repository/package that qiskit-terra can depend on. The tradeoff here is that we'll be adding friction to the api boundary between the pass and the core swap trials interface. But, it does ease the dependency on development for qiskit-terra. * Sanitize packaging to support future modules This commit fixes how we package the compiled rust module in qiskit-terra. As a single rust project only gives us a single compiled binary output we can't use the same scheme we did previously with cython with a separate dynamic lib file for each module. This shifts us to making the rust code build a `qiskit._accelerate` module and in that we have submodules for everything we need from compiled code. For this PR there is only one submodule, `stochastic_swap`, so for example the parallel swap_trials routine can be imported from `qiskit._accelerate.stochastic_swap.swap_trials`. In the future we can have additional submodules for other pieces of compiled code in qiskit. For example, the likely next candidate is the pauli expectation value cython module, which we'll likely port to rust and also make parallel (for sufficiently large number of qubits). In that case we'd add a new submodule for that functionality. * Adjust random normal distribution to use correct mean This commit corrects the use of the normal distribution to have the mean set to 1.0. Previously we were doing this out of band for each value by adding 1 to the random value which wasn't necessary because we could just generate it with a mean of 1.0. * Remove unecessary extra scope from locked read This commit removes an unecessary extra scope around the locked read for where we store the best solution. The scope was previously there to release the lock after we check if there is a solution or not. However this wasn't actually needed as we can just do the check inline and the lock will release after the condition block. * Remove unecessary explicit type from opt_edges variable * Fix indices typo in NLayout constructor Co-authored-by: Jake Lishman <jake@binhbar.com> * Remove explicit lifetime annotation from swap_trials Previously the swap_trials() function had an explicit lifetime annotation `'p` which wasn't necessary because the compiler can determine this on it's own. Normally when dealing with numpy views and a Python object (i.e. a GIL handle) we need a lifetime annotation to tell the rust compiler the numpy view and the python gil handle will have the same lifetime. But since swap_trials doesn't take a gil handle and operates purely in rust we don't need this lifetime and the rust compiler can deal with the lifetime of the numpy views on their own. * Use sum() instead of fold() * Fix lint and add rust style and lint checks to CI This commit fixes the python lint failures and also updates the ci configuration for the lint job to also run rust's style and lint enforcement. * Fix returned layout mapping from NLayout This commit fixes the output list from the `layout_mapping()` method of `NLayout`. Previously, it incorrectly would return the wrong indices it should be a list of virtual -> physical to qubit pairs. This commit corrects this error Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Tweak tox configuration to try and reliably build rust extension * Make swap_trials parallelization configurable This commit makes the parallelization of the swap_trials() configurable. This is dones in two ways, first a new argument parallel_threshold is added which takes an optional int which is the number of qubits to switch between a parallel and serial version. The second is that it takes into account the the state of the QISKIT_IN_PARALLEL environment variable. This variable is set to TRUE by parallel_map() when we're running in a multiprocessing context. In those cases also running stochastic swap in parallel will likely just cause too much load as we're potentially oversubscribing work to the number of available CPUs. So, if QISKIT_IN_PARALLEL is set to True we run swap_trials serially. * Revert "Make swap_trials parallelization configurable" This reverts commit 57790c84b03da10fd7296c57b38b54c5bccebf4c. That commit attempted to sovle some issues in test running, mainly around multiple parallel dispatch causing exceess load. But in practice it was broken and caused more issues than it fixed. We'll investigate and add control for the parallelization in a future commit separately after all the tests are passing so we have a good baseline. * Add docs to swap_trials() and remove unecessary num_gates arg * Fix race condition leading to non-deterministic behavior Previously, in the case of circuits that had multiple best possible depth == 1 solutions for a layer, there was a race condition in the fast exit path between the threads which could lead to a non-deterministic result even with a fixed seed. The output was always valid, but which result was dependent on which parallel thread with an ideal solution finished last and wrote to the locked best result last. This was causing weird non-deterministic test failures for some tests because of #1794 as the exact match result would change between runs. This could be a bigger issue because user expectations are that with a fixed seed set on the transpiler that the output circuit will be deterministically reproducible. To address this is issue this commit trades off some performance to ensure we're always returning a deterministic result in this case. This is accomplished by updating/checking if a depth==1 solution has been found in another trial thread we only act (so either exit early or update the already found depth == 1 solution) if that solution already found has a trial number that is less than this thread's trial number. This does limit the effectiveness of the fast exit, but in practice it should hopefully not effect the speed too much. As part of this commit some tests are updated because the new deterministic behavior is slightly different from the previous results from the cython serial implementation. I manually verified that the new output circuits are still valid (it also looks like the quality of the results in some of those cases improved, but this is strictly anecdotal and shouldn't be taken as a general trend with this PR). * Apply suggestions from code review Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Fix compiler errors in previous commit * Revert accidental commit of parallel reduction in compute_cost This was only a for local testing to prove it was a bad idea and was accidently included in the branch. We should not nest the parallel execution like this. * Eliminate short circuit for depth == 1 swap_trial() result This commit eliminates the short circuit fast return in swap_trial() when another trial thread has found an ideal solution. Trying to do this in a parallel context is tricky to make deterministic because in cases of >1 depth == 1 solutions there is an inherent race condition between the threads for writing out their depth == 1 result to the shared location. Different strategies were tried to make this reliably deterministic but there wa still a race condition. Since this was just a performance optimization to avoid doing unnecessary work this commit removes this step. Weighing improved performance against repeatability in the output of the compiler, the reproducible results are more important. After we've adopted a multithreaded stochastic swap we can investigate adding this back as a potential future optimization. * Add missing docstrings * Add section to contributing on installing form source * Make rust python classes pickleable * Add rust compiler install to linux wheel jobs * Try more tox changes to fix docs builds * Revert "Eliminate short circuit for depth == 1 swap_trial() result" This reverts commit c510764a770cb610661bdb3732337cd45ab587fd. The removal there was premature and we had a fix for the non-determinism in place, ignoring a typo which was preventing it from working. Co-Authored-By: Georgios Tsilimigkounakis <45130028+georgios-ts@users.noreply.github.com> * Fix submodule declaration and module attribute on rust classes * Fix rust lint * Fix docs job definition * Disable multiprocessing parallelism in unit tests This commit disables the multiprocessing based parallelism when running unittest jobs in CI. We historically have defaulted the use of multiprocessing in environments only where the "fork" start method is available because this has the best performance and has no caveats around how it is used by users (you don't need an `if __name__ == "__main__"` guard). However, the use of the "fork" method isn't always 100% reliable (see https://bugs.python.org/issue40379), which we saw on Python 3.9 #6188. In unittest CI (and tox) by default we use stestr which spawns (not using fork) parallel workers to run tests in parallel. With this PR this means in unittest we're now running multiple test runner subprocesses, which are executing parallel dispatched code using multiprocessing's fork start method, which is executing multithreaded rust code. This three layers of nesting is fairly reliably hanging as Python's fork doesn't seem to be able to handle this many layers of nested parallelism. There are 2 ways I've been able to fix this, the first is to change the start method used by `parallel_map()` to either "spawn" or "forkserver" either of these does not suffer from random hanging. However, doing this in the unittest context causes significant overhead and slows down test executing significantly. The other is to just disable the multiprocessing which fixes the hanging and doesn't impact runtime performance signifcantly (and might actually help in CI so we're not oversubscribing the limited resources. As I have not been able to reproduce `parallel_map()` hanging in a standalone context with multithreaded stochastic swap this commit opts for just disabling multiprocessing in CI and documenting the known issue in the release notes as this is the simpler solution. It's unlikely that users will nest parallel processes as it typically hurts performance (and parallel_map() actively guards against it), we only did it in testing previously because the tests which relied on it were a small portion of the test suite (roughly 65 tests) and typically did not have a significant impact on the total throughput of the test suite. * Fix typo in azure pipelines config * Remove unecessary extension compilation for image tests * Add test script to explicitly verify parallel dispatch In an earlier commit we disabled the use of parallel dispatch in parallel_map() to avoid a bug in cpython associated with their fork() based subprocess launch. Doing this works around the bug which was reliably triggered by running multiprocessing in parallel subprocesses. It also has the side benefit of providing a ~2x speed up for test suite execution in CI. However, this meant we lost our test coverage in CI for running parallel_map() with actual multiprocessing based parallel dispatch. To ensure we don't inadvertandtly regress this code path moving forward this commit adds a dedicated test script which runs a simple transpilation in parallel and verifies that everything works as expected with the default parallelism settings. * Avoid multi-threading when run in a multiprocessing context This commit adds a switch on running between a single threaded and a multithreaded variant of the swap_trials loop based on whether the QISKIT_IN_PARALLEL flag is set. If QISKIT_IN_PARALLEL is set to TRUE this means the `parallel_map()` function is running in the outer python context and we're running in multiprocessing already. This means we do not want to be running in multiple threads generally as that will lead to potential resource exhaustion by spawn n processes each potentially running with m threads where `n` is `min(num_phys_cpus, num_tasks)` and `m` is num_logical_cpus (although only `min(num_logical_cpus, num_trials)` will be active) which on the typical system there aren't enough cores to leverage both multiprocessing and multithreading. However, in case a user does have such an environment they can set the `QISKIT_FORCE_THREADS` env variable to `TRUE` which will use threading regardless of the status of `QISKIT_IN_PARALLEL`. * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> * Minor fixes from review comments This commits fixes some minor details found during code review. It expands the section on building from source to explain how to build a release optimized binary with editable mode, makes the QISKIT_PARALLEL env variable usage consistent across all jobs, and adds a missing shebang to the `install_rush.sh` script which is used to install rust in the manylinux container environment. * Simplify tox configuration In earlier commits the tox configuration was changed to try and fix the docs CI job by going to great effort to try and enforce that setuptools-rust was installed in all situations, even before it was actually needed. However, the problem with the docs ci job was unrelated to the tox configuration and this reverts the configuration to something that works with more versions of tox and setuptools-rust. * Add missing pieces of cargo configuration Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2022-03-01 05:49:54 +08:00
edition = "2018"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[lib]
name = "qiskit_accelerate"
crate-type = ["cdylib"]
[dependencies]
rayon = "1.5"
numpy = "0.16.2"
Implement multithreaded stochastic swap in rust (#7658) * Implement multithreaded stochastic swap in rust This commit is a rewrite of the core swap trials functionality in the StochasticSwap transpiler pass. Previously this core routine was written using Cython (see #1789) which had great performance, but that implementation was single threaded. The core of the stochastic swap algorithm by it's nature is well suited to be executed in parallel, it attempts a number of random trials and then picks the best result from all the trials and uses that for that layer. These trials can easily be run in parallel as there is no data dependency between the trials (there are shared inputs but read-only). As the algorithm generally scales exponentially the speed up from running the trials in parallel can offset this and improve the scaling of the pass. Running the pass in parallel was previously tried in #4781 using Python multiprocessing but the overhead of launching an additional process and serializing the input arrays for each trial was significantly larger than the speed gains. To run the algorithm efficiently in parallel multithreading is needed to leverage shared memory on shared inputs. This commit rewrites the cython routine using rust. This was done for two reasons. The first is that rust's safety guarantees make dealing with and writing parallel code much easier and safer. It's also multiplatform because the rust language supports native threading primatives in language. The second is while writing parallel cython code using open-mp there are limitations with it, mainly on windows. In practice it was also difficult to write and maintain parallel cython code as it has very strict requirements on python and c code interactions. It was much faster and easier to port it to rust and the performance for each iteration (outside of parallelism) is the same (in some cases marginally faster) in rust. The implementation here reuses the data structures that the previous cython implementation introduced (mainly flattening all the terra objects into 1d or 2d numpy arrays for efficient access from C). The speedups from this PR can be significant, calling transpile() on a 400 qubit (with a depth of 10) QV model circuit targetting a 409 heavy hex coupling map goes from ~200 seconds with the single threaded cython to ~60 seconds with this PR locally on a 32 core system, When transpiling a 1000 qubit (also with a depth of 10) QV model circuit targetting a 1081 qubit heavy hex coupling map goes from taking ~6500 seconds to ~720 seconds. The tradeoff with this PR is for local qiskit-terra development a rust compiler needs to be installed. This is made trivial using rustup (https://rustup.rs/), but it is an additional burden and one that we might not want to make. If so we can look at turning this PR into a separate repository/package that qiskit-terra can depend on. The tradeoff here is that we'll be adding friction to the api boundary between the pass and the core swap trials interface. But, it does ease the dependency on development for qiskit-terra. * Sanitize packaging to support future modules This commit fixes how we package the compiled rust module in qiskit-terra. As a single rust project only gives us a single compiled binary output we can't use the same scheme we did previously with cython with a separate dynamic lib file for each module. This shifts us to making the rust code build a `qiskit._accelerate` module and in that we have submodules for everything we need from compiled code. For this PR there is only one submodule, `stochastic_swap`, so for example the parallel swap_trials routine can be imported from `qiskit._accelerate.stochastic_swap.swap_trials`. In the future we can have additional submodules for other pieces of compiled code in qiskit. For example, the likely next candidate is the pauli expectation value cython module, which we'll likely port to rust and also make parallel (for sufficiently large number of qubits). In that case we'd add a new submodule for that functionality. * Adjust random normal distribution to use correct mean This commit corrects the use of the normal distribution to have the mean set to 1.0. Previously we were doing this out of band for each value by adding 1 to the random value which wasn't necessary because we could just generate it with a mean of 1.0. * Remove unecessary extra scope from locked read This commit removes an unecessary extra scope around the locked read for where we store the best solution. The scope was previously there to release the lock after we check if there is a solution or not. However this wasn't actually needed as we can just do the check inline and the lock will release after the condition block. * Remove unecessary explicit type from opt_edges variable * Fix indices typo in NLayout constructor Co-authored-by: Jake Lishman <jake@binhbar.com> * Remove explicit lifetime annotation from swap_trials Previously the swap_trials() function had an explicit lifetime annotation `'p` which wasn't necessary because the compiler can determine this on it's own. Normally when dealing with numpy views and a Python object (i.e. a GIL handle) we need a lifetime annotation to tell the rust compiler the numpy view and the python gil handle will have the same lifetime. But since swap_trials doesn't take a gil handle and operates purely in rust we don't need this lifetime and the rust compiler can deal with the lifetime of the numpy views on their own. * Use sum() instead of fold() * Fix lint and add rust style and lint checks to CI This commit fixes the python lint failures and also updates the ci configuration for the lint job to also run rust's style and lint enforcement. * Fix returned layout mapping from NLayout This commit fixes the output list from the `layout_mapping()` method of `NLayout`. Previously, it incorrectly would return the wrong indices it should be a list of virtual -> physical to qubit pairs. This commit corrects this error Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Tweak tox configuration to try and reliably build rust extension * Make swap_trials parallelization configurable This commit makes the parallelization of the swap_trials() configurable. This is dones in two ways, first a new argument parallel_threshold is added which takes an optional int which is the number of qubits to switch between a parallel and serial version. The second is that it takes into account the the state of the QISKIT_IN_PARALLEL environment variable. This variable is set to TRUE by parallel_map() when we're running in a multiprocessing context. In those cases also running stochastic swap in parallel will likely just cause too much load as we're potentially oversubscribing work to the number of available CPUs. So, if QISKIT_IN_PARALLEL is set to True we run swap_trials serially. * Revert "Make swap_trials parallelization configurable" This reverts commit 57790c84b03da10fd7296c57b38b54c5bccebf4c. That commit attempted to sovle some issues in test running, mainly around multiple parallel dispatch causing exceess load. But in practice it was broken and caused more issues than it fixed. We'll investigate and add control for the parallelization in a future commit separately after all the tests are passing so we have a good baseline. * Add docs to swap_trials() and remove unecessary num_gates arg * Fix race condition leading to non-deterministic behavior Previously, in the case of circuits that had multiple best possible depth == 1 solutions for a layer, there was a race condition in the fast exit path between the threads which could lead to a non-deterministic result even with a fixed seed. The output was always valid, but which result was dependent on which parallel thread with an ideal solution finished last and wrote to the locked best result last. This was causing weird non-deterministic test failures for some tests because of #1794 as the exact match result would change between runs. This could be a bigger issue because user expectations are that with a fixed seed set on the transpiler that the output circuit will be deterministically reproducible. To address this is issue this commit trades off some performance to ensure we're always returning a deterministic result in this case. This is accomplished by updating/checking if a depth==1 solution has been found in another trial thread we only act (so either exit early or update the already found depth == 1 solution) if that solution already found has a trial number that is less than this thread's trial number. This does limit the effectiveness of the fast exit, but in practice it should hopefully not effect the speed too much. As part of this commit some tests are updated because the new deterministic behavior is slightly different from the previous results from the cython serial implementation. I manually verified that the new output circuits are still valid (it also looks like the quality of the results in some of those cases improved, but this is strictly anecdotal and shouldn't be taken as a general trend with this PR). * Apply suggestions from code review Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Fix compiler errors in previous commit * Revert accidental commit of parallel reduction in compute_cost This was only a for local testing to prove it was a bad idea and was accidently included in the branch. We should not nest the parallel execution like this. * Eliminate short circuit for depth == 1 swap_trial() result This commit eliminates the short circuit fast return in swap_trial() when another trial thread has found an ideal solution. Trying to do this in a parallel context is tricky to make deterministic because in cases of >1 depth == 1 solutions there is an inherent race condition between the threads for writing out their depth == 1 result to the shared location. Different strategies were tried to make this reliably deterministic but there wa still a race condition. Since this was just a performance optimization to avoid doing unnecessary work this commit removes this step. Weighing improved performance against repeatability in the output of the compiler, the reproducible results are more important. After we've adopted a multithreaded stochastic swap we can investigate adding this back as a potential future optimization. * Add missing docstrings * Add section to contributing on installing form source * Make rust python classes pickleable * Add rust compiler install to linux wheel jobs * Try more tox changes to fix docs builds * Revert "Eliminate short circuit for depth == 1 swap_trial() result" This reverts commit c510764a770cb610661bdb3732337cd45ab587fd. The removal there was premature and we had a fix for the non-determinism in place, ignoring a typo which was preventing it from working. Co-Authored-By: Georgios Tsilimigkounakis <45130028+georgios-ts@users.noreply.github.com> * Fix submodule declaration and module attribute on rust classes * Fix rust lint * Fix docs job definition * Disable multiprocessing parallelism in unit tests This commit disables the multiprocessing based parallelism when running unittest jobs in CI. We historically have defaulted the use of multiprocessing in environments only where the "fork" start method is available because this has the best performance and has no caveats around how it is used by users (you don't need an `if __name__ == "__main__"` guard). However, the use of the "fork" method isn't always 100% reliable (see https://bugs.python.org/issue40379), which we saw on Python 3.9 #6188. In unittest CI (and tox) by default we use stestr which spawns (not using fork) parallel workers to run tests in parallel. With this PR this means in unittest we're now running multiple test runner subprocesses, which are executing parallel dispatched code using multiprocessing's fork start method, which is executing multithreaded rust code. This three layers of nesting is fairly reliably hanging as Python's fork doesn't seem to be able to handle this many layers of nested parallelism. There are 2 ways I've been able to fix this, the first is to change the start method used by `parallel_map()` to either "spawn" or "forkserver" either of these does not suffer from random hanging. However, doing this in the unittest context causes significant overhead and slows down test executing significantly. The other is to just disable the multiprocessing which fixes the hanging and doesn't impact runtime performance signifcantly (and might actually help in CI so we're not oversubscribing the limited resources. As I have not been able to reproduce `parallel_map()` hanging in a standalone context with multithreaded stochastic swap this commit opts for just disabling multiprocessing in CI and documenting the known issue in the release notes as this is the simpler solution. It's unlikely that users will nest parallel processes as it typically hurts performance (and parallel_map() actively guards against it), we only did it in testing previously because the tests which relied on it were a small portion of the test suite (roughly 65 tests) and typically did not have a significant impact on the total throughput of the test suite. * Fix typo in azure pipelines config * Remove unecessary extension compilation for image tests * Add test script to explicitly verify parallel dispatch In an earlier commit we disabled the use of parallel dispatch in parallel_map() to avoid a bug in cpython associated with their fork() based subprocess launch. Doing this works around the bug which was reliably triggered by running multiprocessing in parallel subprocesses. It also has the side benefit of providing a ~2x speed up for test suite execution in CI. However, this meant we lost our test coverage in CI for running parallel_map() with actual multiprocessing based parallel dispatch. To ensure we don't inadvertandtly regress this code path moving forward this commit adds a dedicated test script which runs a simple transpilation in parallel and verifies that everything works as expected with the default parallelism settings. * Avoid multi-threading when run in a multiprocessing context This commit adds a switch on running between a single threaded and a multithreaded variant of the swap_trials loop based on whether the QISKIT_IN_PARALLEL flag is set. If QISKIT_IN_PARALLEL is set to TRUE this means the `parallel_map()` function is running in the outer python context and we're running in multiprocessing already. This means we do not want to be running in multiple threads generally as that will lead to potential resource exhaustion by spawn n processes each potentially running with m threads where `n` is `min(num_phys_cpus, num_tasks)` and `m` is num_logical_cpus (although only `min(num_logical_cpus, num_trials)` will be active) which on the typical system there aren't enough cores to leverage both multiprocessing and multithreading. However, in case a user does have such an environment they can set the `QISKIT_FORCE_THREADS` env variable to `TRUE` which will use threading regardless of the status of `QISKIT_IN_PARALLEL`. * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> * Minor fixes from review comments This commits fixes some minor details found during code review. It expands the section on building from source to explain how to build a release optimized binary with editable mode, makes the QISKIT_PARALLEL env variable usage consistent across all jobs, and adds a missing shebang to the `install_rush.sh` script which is used to install rust in the manylinux container environment. * Simplify tox configuration In earlier commits the tox configuration was changed to try and fix the docs CI job by going to great effort to try and enforce that setuptools-rust was installed in all situations, even before it was actually needed. However, the problem with the docs ci job was unrelated to the tox configuration and this reverts the configuration to something that works with more versions of tox and setuptools-rust. * Add missing pieces of cargo configuration Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2022-03-01 05:49:54 +08:00
rand = "0.8"
rand_pcg = "0.3"
rand_distr = "0.4.3"
ahash = "0.8.0"
Replace pauli expectation value cython with multithreaded rust implementation (#7702) * Replace pauli expectation value cython with rust implementation This commit replaces the cython implementation of the pauli expectation value functions with a multithreaded rust implementation. This was done primarily for two reasons, the first and primary reason for this change is because after #7658 this module was the only cython code left in the qiskit-terra repository so unifying on a single compiled language will reduce the maintanence burden in qiskit-terra. The second reason is similar to the rationale in #7658 around why using rust over cython for multi-threaded hybrid python module. The difference here though is unlike in stochastic swap this module isn't as performance critical as it's not nearly as widely used. * Tune single threaded performance for rust sum This commit tunes the sum for the single threaded path. Using the iterator sum() method is very convienent but for the single threaded path it doesn't create the most efficient output. This was causing a regression in performance over the previous cython version. To address that issue, this commit adds a new tuned function which does a chunked sum which the compiler can handle better. It more closely models how we'd do this with vectorized SIMD instructions. As a future step we can look at using simdeez https://github.com/jackmott/simdeez to further optimize this by doing runtime CPU feature detection and leveraging SIMD instrinsics (we might want to look at using `fast_sum()` in the multithreaded path if we do that too). * Add release notes * Fix lint * Add docstring and signature to rust functions * Define parallel threshold as a constant * Add attribution comment to fast_sum() * Rename eval_parallel_env -> getenv_use_multiple_threads * Use inline literal type for size Co-authored-by: Kevin Hartman <kevin@hart.mn> * Add overflow check on num_qubits The functions only work for at most for number of qubits < usize bits anything larger would cause an overflow. While rust provides overflow checking in debug mode it disables this for performance in release mode. Sice we ship binaries in release mode this commit adds an overflow check for the num_qubits argument to ensure that we don't overflow and produce incorrect results. * Remove unecessary setup_requires field from setup.py The setup_requires field in the setup.py is deprecated and has been superseded by the pyproject.toml to define build system dependencies. Since we're already relying on the pyproject.toml to install setuptools-rust for us having the setup_requires line will do nothing but potentially cause issues as it will use an older install mechanism that will potentially conflict with people's environments. * Drop `.iter().take(LANES)`. * Fix typo. Co-authored-by: Kevin Hartman <kevin@hart.mn>
2022-03-11 04:54:24 +08:00
num-complex = "0.4"
num-bigint = "0.4"
Implement multithreaded stochastic swap in rust (#7658) * Implement multithreaded stochastic swap in rust This commit is a rewrite of the core swap trials functionality in the StochasticSwap transpiler pass. Previously this core routine was written using Cython (see #1789) which had great performance, but that implementation was single threaded. The core of the stochastic swap algorithm by it's nature is well suited to be executed in parallel, it attempts a number of random trials and then picks the best result from all the trials and uses that for that layer. These trials can easily be run in parallel as there is no data dependency between the trials (there are shared inputs but read-only). As the algorithm generally scales exponentially the speed up from running the trials in parallel can offset this and improve the scaling of the pass. Running the pass in parallel was previously tried in #4781 using Python multiprocessing but the overhead of launching an additional process and serializing the input arrays for each trial was significantly larger than the speed gains. To run the algorithm efficiently in parallel multithreading is needed to leverage shared memory on shared inputs. This commit rewrites the cython routine using rust. This was done for two reasons. The first is that rust's safety guarantees make dealing with and writing parallel code much easier and safer. It's also multiplatform because the rust language supports native threading primatives in language. The second is while writing parallel cython code using open-mp there are limitations with it, mainly on windows. In practice it was also difficult to write and maintain parallel cython code as it has very strict requirements on python and c code interactions. It was much faster and easier to port it to rust and the performance for each iteration (outside of parallelism) is the same (in some cases marginally faster) in rust. The implementation here reuses the data structures that the previous cython implementation introduced (mainly flattening all the terra objects into 1d or 2d numpy arrays for efficient access from C). The speedups from this PR can be significant, calling transpile() on a 400 qubit (with a depth of 10) QV model circuit targetting a 409 heavy hex coupling map goes from ~200 seconds with the single threaded cython to ~60 seconds with this PR locally on a 32 core system, When transpiling a 1000 qubit (also with a depth of 10) QV model circuit targetting a 1081 qubit heavy hex coupling map goes from taking ~6500 seconds to ~720 seconds. The tradeoff with this PR is for local qiskit-terra development a rust compiler needs to be installed. This is made trivial using rustup (https://rustup.rs/), but it is an additional burden and one that we might not want to make. If so we can look at turning this PR into a separate repository/package that qiskit-terra can depend on. The tradeoff here is that we'll be adding friction to the api boundary between the pass and the core swap trials interface. But, it does ease the dependency on development for qiskit-terra. * Sanitize packaging to support future modules This commit fixes how we package the compiled rust module in qiskit-terra. As a single rust project only gives us a single compiled binary output we can't use the same scheme we did previously with cython with a separate dynamic lib file for each module. This shifts us to making the rust code build a `qiskit._accelerate` module and in that we have submodules for everything we need from compiled code. For this PR there is only one submodule, `stochastic_swap`, so for example the parallel swap_trials routine can be imported from `qiskit._accelerate.stochastic_swap.swap_trials`. In the future we can have additional submodules for other pieces of compiled code in qiskit. For example, the likely next candidate is the pauli expectation value cython module, which we'll likely port to rust and also make parallel (for sufficiently large number of qubits). In that case we'd add a new submodule for that functionality. * Adjust random normal distribution to use correct mean This commit corrects the use of the normal distribution to have the mean set to 1.0. Previously we were doing this out of band for each value by adding 1 to the random value which wasn't necessary because we could just generate it with a mean of 1.0. * Remove unecessary extra scope from locked read This commit removes an unecessary extra scope around the locked read for where we store the best solution. The scope was previously there to release the lock after we check if there is a solution or not. However this wasn't actually needed as we can just do the check inline and the lock will release after the condition block. * Remove unecessary explicit type from opt_edges variable * Fix indices typo in NLayout constructor Co-authored-by: Jake Lishman <jake@binhbar.com> * Remove explicit lifetime annotation from swap_trials Previously the swap_trials() function had an explicit lifetime annotation `'p` which wasn't necessary because the compiler can determine this on it's own. Normally when dealing with numpy views and a Python object (i.e. a GIL handle) we need a lifetime annotation to tell the rust compiler the numpy view and the python gil handle will have the same lifetime. But since swap_trials doesn't take a gil handle and operates purely in rust we don't need this lifetime and the rust compiler can deal with the lifetime of the numpy views on their own. * Use sum() instead of fold() * Fix lint and add rust style and lint checks to CI This commit fixes the python lint failures and also updates the ci configuration for the lint job to also run rust's style and lint enforcement. * Fix returned layout mapping from NLayout This commit fixes the output list from the `layout_mapping()` method of `NLayout`. Previously, it incorrectly would return the wrong indices it should be a list of virtual -> physical to qubit pairs. This commit corrects this error Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Tweak tox configuration to try and reliably build rust extension * Make swap_trials parallelization configurable This commit makes the parallelization of the swap_trials() configurable. This is dones in two ways, first a new argument parallel_threshold is added which takes an optional int which is the number of qubits to switch between a parallel and serial version. The second is that it takes into account the the state of the QISKIT_IN_PARALLEL environment variable. This variable is set to TRUE by parallel_map() when we're running in a multiprocessing context. In those cases also running stochastic swap in parallel will likely just cause too much load as we're potentially oversubscribing work to the number of available CPUs. So, if QISKIT_IN_PARALLEL is set to True we run swap_trials serially. * Revert "Make swap_trials parallelization configurable" This reverts commit 57790c84b03da10fd7296c57b38b54c5bccebf4c. That commit attempted to sovle some issues in test running, mainly around multiple parallel dispatch causing exceess load. But in practice it was broken and caused more issues than it fixed. We'll investigate and add control for the parallelization in a future commit separately after all the tests are passing so we have a good baseline. * Add docs to swap_trials() and remove unecessary num_gates arg * Fix race condition leading to non-deterministic behavior Previously, in the case of circuits that had multiple best possible depth == 1 solutions for a layer, there was a race condition in the fast exit path between the threads which could lead to a non-deterministic result even with a fixed seed. The output was always valid, but which result was dependent on which parallel thread with an ideal solution finished last and wrote to the locked best result last. This was causing weird non-deterministic test failures for some tests because of #1794 as the exact match result would change between runs. This could be a bigger issue because user expectations are that with a fixed seed set on the transpiler that the output circuit will be deterministically reproducible. To address this is issue this commit trades off some performance to ensure we're always returning a deterministic result in this case. This is accomplished by updating/checking if a depth==1 solution has been found in another trial thread we only act (so either exit early or update the already found depth == 1 solution) if that solution already found has a trial number that is less than this thread's trial number. This does limit the effectiveness of the fast exit, but in practice it should hopefully not effect the speed too much. As part of this commit some tests are updated because the new deterministic behavior is slightly different from the previous results from the cython serial implementation. I manually verified that the new output circuits are still valid (it also looks like the quality of the results in some of those cases improved, but this is strictly anecdotal and shouldn't be taken as a general trend with this PR). * Apply suggestions from code review Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Fix compiler errors in previous commit * Revert accidental commit of parallel reduction in compute_cost This was only a for local testing to prove it was a bad idea and was accidently included in the branch. We should not nest the parallel execution like this. * Eliminate short circuit for depth == 1 swap_trial() result This commit eliminates the short circuit fast return in swap_trial() when another trial thread has found an ideal solution. Trying to do this in a parallel context is tricky to make deterministic because in cases of >1 depth == 1 solutions there is an inherent race condition between the threads for writing out their depth == 1 result to the shared location. Different strategies were tried to make this reliably deterministic but there wa still a race condition. Since this was just a performance optimization to avoid doing unnecessary work this commit removes this step. Weighing improved performance against repeatability in the output of the compiler, the reproducible results are more important. After we've adopted a multithreaded stochastic swap we can investigate adding this back as a potential future optimization. * Add missing docstrings * Add section to contributing on installing form source * Make rust python classes pickleable * Add rust compiler install to linux wheel jobs * Try more tox changes to fix docs builds * Revert "Eliminate short circuit for depth == 1 swap_trial() result" This reverts commit c510764a770cb610661bdb3732337cd45ab587fd. The removal there was premature and we had a fix for the non-determinism in place, ignoring a typo which was preventing it from working. Co-Authored-By: Georgios Tsilimigkounakis <45130028+georgios-ts@users.noreply.github.com> * Fix submodule declaration and module attribute on rust classes * Fix rust lint * Fix docs job definition * Disable multiprocessing parallelism in unit tests This commit disables the multiprocessing based parallelism when running unittest jobs in CI. We historically have defaulted the use of multiprocessing in environments only where the "fork" start method is available because this has the best performance and has no caveats around how it is used by users (you don't need an `if __name__ == "__main__"` guard). However, the use of the "fork" method isn't always 100% reliable (see https://bugs.python.org/issue40379), which we saw on Python 3.9 #6188. In unittest CI (and tox) by default we use stestr which spawns (not using fork) parallel workers to run tests in parallel. With this PR this means in unittest we're now running multiple test runner subprocesses, which are executing parallel dispatched code using multiprocessing's fork start method, which is executing multithreaded rust code. This three layers of nesting is fairly reliably hanging as Python's fork doesn't seem to be able to handle this many layers of nested parallelism. There are 2 ways I've been able to fix this, the first is to change the start method used by `parallel_map()` to either "spawn" or "forkserver" either of these does not suffer from random hanging. However, doing this in the unittest context causes significant overhead and slows down test executing significantly. The other is to just disable the multiprocessing which fixes the hanging and doesn't impact runtime performance signifcantly (and might actually help in CI so we're not oversubscribing the limited resources. As I have not been able to reproduce `parallel_map()` hanging in a standalone context with multithreaded stochastic swap this commit opts for just disabling multiprocessing in CI and documenting the known issue in the release notes as this is the simpler solution. It's unlikely that users will nest parallel processes as it typically hurts performance (and parallel_map() actively guards against it), we only did it in testing previously because the tests which relied on it were a small portion of the test suite (roughly 65 tests) and typically did not have a significant impact on the total throughput of the test suite. * Fix typo in azure pipelines config * Remove unecessary extension compilation for image tests * Add test script to explicitly verify parallel dispatch In an earlier commit we disabled the use of parallel dispatch in parallel_map() to avoid a bug in cpython associated with their fork() based subprocess launch. Doing this works around the bug which was reliably triggered by running multiprocessing in parallel subprocesses. It also has the side benefit of providing a ~2x speed up for test suite execution in CI. However, this meant we lost our test coverage in CI for running parallel_map() with actual multiprocessing based parallel dispatch. To ensure we don't inadvertandtly regress this code path moving forward this commit adds a dedicated test script which runs a simple transpilation in parallel and verifies that everything works as expected with the default parallelism settings. * Avoid multi-threading when run in a multiprocessing context This commit adds a switch on running between a single threaded and a multithreaded variant of the swap_trials loop based on whether the QISKIT_IN_PARALLEL flag is set. If QISKIT_IN_PARALLEL is set to TRUE this means the `parallel_map()` function is running in the outer python context and we're running in multiprocessing already. This means we do not want to be running in multiple threads generally as that will lead to potential resource exhaustion by spawn n processes each potentially running with m threads where `n` is `min(num_phys_cpus, num_tasks)` and `m` is num_logical_cpus (although only `min(num_logical_cpus, num_trials)` will be active) which on the typical system there aren't enough cores to leverage both multiprocessing and multithreading. However, in case a user does have such an environment they can set the `QISKIT_FORCE_THREADS` env variable to `TRUE` which will use threading regardless of the status of `QISKIT_IN_PARALLEL`. * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> * Minor fixes from review comments This commits fixes some minor details found during code review. It expands the section on building from source to explain how to build a release optimized binary with editable mode, makes the QISKIT_PARALLEL env variable usage consistent across all jobs, and adds a missing shebang to the `install_rush.sh` script which is used to install rust in the manylinux container environment. * Simplify tox configuration In earlier commits the tox configuration was changed to try and fix the docs CI job by going to great effort to try and enforce that setuptools-rust was installed in all situations, even before it was actually needed. However, the problem with the docs ci job was unrelated to the tox configuration and this reverts the configuration to something that works with more versions of tox and setuptools-rust. * Add missing pieces of cargo configuration Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2022-03-01 05:49:54 +08:00
[dependencies.pyo3]
version = "0.16.5"
features = ["extension-module", "hashbrown", "num-complex", "num-bigint"]
Implement multithreaded stochastic swap in rust (#7658) * Implement multithreaded stochastic swap in rust This commit is a rewrite of the core swap trials functionality in the StochasticSwap transpiler pass. Previously this core routine was written using Cython (see #1789) which had great performance, but that implementation was single threaded. The core of the stochastic swap algorithm by it's nature is well suited to be executed in parallel, it attempts a number of random trials and then picks the best result from all the trials and uses that for that layer. These trials can easily be run in parallel as there is no data dependency between the trials (there are shared inputs but read-only). As the algorithm generally scales exponentially the speed up from running the trials in parallel can offset this and improve the scaling of the pass. Running the pass in parallel was previously tried in #4781 using Python multiprocessing but the overhead of launching an additional process and serializing the input arrays for each trial was significantly larger than the speed gains. To run the algorithm efficiently in parallel multithreading is needed to leverage shared memory on shared inputs. This commit rewrites the cython routine using rust. This was done for two reasons. The first is that rust's safety guarantees make dealing with and writing parallel code much easier and safer. It's also multiplatform because the rust language supports native threading primatives in language. The second is while writing parallel cython code using open-mp there are limitations with it, mainly on windows. In practice it was also difficult to write and maintain parallel cython code as it has very strict requirements on python and c code interactions. It was much faster and easier to port it to rust and the performance for each iteration (outside of parallelism) is the same (in some cases marginally faster) in rust. The implementation here reuses the data structures that the previous cython implementation introduced (mainly flattening all the terra objects into 1d or 2d numpy arrays for efficient access from C). The speedups from this PR can be significant, calling transpile() on a 400 qubit (with a depth of 10) QV model circuit targetting a 409 heavy hex coupling map goes from ~200 seconds with the single threaded cython to ~60 seconds with this PR locally on a 32 core system, When transpiling a 1000 qubit (also with a depth of 10) QV model circuit targetting a 1081 qubit heavy hex coupling map goes from taking ~6500 seconds to ~720 seconds. The tradeoff with this PR is for local qiskit-terra development a rust compiler needs to be installed. This is made trivial using rustup (https://rustup.rs/), but it is an additional burden and one that we might not want to make. If so we can look at turning this PR into a separate repository/package that qiskit-terra can depend on. The tradeoff here is that we'll be adding friction to the api boundary between the pass and the core swap trials interface. But, it does ease the dependency on development for qiskit-terra. * Sanitize packaging to support future modules This commit fixes how we package the compiled rust module in qiskit-terra. As a single rust project only gives us a single compiled binary output we can't use the same scheme we did previously with cython with a separate dynamic lib file for each module. This shifts us to making the rust code build a `qiskit._accelerate` module and in that we have submodules for everything we need from compiled code. For this PR there is only one submodule, `stochastic_swap`, so for example the parallel swap_trials routine can be imported from `qiskit._accelerate.stochastic_swap.swap_trials`. In the future we can have additional submodules for other pieces of compiled code in qiskit. For example, the likely next candidate is the pauli expectation value cython module, which we'll likely port to rust and also make parallel (for sufficiently large number of qubits). In that case we'd add a new submodule for that functionality. * Adjust random normal distribution to use correct mean This commit corrects the use of the normal distribution to have the mean set to 1.0. Previously we were doing this out of band for each value by adding 1 to the random value which wasn't necessary because we could just generate it with a mean of 1.0. * Remove unecessary extra scope from locked read This commit removes an unecessary extra scope around the locked read for where we store the best solution. The scope was previously there to release the lock after we check if there is a solution or not. However this wasn't actually needed as we can just do the check inline and the lock will release after the condition block. * Remove unecessary explicit type from opt_edges variable * Fix indices typo in NLayout constructor Co-authored-by: Jake Lishman <jake@binhbar.com> * Remove explicit lifetime annotation from swap_trials Previously the swap_trials() function had an explicit lifetime annotation `'p` which wasn't necessary because the compiler can determine this on it's own. Normally when dealing with numpy views and a Python object (i.e. a GIL handle) we need a lifetime annotation to tell the rust compiler the numpy view and the python gil handle will have the same lifetime. But since swap_trials doesn't take a gil handle and operates purely in rust we don't need this lifetime and the rust compiler can deal with the lifetime of the numpy views on their own. * Use sum() instead of fold() * Fix lint and add rust style and lint checks to CI This commit fixes the python lint failures and also updates the ci configuration for the lint job to also run rust's style and lint enforcement. * Fix returned layout mapping from NLayout This commit fixes the output list from the `layout_mapping()` method of `NLayout`. Previously, it incorrectly would return the wrong indices it should be a list of virtual -> physical to qubit pairs. This commit corrects this error Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Tweak tox configuration to try and reliably build rust extension * Make swap_trials parallelization configurable This commit makes the parallelization of the swap_trials() configurable. This is dones in two ways, first a new argument parallel_threshold is added which takes an optional int which is the number of qubits to switch between a parallel and serial version. The second is that it takes into account the the state of the QISKIT_IN_PARALLEL environment variable. This variable is set to TRUE by parallel_map() when we're running in a multiprocessing context. In those cases also running stochastic swap in parallel will likely just cause too much load as we're potentially oversubscribing work to the number of available CPUs. So, if QISKIT_IN_PARALLEL is set to True we run swap_trials serially. * Revert "Make swap_trials parallelization configurable" This reverts commit 57790c84b03da10fd7296c57b38b54c5bccebf4c. That commit attempted to sovle some issues in test running, mainly around multiple parallel dispatch causing exceess load. But in practice it was broken and caused more issues than it fixed. We'll investigate and add control for the parallelization in a future commit separately after all the tests are passing so we have a good baseline. * Add docs to swap_trials() and remove unecessary num_gates arg * Fix race condition leading to non-deterministic behavior Previously, in the case of circuits that had multiple best possible depth == 1 solutions for a layer, there was a race condition in the fast exit path between the threads which could lead to a non-deterministic result even with a fixed seed. The output was always valid, but which result was dependent on which parallel thread with an ideal solution finished last and wrote to the locked best result last. This was causing weird non-deterministic test failures for some tests because of #1794 as the exact match result would change between runs. This could be a bigger issue because user expectations are that with a fixed seed set on the transpiler that the output circuit will be deterministically reproducible. To address this is issue this commit trades off some performance to ensure we're always returning a deterministic result in this case. This is accomplished by updating/checking if a depth==1 solution has been found in another trial thread we only act (so either exit early or update the already found depth == 1 solution) if that solution already found has a trial number that is less than this thread's trial number. This does limit the effectiveness of the fast exit, but in practice it should hopefully not effect the speed too much. As part of this commit some tests are updated because the new deterministic behavior is slightly different from the previous results from the cython serial implementation. I manually verified that the new output circuits are still valid (it also looks like the quality of the results in some of those cases improved, but this is strictly anecdotal and shouldn't be taken as a general trend with this PR). * Apply suggestions from code review Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Fix compiler errors in previous commit * Revert accidental commit of parallel reduction in compute_cost This was only a for local testing to prove it was a bad idea and was accidently included in the branch. We should not nest the parallel execution like this. * Eliminate short circuit for depth == 1 swap_trial() result This commit eliminates the short circuit fast return in swap_trial() when another trial thread has found an ideal solution. Trying to do this in a parallel context is tricky to make deterministic because in cases of >1 depth == 1 solutions there is an inherent race condition between the threads for writing out their depth == 1 result to the shared location. Different strategies were tried to make this reliably deterministic but there wa still a race condition. Since this was just a performance optimization to avoid doing unnecessary work this commit removes this step. Weighing improved performance against repeatability in the output of the compiler, the reproducible results are more important. After we've adopted a multithreaded stochastic swap we can investigate adding this back as a potential future optimization. * Add missing docstrings * Add section to contributing on installing form source * Make rust python classes pickleable * Add rust compiler install to linux wheel jobs * Try more tox changes to fix docs builds * Revert "Eliminate short circuit for depth == 1 swap_trial() result" This reverts commit c510764a770cb610661bdb3732337cd45ab587fd. The removal there was premature and we had a fix for the non-determinism in place, ignoring a typo which was preventing it from working. Co-Authored-By: Georgios Tsilimigkounakis <45130028+georgios-ts@users.noreply.github.com> * Fix submodule declaration and module attribute on rust classes * Fix rust lint * Fix docs job definition * Disable multiprocessing parallelism in unit tests This commit disables the multiprocessing based parallelism when running unittest jobs in CI. We historically have defaulted the use of multiprocessing in environments only where the "fork" start method is available because this has the best performance and has no caveats around how it is used by users (you don't need an `if __name__ == "__main__"` guard). However, the use of the "fork" method isn't always 100% reliable (see https://bugs.python.org/issue40379), which we saw on Python 3.9 #6188. In unittest CI (and tox) by default we use stestr which spawns (not using fork) parallel workers to run tests in parallel. With this PR this means in unittest we're now running multiple test runner subprocesses, which are executing parallel dispatched code using multiprocessing's fork start method, which is executing multithreaded rust code. This three layers of nesting is fairly reliably hanging as Python's fork doesn't seem to be able to handle this many layers of nested parallelism. There are 2 ways I've been able to fix this, the first is to change the start method used by `parallel_map()` to either "spawn" or "forkserver" either of these does not suffer from random hanging. However, doing this in the unittest context causes significant overhead and slows down test executing significantly. The other is to just disable the multiprocessing which fixes the hanging and doesn't impact runtime performance signifcantly (and might actually help in CI so we're not oversubscribing the limited resources. As I have not been able to reproduce `parallel_map()` hanging in a standalone context with multithreaded stochastic swap this commit opts for just disabling multiprocessing in CI and documenting the known issue in the release notes as this is the simpler solution. It's unlikely that users will nest parallel processes as it typically hurts performance (and parallel_map() actively guards against it), we only did it in testing previously because the tests which relied on it were a small portion of the test suite (roughly 65 tests) and typically did not have a significant impact on the total throughput of the test suite. * Fix typo in azure pipelines config * Remove unecessary extension compilation for image tests * Add test script to explicitly verify parallel dispatch In an earlier commit we disabled the use of parallel dispatch in parallel_map() to avoid a bug in cpython associated with their fork() based subprocess launch. Doing this works around the bug which was reliably triggered by running multiprocessing in parallel subprocesses. It also has the side benefit of providing a ~2x speed up for test suite execution in CI. However, this meant we lost our test coverage in CI for running parallel_map() with actual multiprocessing based parallel dispatch. To ensure we don't inadvertandtly regress this code path moving forward this commit adds a dedicated test script which runs a simple transpilation in parallel and verifies that everything works as expected with the default parallelism settings. * Avoid multi-threading when run in a multiprocessing context This commit adds a switch on running between a single threaded and a multithreaded variant of the swap_trials loop based on whether the QISKIT_IN_PARALLEL flag is set. If QISKIT_IN_PARALLEL is set to TRUE this means the `parallel_map()` function is running in the outer python context and we're running in multiprocessing already. This means we do not want to be running in multiple threads generally as that will lead to potential resource exhaustion by spawn n processes each potentially running with m threads where `n` is `min(num_phys_cpus, num_tasks)` and `m` is num_logical_cpus (although only `min(num_logical_cpus, num_trials)` will be active) which on the typical system there aren't enough cores to leverage both multiprocessing and multithreading. However, in case a user does have such an environment they can set the `QISKIT_FORCE_THREADS` env variable to `TRUE` which will use threading regardless of the status of `QISKIT_IN_PARALLEL`. * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> * Minor fixes from review comments This commits fixes some minor details found during code review. It expands the section on building from source to explain how to build a release optimized binary with editable mode, makes the QISKIT_PARALLEL env variable usage consistent across all jobs, and adds a missing shebang to the `install_rush.sh` script which is used to install rust in the manylinux container environment. * Simplify tox configuration In earlier commits the tox configuration was changed to try and fix the docs CI job by going to great effort to try and enforce that setuptools-rust was installed in all situations, even before it was actually needed. However, the problem with the docs ci job was unrelated to the tox configuration and this reverts the configuration to something that works with more versions of tox and setuptools-rust. * Add missing pieces of cargo configuration Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2022-03-01 05:49:54 +08:00
[dependencies.ndarray]
version = "^0.15.6"
Implement multithreaded stochastic swap in rust (#7658) * Implement multithreaded stochastic swap in rust This commit is a rewrite of the core swap trials functionality in the StochasticSwap transpiler pass. Previously this core routine was written using Cython (see #1789) which had great performance, but that implementation was single threaded. The core of the stochastic swap algorithm by it's nature is well suited to be executed in parallel, it attempts a number of random trials and then picks the best result from all the trials and uses that for that layer. These trials can easily be run in parallel as there is no data dependency between the trials (there are shared inputs but read-only). As the algorithm generally scales exponentially the speed up from running the trials in parallel can offset this and improve the scaling of the pass. Running the pass in parallel was previously tried in #4781 using Python multiprocessing but the overhead of launching an additional process and serializing the input arrays for each trial was significantly larger than the speed gains. To run the algorithm efficiently in parallel multithreading is needed to leverage shared memory on shared inputs. This commit rewrites the cython routine using rust. This was done for two reasons. The first is that rust's safety guarantees make dealing with and writing parallel code much easier and safer. It's also multiplatform because the rust language supports native threading primatives in language. The second is while writing parallel cython code using open-mp there are limitations with it, mainly on windows. In practice it was also difficult to write and maintain parallel cython code as it has very strict requirements on python and c code interactions. It was much faster and easier to port it to rust and the performance for each iteration (outside of parallelism) is the same (in some cases marginally faster) in rust. The implementation here reuses the data structures that the previous cython implementation introduced (mainly flattening all the terra objects into 1d or 2d numpy arrays for efficient access from C). The speedups from this PR can be significant, calling transpile() on a 400 qubit (with a depth of 10) QV model circuit targetting a 409 heavy hex coupling map goes from ~200 seconds with the single threaded cython to ~60 seconds with this PR locally on a 32 core system, When transpiling a 1000 qubit (also with a depth of 10) QV model circuit targetting a 1081 qubit heavy hex coupling map goes from taking ~6500 seconds to ~720 seconds. The tradeoff with this PR is for local qiskit-terra development a rust compiler needs to be installed. This is made trivial using rustup (https://rustup.rs/), but it is an additional burden and one that we might not want to make. If so we can look at turning this PR into a separate repository/package that qiskit-terra can depend on. The tradeoff here is that we'll be adding friction to the api boundary between the pass and the core swap trials interface. But, it does ease the dependency on development for qiskit-terra. * Sanitize packaging to support future modules This commit fixes how we package the compiled rust module in qiskit-terra. As a single rust project only gives us a single compiled binary output we can't use the same scheme we did previously with cython with a separate dynamic lib file for each module. This shifts us to making the rust code build a `qiskit._accelerate` module and in that we have submodules for everything we need from compiled code. For this PR there is only one submodule, `stochastic_swap`, so for example the parallel swap_trials routine can be imported from `qiskit._accelerate.stochastic_swap.swap_trials`. In the future we can have additional submodules for other pieces of compiled code in qiskit. For example, the likely next candidate is the pauli expectation value cython module, which we'll likely port to rust and also make parallel (for sufficiently large number of qubits). In that case we'd add a new submodule for that functionality. * Adjust random normal distribution to use correct mean This commit corrects the use of the normal distribution to have the mean set to 1.0. Previously we were doing this out of band for each value by adding 1 to the random value which wasn't necessary because we could just generate it with a mean of 1.0. * Remove unecessary extra scope from locked read This commit removes an unecessary extra scope around the locked read for where we store the best solution. The scope was previously there to release the lock after we check if there is a solution or not. However this wasn't actually needed as we can just do the check inline and the lock will release after the condition block. * Remove unecessary explicit type from opt_edges variable * Fix indices typo in NLayout constructor Co-authored-by: Jake Lishman <jake@binhbar.com> * Remove explicit lifetime annotation from swap_trials Previously the swap_trials() function had an explicit lifetime annotation `'p` which wasn't necessary because the compiler can determine this on it's own. Normally when dealing with numpy views and a Python object (i.e. a GIL handle) we need a lifetime annotation to tell the rust compiler the numpy view and the python gil handle will have the same lifetime. But since swap_trials doesn't take a gil handle and operates purely in rust we don't need this lifetime and the rust compiler can deal with the lifetime of the numpy views on their own. * Use sum() instead of fold() * Fix lint and add rust style and lint checks to CI This commit fixes the python lint failures and also updates the ci configuration for the lint job to also run rust's style and lint enforcement. * Fix returned layout mapping from NLayout This commit fixes the output list from the `layout_mapping()` method of `NLayout`. Previously, it incorrectly would return the wrong indices it should be a list of virtual -> physical to qubit pairs. This commit corrects this error Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Tweak tox configuration to try and reliably build rust extension * Make swap_trials parallelization configurable This commit makes the parallelization of the swap_trials() configurable. This is dones in two ways, first a new argument parallel_threshold is added which takes an optional int which is the number of qubits to switch between a parallel and serial version. The second is that it takes into account the the state of the QISKIT_IN_PARALLEL environment variable. This variable is set to TRUE by parallel_map() when we're running in a multiprocessing context. In those cases also running stochastic swap in parallel will likely just cause too much load as we're potentially oversubscribing work to the number of available CPUs. So, if QISKIT_IN_PARALLEL is set to True we run swap_trials serially. * Revert "Make swap_trials parallelization configurable" This reverts commit 57790c84b03da10fd7296c57b38b54c5bccebf4c. That commit attempted to sovle some issues in test running, mainly around multiple parallel dispatch causing exceess load. But in practice it was broken and caused more issues than it fixed. We'll investigate and add control for the parallelization in a future commit separately after all the tests are passing so we have a good baseline. * Add docs to swap_trials() and remove unecessary num_gates arg * Fix race condition leading to non-deterministic behavior Previously, in the case of circuits that had multiple best possible depth == 1 solutions for a layer, there was a race condition in the fast exit path between the threads which could lead to a non-deterministic result even with a fixed seed. The output was always valid, but which result was dependent on which parallel thread with an ideal solution finished last and wrote to the locked best result last. This was causing weird non-deterministic test failures for some tests because of #1794 as the exact match result would change between runs. This could be a bigger issue because user expectations are that with a fixed seed set on the transpiler that the output circuit will be deterministically reproducible. To address this is issue this commit trades off some performance to ensure we're always returning a deterministic result in this case. This is accomplished by updating/checking if a depth==1 solution has been found in another trial thread we only act (so either exit early or update the already found depth == 1 solution) if that solution already found has a trial number that is less than this thread's trial number. This does limit the effectiveness of the fast exit, but in practice it should hopefully not effect the speed too much. As part of this commit some tests are updated because the new deterministic behavior is slightly different from the previous results from the cython serial implementation. I manually verified that the new output circuits are still valid (it also looks like the quality of the results in some of those cases improved, but this is strictly anecdotal and shouldn't be taken as a general trend with this PR). * Apply suggestions from code review Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Fix compiler errors in previous commit * Revert accidental commit of parallel reduction in compute_cost This was only a for local testing to prove it was a bad idea and was accidently included in the branch. We should not nest the parallel execution like this. * Eliminate short circuit for depth == 1 swap_trial() result This commit eliminates the short circuit fast return in swap_trial() when another trial thread has found an ideal solution. Trying to do this in a parallel context is tricky to make deterministic because in cases of >1 depth == 1 solutions there is an inherent race condition between the threads for writing out their depth == 1 result to the shared location. Different strategies were tried to make this reliably deterministic but there wa still a race condition. Since this was just a performance optimization to avoid doing unnecessary work this commit removes this step. Weighing improved performance against repeatability in the output of the compiler, the reproducible results are more important. After we've adopted a multithreaded stochastic swap we can investigate adding this back as a potential future optimization. * Add missing docstrings * Add section to contributing on installing form source * Make rust python classes pickleable * Add rust compiler install to linux wheel jobs * Try more tox changes to fix docs builds * Revert "Eliminate short circuit for depth == 1 swap_trial() result" This reverts commit c510764a770cb610661bdb3732337cd45ab587fd. The removal there was premature and we had a fix for the non-determinism in place, ignoring a typo which was preventing it from working. Co-Authored-By: Georgios Tsilimigkounakis <45130028+georgios-ts@users.noreply.github.com> * Fix submodule declaration and module attribute on rust classes * Fix rust lint * Fix docs job definition * Disable multiprocessing parallelism in unit tests This commit disables the multiprocessing based parallelism when running unittest jobs in CI. We historically have defaulted the use of multiprocessing in environments only where the "fork" start method is available because this has the best performance and has no caveats around how it is used by users (you don't need an `if __name__ == "__main__"` guard). However, the use of the "fork" method isn't always 100% reliable (see https://bugs.python.org/issue40379), which we saw on Python 3.9 #6188. In unittest CI (and tox) by default we use stestr which spawns (not using fork) parallel workers to run tests in parallel. With this PR this means in unittest we're now running multiple test runner subprocesses, which are executing parallel dispatched code using multiprocessing's fork start method, which is executing multithreaded rust code. This three layers of nesting is fairly reliably hanging as Python's fork doesn't seem to be able to handle this many layers of nested parallelism. There are 2 ways I've been able to fix this, the first is to change the start method used by `parallel_map()` to either "spawn" or "forkserver" either of these does not suffer from random hanging. However, doing this in the unittest context causes significant overhead and slows down test executing significantly. The other is to just disable the multiprocessing which fixes the hanging and doesn't impact runtime performance signifcantly (and might actually help in CI so we're not oversubscribing the limited resources. As I have not been able to reproduce `parallel_map()` hanging in a standalone context with multithreaded stochastic swap this commit opts for just disabling multiprocessing in CI and documenting the known issue in the release notes as this is the simpler solution. It's unlikely that users will nest parallel processes as it typically hurts performance (and parallel_map() actively guards against it), we only did it in testing previously because the tests which relied on it were a small portion of the test suite (roughly 65 tests) and typically did not have a significant impact on the total throughput of the test suite. * Fix typo in azure pipelines config * Remove unecessary extension compilation for image tests * Add test script to explicitly verify parallel dispatch In an earlier commit we disabled the use of parallel dispatch in parallel_map() to avoid a bug in cpython associated with their fork() based subprocess launch. Doing this works around the bug which was reliably triggered by running multiprocessing in parallel subprocesses. It also has the side benefit of providing a ~2x speed up for test suite execution in CI. However, this meant we lost our test coverage in CI for running parallel_map() with actual multiprocessing based parallel dispatch. To ensure we don't inadvertandtly regress this code path moving forward this commit adds a dedicated test script which runs a simple transpilation in parallel and verifies that everything works as expected with the default parallelism settings. * Avoid multi-threading when run in a multiprocessing context This commit adds a switch on running between a single threaded and a multithreaded variant of the swap_trials loop based on whether the QISKIT_IN_PARALLEL flag is set. If QISKIT_IN_PARALLEL is set to TRUE this means the `parallel_map()` function is running in the outer python context and we're running in multiprocessing already. This means we do not want to be running in multiple threads generally as that will lead to potential resource exhaustion by spawn n processes each potentially running with m threads where `n` is `min(num_phys_cpus, num_tasks)` and `m` is num_logical_cpus (although only `min(num_logical_cpus, num_trials)` will be active) which on the typical system there aren't enough cores to leverage both multiprocessing and multithreading. However, in case a user does have such an environment they can set the `QISKIT_FORCE_THREADS` env variable to `TRUE` which will use threading regardless of the status of `QISKIT_IN_PARALLEL`. * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> * Minor fixes from review comments This commits fixes some minor details found during code review. It expands the section on building from source to explain how to build a release optimized binary with editable mode, makes the QISKIT_PARALLEL env variable usage consistent across all jobs, and adds a missing shebang to the `install_rush.sh` script which is used to install rust in the manylinux container environment. * Simplify tox configuration In earlier commits the tox configuration was changed to try and fix the docs CI job by going to great effort to try and enforce that setuptools-rust was installed in all situations, even before it was actually needed. However, the problem with the docs ci job was unrelated to the tox configuration and this reverts the configuration to something that works with more versions of tox and setuptools-rust. * Add missing pieces of cargo configuration Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2022-03-01 05:49:54 +08:00
features = ["rayon"]
[dependencies.hashbrown]
version = "0.12.3"
Implement multithreaded stochastic swap in rust (#7658) * Implement multithreaded stochastic swap in rust This commit is a rewrite of the core swap trials functionality in the StochasticSwap transpiler pass. Previously this core routine was written using Cython (see #1789) which had great performance, but that implementation was single threaded. The core of the stochastic swap algorithm by it's nature is well suited to be executed in parallel, it attempts a number of random trials and then picks the best result from all the trials and uses that for that layer. These trials can easily be run in parallel as there is no data dependency between the trials (there are shared inputs but read-only). As the algorithm generally scales exponentially the speed up from running the trials in parallel can offset this and improve the scaling of the pass. Running the pass in parallel was previously tried in #4781 using Python multiprocessing but the overhead of launching an additional process and serializing the input arrays for each trial was significantly larger than the speed gains. To run the algorithm efficiently in parallel multithreading is needed to leverage shared memory on shared inputs. This commit rewrites the cython routine using rust. This was done for two reasons. The first is that rust's safety guarantees make dealing with and writing parallel code much easier and safer. It's also multiplatform because the rust language supports native threading primatives in language. The second is while writing parallel cython code using open-mp there are limitations with it, mainly on windows. In practice it was also difficult to write and maintain parallel cython code as it has very strict requirements on python and c code interactions. It was much faster and easier to port it to rust and the performance for each iteration (outside of parallelism) is the same (in some cases marginally faster) in rust. The implementation here reuses the data structures that the previous cython implementation introduced (mainly flattening all the terra objects into 1d or 2d numpy arrays for efficient access from C). The speedups from this PR can be significant, calling transpile() on a 400 qubit (with a depth of 10) QV model circuit targetting a 409 heavy hex coupling map goes from ~200 seconds with the single threaded cython to ~60 seconds with this PR locally on a 32 core system, When transpiling a 1000 qubit (also with a depth of 10) QV model circuit targetting a 1081 qubit heavy hex coupling map goes from taking ~6500 seconds to ~720 seconds. The tradeoff with this PR is for local qiskit-terra development a rust compiler needs to be installed. This is made trivial using rustup (https://rustup.rs/), but it is an additional burden and one that we might not want to make. If so we can look at turning this PR into a separate repository/package that qiskit-terra can depend on. The tradeoff here is that we'll be adding friction to the api boundary between the pass and the core swap trials interface. But, it does ease the dependency on development for qiskit-terra. * Sanitize packaging to support future modules This commit fixes how we package the compiled rust module in qiskit-terra. As a single rust project only gives us a single compiled binary output we can't use the same scheme we did previously with cython with a separate dynamic lib file for each module. This shifts us to making the rust code build a `qiskit._accelerate` module and in that we have submodules for everything we need from compiled code. For this PR there is only one submodule, `stochastic_swap`, so for example the parallel swap_trials routine can be imported from `qiskit._accelerate.stochastic_swap.swap_trials`. In the future we can have additional submodules for other pieces of compiled code in qiskit. For example, the likely next candidate is the pauli expectation value cython module, which we'll likely port to rust and also make parallel (for sufficiently large number of qubits). In that case we'd add a new submodule for that functionality. * Adjust random normal distribution to use correct mean This commit corrects the use of the normal distribution to have the mean set to 1.0. Previously we were doing this out of band for each value by adding 1 to the random value which wasn't necessary because we could just generate it with a mean of 1.0. * Remove unecessary extra scope from locked read This commit removes an unecessary extra scope around the locked read for where we store the best solution. The scope was previously there to release the lock after we check if there is a solution or not. However this wasn't actually needed as we can just do the check inline and the lock will release after the condition block. * Remove unecessary explicit type from opt_edges variable * Fix indices typo in NLayout constructor Co-authored-by: Jake Lishman <jake@binhbar.com> * Remove explicit lifetime annotation from swap_trials Previously the swap_trials() function had an explicit lifetime annotation `'p` which wasn't necessary because the compiler can determine this on it's own. Normally when dealing with numpy views and a Python object (i.e. a GIL handle) we need a lifetime annotation to tell the rust compiler the numpy view and the python gil handle will have the same lifetime. But since swap_trials doesn't take a gil handle and operates purely in rust we don't need this lifetime and the rust compiler can deal with the lifetime of the numpy views on their own. * Use sum() instead of fold() * Fix lint and add rust style and lint checks to CI This commit fixes the python lint failures and also updates the ci configuration for the lint job to also run rust's style and lint enforcement. * Fix returned layout mapping from NLayout This commit fixes the output list from the `layout_mapping()` method of `NLayout`. Previously, it incorrectly would return the wrong indices it should be a list of virtual -> physical to qubit pairs. This commit corrects this error Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Tweak tox configuration to try and reliably build rust extension * Make swap_trials parallelization configurable This commit makes the parallelization of the swap_trials() configurable. This is dones in two ways, first a new argument parallel_threshold is added which takes an optional int which is the number of qubits to switch between a parallel and serial version. The second is that it takes into account the the state of the QISKIT_IN_PARALLEL environment variable. This variable is set to TRUE by parallel_map() when we're running in a multiprocessing context. In those cases also running stochastic swap in parallel will likely just cause too much load as we're potentially oversubscribing work to the number of available CPUs. So, if QISKIT_IN_PARALLEL is set to True we run swap_trials serially. * Revert "Make swap_trials parallelization configurable" This reverts commit 57790c84b03da10fd7296c57b38b54c5bccebf4c. That commit attempted to sovle some issues in test running, mainly around multiple parallel dispatch causing exceess load. But in practice it was broken and caused more issues than it fixed. We'll investigate and add control for the parallelization in a future commit separately after all the tests are passing so we have a good baseline. * Add docs to swap_trials() and remove unecessary num_gates arg * Fix race condition leading to non-deterministic behavior Previously, in the case of circuits that had multiple best possible depth == 1 solutions for a layer, there was a race condition in the fast exit path between the threads which could lead to a non-deterministic result even with a fixed seed. The output was always valid, but which result was dependent on which parallel thread with an ideal solution finished last and wrote to the locked best result last. This was causing weird non-deterministic test failures for some tests because of #1794 as the exact match result would change between runs. This could be a bigger issue because user expectations are that with a fixed seed set on the transpiler that the output circuit will be deterministically reproducible. To address this is issue this commit trades off some performance to ensure we're always returning a deterministic result in this case. This is accomplished by updating/checking if a depth==1 solution has been found in another trial thread we only act (so either exit early or update the already found depth == 1 solution) if that solution already found has a trial number that is less than this thread's trial number. This does limit the effectiveness of the fast exit, but in practice it should hopefully not effect the speed too much. As part of this commit some tests are updated because the new deterministic behavior is slightly different from the previous results from the cython serial implementation. I manually verified that the new output circuits are still valid (it also looks like the quality of the results in some of those cases improved, but this is strictly anecdotal and shouldn't be taken as a general trend with this PR). * Apply suggestions from code review Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Fix compiler errors in previous commit * Revert accidental commit of parallel reduction in compute_cost This was only a for local testing to prove it was a bad idea and was accidently included in the branch. We should not nest the parallel execution like this. * Eliminate short circuit for depth == 1 swap_trial() result This commit eliminates the short circuit fast return in swap_trial() when another trial thread has found an ideal solution. Trying to do this in a parallel context is tricky to make deterministic because in cases of >1 depth == 1 solutions there is an inherent race condition between the threads for writing out their depth == 1 result to the shared location. Different strategies were tried to make this reliably deterministic but there wa still a race condition. Since this was just a performance optimization to avoid doing unnecessary work this commit removes this step. Weighing improved performance against repeatability in the output of the compiler, the reproducible results are more important. After we've adopted a multithreaded stochastic swap we can investigate adding this back as a potential future optimization. * Add missing docstrings * Add section to contributing on installing form source * Make rust python classes pickleable * Add rust compiler install to linux wheel jobs * Try more tox changes to fix docs builds * Revert "Eliminate short circuit for depth == 1 swap_trial() result" This reverts commit c510764a770cb610661bdb3732337cd45ab587fd. The removal there was premature and we had a fix for the non-determinism in place, ignoring a typo which was preventing it from working. Co-Authored-By: Georgios Tsilimigkounakis <45130028+georgios-ts@users.noreply.github.com> * Fix submodule declaration and module attribute on rust classes * Fix rust lint * Fix docs job definition * Disable multiprocessing parallelism in unit tests This commit disables the multiprocessing based parallelism when running unittest jobs in CI. We historically have defaulted the use of multiprocessing in environments only where the "fork" start method is available because this has the best performance and has no caveats around how it is used by users (you don't need an `if __name__ == "__main__"` guard). However, the use of the "fork" method isn't always 100% reliable (see https://bugs.python.org/issue40379), which we saw on Python 3.9 #6188. In unittest CI (and tox) by default we use stestr which spawns (not using fork) parallel workers to run tests in parallel. With this PR this means in unittest we're now running multiple test runner subprocesses, which are executing parallel dispatched code using multiprocessing's fork start method, which is executing multithreaded rust code. This three layers of nesting is fairly reliably hanging as Python's fork doesn't seem to be able to handle this many layers of nested parallelism. There are 2 ways I've been able to fix this, the first is to change the start method used by `parallel_map()` to either "spawn" or "forkserver" either of these does not suffer from random hanging. However, doing this in the unittest context causes significant overhead and slows down test executing significantly. The other is to just disable the multiprocessing which fixes the hanging and doesn't impact runtime performance signifcantly (and might actually help in CI so we're not oversubscribing the limited resources. As I have not been able to reproduce `parallel_map()` hanging in a standalone context with multithreaded stochastic swap this commit opts for just disabling multiprocessing in CI and documenting the known issue in the release notes as this is the simpler solution. It's unlikely that users will nest parallel processes as it typically hurts performance (and parallel_map() actively guards against it), we only did it in testing previously because the tests which relied on it were a small portion of the test suite (roughly 65 tests) and typically did not have a significant impact on the total throughput of the test suite. * Fix typo in azure pipelines config * Remove unecessary extension compilation for image tests * Add test script to explicitly verify parallel dispatch In an earlier commit we disabled the use of parallel dispatch in parallel_map() to avoid a bug in cpython associated with their fork() based subprocess launch. Doing this works around the bug which was reliably triggered by running multiprocessing in parallel subprocesses. It also has the side benefit of providing a ~2x speed up for test suite execution in CI. However, this meant we lost our test coverage in CI for running parallel_map() with actual multiprocessing based parallel dispatch. To ensure we don't inadvertandtly regress this code path moving forward this commit adds a dedicated test script which runs a simple transpilation in parallel and verifies that everything works as expected with the default parallelism settings. * Avoid multi-threading when run in a multiprocessing context This commit adds a switch on running between a single threaded and a multithreaded variant of the swap_trials loop based on whether the QISKIT_IN_PARALLEL flag is set. If QISKIT_IN_PARALLEL is set to TRUE this means the `parallel_map()` function is running in the outer python context and we're running in multiprocessing already. This means we do not want to be running in multiple threads generally as that will lead to potential resource exhaustion by spawn n processes each potentially running with m threads where `n` is `min(num_phys_cpus, num_tasks)` and `m` is num_logical_cpus (although only `min(num_logical_cpus, num_trials)` will be active) which on the typical system there aren't enough cores to leverage both multiprocessing and multithreading. However, in case a user does have such an environment they can set the `QISKIT_FORCE_THREADS` env variable to `TRUE` which will use threading regardless of the status of `QISKIT_IN_PARALLEL`. * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> * Minor fixes from review comments This commits fixes some minor details found during code review. It expands the section on building from source to explain how to build a release optimized binary with editable mode, makes the QISKIT_PARALLEL env variable usage consistent across all jobs, and adds a missing shebang to the `install_rush.sh` script which is used to install rust in the manylinux container environment. * Simplify tox configuration In earlier commits the tox configuration was changed to try and fix the docs CI job by going to great effort to try and enforce that setuptools-rust was installed in all situations, even before it was actually needed. However, the problem with the docs ci job was unrelated to the tox configuration and this reverts the configuration to something that works with more versions of tox and setuptools-rust. * Add missing pieces of cargo configuration Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2022-03-01 05:49:54 +08:00
features = ["rayon"]
Reimplement SabreSwap heuristic scoring in Rust (#7977) * Reimplement SabreSwap heuristic scoring in multithreaded Rust This commit re-implements the core heuristic scoring of swap candidates in the SabreSwap pass as a multithread Rust routine. The heuristic scoring in sabre previously looped over all potential swap candidates serially in Python and applied a computed a heuristic score on which to candidate to pick. This can easily be done in parallel as there is no data dependency between scoring the different candidates. By performing this in Rust not only is the scoring operation done more quickly for each candidate but we can also leverage multithreading to do this efficiently in parallel. * Make sabre_swap a separate Rust module This commit moves the sabre specific code into a separate rust module. We already were using a separate Python module for the sabre code this just mirrors that in the rust code for better organization. * Fix lint * Remove unnecessary parallel iteration This commit removes an unecessary parallel iterator over the swap scores to find the minimum and just does it serially. The threading overhead for the parallel iterator is unecessary as it is fairly quick. * Revert change to DECAY_RESET_INTERVAL behavior * Avoid Bit._index * Add __str__ definition for DEBUG logs * Cleanup greedy swap path * Preserve insertion order in SwapScores The use of an inner hashmap meant the swap candidates were being evaluated in a different order based on the hash seeding instead of the order generated from the python side. This commit fixes by switching the internal type to an IndexMap which for a little overhead preserves the insertion order on iteration. * Work with virtual indices win obtain swap * Simplify decay reset() method * Fix lint * Fix typo * Rename nlayout methods * Update docstrings for SwapScores type * Use correct swap method for _undo_operations() * Fix rebase error * Revert test change * Reverse if condition in lookahead cost * Fix missing len division on lookahead cost * Remove unused EXTENDED_SET_WEIGHT python global * Switch to serial iterator for heuristic scoring While the heuristic scoring can be done in parallel as there is no data dependency between computing the score for candidates the overhead of dealing with multithreading eliminates and benefits from parallel execution. This is because the relative computation is fairly quick and the number of candidates is never very large (since coupling maps are typically sparsely connected). This commit switches to a serial iterator which will speed up execution in practice over running the iteration in parallel. * Return a 2d numpy array for best swaps and avoid conversion cost * Migrate obtain_swaps to rust This commit simplifies the rust loop by avoiding the need to have a mutable shared swap scores between rust and python. Instead the obtain swaps function to get the swap candidates for each layer is migrated to rust using a new neighbors table which is computed once per sabre class. This moves the iteration from obtain swaps to rust and eliminates it as a bottleneck. * Remove unused SwapScores class * Fix module metadata path * Add release note * Add rust docstrings * Pre-allocate candidate_swaps * Double swap instead of clone * Remove unnecessary list comprehensions * Move random choice into rust After rewriting the heuristic scoring in rust the biggest bottleneck in the function (outside of computing the extended set and applying gates to the dag) was performing the random choice between the best candidates via numpy. This wasn't necessary since we can just do the random choice in rust and have it return the best candidate. This commit adds a new class to represent a shared rng that is reused on each scoring call and changes sabre_score_heuristic to return the best swap. The tradeoff with this PR is that it changes the seeding so when compared to previous versions of SabreSwap different results will be returned with the same seed value. * Use int32 for max default rng seed for windows compat * Fix bounds check on custom sequence type's __getitem__ Co-authored-by: Kevin Hartman <kevin@hart.mn> * Only run parallel sort if not in a parallel context This commit updates the sort step in the sabre algorithm to only run a parallel sort if we're not already in a parallel context. This is to prevent a potential over dispatch of work if we're trying to use multiple threads from multiple processes. At the same time the sort algorithm used is switched to the unstable variant because a stable sort isn't necessary for this application and an unstable sort has less overhead. Co-authored-by: Kevin Hartman <kevin@hart.mn>
2022-07-19 23:34:38 +08:00
[dependencies.indexmap]
version = "1.9.1"
features = ["rayon"]
Implement multithreaded stochastic swap in rust (#7658) * Implement multithreaded stochastic swap in rust This commit is a rewrite of the core swap trials functionality in the StochasticSwap transpiler pass. Previously this core routine was written using Cython (see #1789) which had great performance, but that implementation was single threaded. The core of the stochastic swap algorithm by it's nature is well suited to be executed in parallel, it attempts a number of random trials and then picks the best result from all the trials and uses that for that layer. These trials can easily be run in parallel as there is no data dependency between the trials (there are shared inputs but read-only). As the algorithm generally scales exponentially the speed up from running the trials in parallel can offset this and improve the scaling of the pass. Running the pass in parallel was previously tried in #4781 using Python multiprocessing but the overhead of launching an additional process and serializing the input arrays for each trial was significantly larger than the speed gains. To run the algorithm efficiently in parallel multithreading is needed to leverage shared memory on shared inputs. This commit rewrites the cython routine using rust. This was done for two reasons. The first is that rust's safety guarantees make dealing with and writing parallel code much easier and safer. It's also multiplatform because the rust language supports native threading primatives in language. The second is while writing parallel cython code using open-mp there are limitations with it, mainly on windows. In practice it was also difficult to write and maintain parallel cython code as it has very strict requirements on python and c code interactions. It was much faster and easier to port it to rust and the performance for each iteration (outside of parallelism) is the same (in some cases marginally faster) in rust. The implementation here reuses the data structures that the previous cython implementation introduced (mainly flattening all the terra objects into 1d or 2d numpy arrays for efficient access from C). The speedups from this PR can be significant, calling transpile() on a 400 qubit (with a depth of 10) QV model circuit targetting a 409 heavy hex coupling map goes from ~200 seconds with the single threaded cython to ~60 seconds with this PR locally on a 32 core system, When transpiling a 1000 qubit (also with a depth of 10) QV model circuit targetting a 1081 qubit heavy hex coupling map goes from taking ~6500 seconds to ~720 seconds. The tradeoff with this PR is for local qiskit-terra development a rust compiler needs to be installed. This is made trivial using rustup (https://rustup.rs/), but it is an additional burden and one that we might not want to make. If so we can look at turning this PR into a separate repository/package that qiskit-terra can depend on. The tradeoff here is that we'll be adding friction to the api boundary between the pass and the core swap trials interface. But, it does ease the dependency on development for qiskit-terra. * Sanitize packaging to support future modules This commit fixes how we package the compiled rust module in qiskit-terra. As a single rust project only gives us a single compiled binary output we can't use the same scheme we did previously with cython with a separate dynamic lib file for each module. This shifts us to making the rust code build a `qiskit._accelerate` module and in that we have submodules for everything we need from compiled code. For this PR there is only one submodule, `stochastic_swap`, so for example the parallel swap_trials routine can be imported from `qiskit._accelerate.stochastic_swap.swap_trials`. In the future we can have additional submodules for other pieces of compiled code in qiskit. For example, the likely next candidate is the pauli expectation value cython module, which we'll likely port to rust and also make parallel (for sufficiently large number of qubits). In that case we'd add a new submodule for that functionality. * Adjust random normal distribution to use correct mean This commit corrects the use of the normal distribution to have the mean set to 1.0. Previously we were doing this out of band for each value by adding 1 to the random value which wasn't necessary because we could just generate it with a mean of 1.0. * Remove unecessary extra scope from locked read This commit removes an unecessary extra scope around the locked read for where we store the best solution. The scope was previously there to release the lock after we check if there is a solution or not. However this wasn't actually needed as we can just do the check inline and the lock will release after the condition block. * Remove unecessary explicit type from opt_edges variable * Fix indices typo in NLayout constructor Co-authored-by: Jake Lishman <jake@binhbar.com> * Remove explicit lifetime annotation from swap_trials Previously the swap_trials() function had an explicit lifetime annotation `'p` which wasn't necessary because the compiler can determine this on it's own. Normally when dealing with numpy views and a Python object (i.e. a GIL handle) we need a lifetime annotation to tell the rust compiler the numpy view and the python gil handle will have the same lifetime. But since swap_trials doesn't take a gil handle and operates purely in rust we don't need this lifetime and the rust compiler can deal with the lifetime of the numpy views on their own. * Use sum() instead of fold() * Fix lint and add rust style and lint checks to CI This commit fixes the python lint failures and also updates the ci configuration for the lint job to also run rust's style and lint enforcement. * Fix returned layout mapping from NLayout This commit fixes the output list from the `layout_mapping()` method of `NLayout`. Previously, it incorrectly would return the wrong indices it should be a list of virtual -> physical to qubit pairs. This commit corrects this error Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Tweak tox configuration to try and reliably build rust extension * Make swap_trials parallelization configurable This commit makes the parallelization of the swap_trials() configurable. This is dones in two ways, first a new argument parallel_threshold is added which takes an optional int which is the number of qubits to switch between a parallel and serial version. The second is that it takes into account the the state of the QISKIT_IN_PARALLEL environment variable. This variable is set to TRUE by parallel_map() when we're running in a multiprocessing context. In those cases also running stochastic swap in parallel will likely just cause too much load as we're potentially oversubscribing work to the number of available CPUs. So, if QISKIT_IN_PARALLEL is set to True we run swap_trials serially. * Revert "Make swap_trials parallelization configurable" This reverts commit 57790c84b03da10fd7296c57b38b54c5bccebf4c. That commit attempted to sovle some issues in test running, mainly around multiple parallel dispatch causing exceess load. But in practice it was broken and caused more issues than it fixed. We'll investigate and add control for the parallelization in a future commit separately after all the tests are passing so we have a good baseline. * Add docs to swap_trials() and remove unecessary num_gates arg * Fix race condition leading to non-deterministic behavior Previously, in the case of circuits that had multiple best possible depth == 1 solutions for a layer, there was a race condition in the fast exit path between the threads which could lead to a non-deterministic result even with a fixed seed. The output was always valid, but which result was dependent on which parallel thread with an ideal solution finished last and wrote to the locked best result last. This was causing weird non-deterministic test failures for some tests because of #1794 as the exact match result would change between runs. This could be a bigger issue because user expectations are that with a fixed seed set on the transpiler that the output circuit will be deterministically reproducible. To address this is issue this commit trades off some performance to ensure we're always returning a deterministic result in this case. This is accomplished by updating/checking if a depth==1 solution has been found in another trial thread we only act (so either exit early or update the already found depth == 1 solution) if that solution already found has a trial number that is less than this thread's trial number. This does limit the effectiveness of the fast exit, but in practice it should hopefully not effect the speed too much. As part of this commit some tests are updated because the new deterministic behavior is slightly different from the previous results from the cython serial implementation. I manually verified that the new output circuits are still valid (it also looks like the quality of the results in some of those cases improved, but this is strictly anecdotal and shouldn't be taken as a general trend with this PR). * Apply suggestions from code review Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> * Fix compiler errors in previous commit * Revert accidental commit of parallel reduction in compute_cost This was only a for local testing to prove it was a bad idea and was accidently included in the branch. We should not nest the parallel execution like this. * Eliminate short circuit for depth == 1 swap_trial() result This commit eliminates the short circuit fast return in swap_trial() when another trial thread has found an ideal solution. Trying to do this in a parallel context is tricky to make deterministic because in cases of >1 depth == 1 solutions there is an inherent race condition between the threads for writing out their depth == 1 result to the shared location. Different strategies were tried to make this reliably deterministic but there wa still a race condition. Since this was just a performance optimization to avoid doing unnecessary work this commit removes this step. Weighing improved performance against repeatability in the output of the compiler, the reproducible results are more important. After we've adopted a multithreaded stochastic swap we can investigate adding this back as a potential future optimization. * Add missing docstrings * Add section to contributing on installing form source * Make rust python classes pickleable * Add rust compiler install to linux wheel jobs * Try more tox changes to fix docs builds * Revert "Eliminate short circuit for depth == 1 swap_trial() result" This reverts commit c510764a770cb610661bdb3732337cd45ab587fd. The removal there was premature and we had a fix for the non-determinism in place, ignoring a typo which was preventing it from working. Co-Authored-By: Georgios Tsilimigkounakis <45130028+georgios-ts@users.noreply.github.com> * Fix submodule declaration and module attribute on rust classes * Fix rust lint * Fix docs job definition * Disable multiprocessing parallelism in unit tests This commit disables the multiprocessing based parallelism when running unittest jobs in CI. We historically have defaulted the use of multiprocessing in environments only where the "fork" start method is available because this has the best performance and has no caveats around how it is used by users (you don't need an `if __name__ == "__main__"` guard). However, the use of the "fork" method isn't always 100% reliable (see https://bugs.python.org/issue40379), which we saw on Python 3.9 #6188. In unittest CI (and tox) by default we use stestr which spawns (not using fork) parallel workers to run tests in parallel. With this PR this means in unittest we're now running multiple test runner subprocesses, which are executing parallel dispatched code using multiprocessing's fork start method, which is executing multithreaded rust code. This three layers of nesting is fairly reliably hanging as Python's fork doesn't seem to be able to handle this many layers of nested parallelism. There are 2 ways I've been able to fix this, the first is to change the start method used by `parallel_map()` to either "spawn" or "forkserver" either of these does not suffer from random hanging. However, doing this in the unittest context causes significant overhead and slows down test executing significantly. The other is to just disable the multiprocessing which fixes the hanging and doesn't impact runtime performance signifcantly (and might actually help in CI so we're not oversubscribing the limited resources. As I have not been able to reproduce `parallel_map()` hanging in a standalone context with multithreaded stochastic swap this commit opts for just disabling multiprocessing in CI and documenting the known issue in the release notes as this is the simpler solution. It's unlikely that users will nest parallel processes as it typically hurts performance (and parallel_map() actively guards against it), we only did it in testing previously because the tests which relied on it were a small portion of the test suite (roughly 65 tests) and typically did not have a significant impact on the total throughput of the test suite. * Fix typo in azure pipelines config * Remove unecessary extension compilation for image tests * Add test script to explicitly verify parallel dispatch In an earlier commit we disabled the use of parallel dispatch in parallel_map() to avoid a bug in cpython associated with their fork() based subprocess launch. Doing this works around the bug which was reliably triggered by running multiprocessing in parallel subprocesses. It also has the side benefit of providing a ~2x speed up for test suite execution in CI. However, this meant we lost our test coverage in CI for running parallel_map() with actual multiprocessing based parallel dispatch. To ensure we don't inadvertandtly regress this code path moving forward this commit adds a dedicated test script which runs a simple transpilation in parallel and verifies that everything works as expected with the default parallelism settings. * Avoid multi-threading when run in a multiprocessing context This commit adds a switch on running between a single threaded and a multithreaded variant of the swap_trials loop based on whether the QISKIT_IN_PARALLEL flag is set. If QISKIT_IN_PARALLEL is set to TRUE this means the `parallel_map()` function is running in the outer python context and we're running in multiprocessing already. This means we do not want to be running in multiple threads generally as that will lead to potential resource exhaustion by spawn n processes each potentially running with m threads where `n` is `min(num_phys_cpus, num_tasks)` and `m` is num_logical_cpus (although only `min(num_logical_cpus, num_trials)` will be active) which on the typical system there aren't enough cores to leverage both multiprocessing and multithreading. However, in case a user does have such an environment they can set the `QISKIT_FORCE_THREADS` env variable to `TRUE` which will use threading regardless of the status of `QISKIT_IN_PARALLEL`. * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> * Minor fixes from review comments This commits fixes some minor details found during code review. It expands the section on building from source to explain how to build a release optimized binary with editable mode, makes the QISKIT_PARALLEL env variable usage consistent across all jobs, and adds a missing shebang to the `install_rush.sh` script which is used to install rust in the manylinux container environment. * Simplify tox configuration In earlier commits the tox configuration was changed to try and fix the docs CI job by going to great effort to try and enforce that setuptools-rust was installed in all situations, even before it was actually needed. However, the problem with the docs ci job was unrelated to the tox configuration and this reverts the configuration to something that works with more versions of tox and setuptools-rust. * Add missing pieces of cargo configuration Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2022-03-01 05:49:54 +08:00
[profile.release]
lto = 'fat'
codegen-units = 1