This commit adds @nkanazawa1989 as a listed code owner for the qpy
module. @nkanazawa1989 introduced the standalone qpy in #7852 which
made the qpy code far more understandable and maintainable. He has
shown good understanding of both file format and terra code using the
format. This commit gives @nkanazawa1989 merge permission on PRs that
touch QPY code and a formal responsibility to review and maintain the
submodule.
Aer currently sets `-ffast-math` during compilation, which when compiled
with GCC causes the CPU's floating-point rounding mode to be set to
"flush to zero", and subnormal numbers are disallowed. This should not
be the case, and Qiskit/qiskit-aer#1469 will solve the problem in
release. Until then, we must instruct `hypothesis` to avoid subnormal
numbers in its floating-point strategies, as since version 6.38 it
explicitly tests to ensure that they are functional, if used.
This commit should be reverted once Aer no longer sets `-ffast-math`.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* Add rust to binder postBuild
In #7658 we updated added rust code to the Qiskit build environment.
This was done to accelerate performance critical portions of the
library. However, in #7658 we overlooked the binder tests which are used
to perform image comparisons for visualizations in a controlled
environment. The base binder docker image does not have the rust
compiler installed, so we need to manually install it prior to running
pip to install terra. This commit takes care of this and adds rust to
the binder environment.
* Install rust via conda for binder env
The binder image build is implicitly installing terra as it launches. So
trying to install rust manually as part of the postBuild script is too
late because it will have failed by then. Looking at the available
configuration files:
https://mybinder.readthedocs.io/en/latest/config_files.html?#configuration-files
we can install conda packages defined in environment.yml as part of the
image build, and this will occur prior to the installation of terra.
This commit pivots to using the environment.yml to do this and we can
rely on the conda packaged version of rust instead of rustup.
* Move all binder files .binder/ dir
* Remove duplicate pip install in postBuild
* Revert "Remove duplicate pip install in postBuild"
This was actually needed, without this terra isn't actually installed.
This reverts commit d61c7a68e8.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
The FakeWashington backend was recently added in #7392 but when that PR
was created the washington device was missing it's pulse defaults
payload. Since that PR was first created the IBM API is now returing a
pulse defaults payload. This commit updates the FakeWashington backend
to use current snapshots which includes the missing data. It is then
changed to be a pulse backend now that we have the defaults payload
available.
By default Rust libraries don't ship a Cargo.lock file. This is to allow
other Rust consumers of the library to pick a compatible version with
the other upstream dependencies. [1] However, the library we build in
Qiskit is a bit different since it's not a traditional Rust library but
instead we're building a C dynamic library that is meant to be consumed
by Python. This is much closer a model to developing a Rust binary
program because we're shipping a standalone binary. To support
reproducible builds we should include the Cargo.lock file in our source
distribution to ensure that all builds of qiskit-terra are using the
same versions of our upstream Rust dependencies. This commit commits the
missing Cargo.lock file, removes it from the .gitignore (which was added
automatically by cargo when creating a library project), and includes it
in the sdist. This will ensure that any downstream consumer of terra
from source will have a reproducible build. Additionally this adds a
dependabot config file so the bot will manage proposing version bumps on
upstream project releases, since we probably want to be using the latest
versions on new releases in our lock file.
[1] https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries
* rm deprecated algo methods
* add reno
* fix tests, remove from varalgo
* intial point was said to be abstract in varalgo!
* attempt to fix sphinx #1 of ?
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* 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 57790c84b0. 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 c510764a77. 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>
* First testing
* Fix display of sidetext gates with conditions in text
* Add comment
* Start it up again
* Add mpl and latex tests
* Add cu1 and rzz tests
* Start it up again
* Break out RZZ and CU1
* Restart
* * First draft of the instruction duration odification.
* * Adding suggestion by Itoko
* * Fix bug where duration and parameters were switched
* * Remove None from tests.
* * black.
* * Added check on None duration.
* * Added test.
* * Reno
* * Test fix.
* * Moved test and updated reno.
* * Docstring.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* Implementing LinearFunction gate and a transpiler pass that collects a sequence of linear gates into a LinearFunction
* removing test file
* running black and pylint
* Reimplementing linear function to inherit from Gate; adding tests for linear functions
* adding tests for CollectLinearFunctions transpiler optimization pass
* Improving tests for CollectLinearFunctions pass
* style
* style
* Adding LinearFunction to exclude in test_gate_definitions
* Normalizing internal representation to numpy array format; adding example of a linear matrix
* using find_bit command
* removing stray pylint comment
* fixing lint error
* adding a comment
* Update qiskit/circuit/library/generalized_gates/linear_function.py
Co-authored-by: Kevin Krsulich <kevin@krsulich.net>
* adding a transpiler pass that synthesizes linear functions, and updating tests
* renaming; changing default behavior to copy
* Adding comments regarding synthesis and big-endian
* adding transpiler passes to synthesize linear functions and to promote them to permutations whenever possible
* adding release notes
* code improvements following the review
* adding explicit reference to pdf
* removing redundant is_permutation check
* First pass over comments in the review
* minor tweak to release notes
* trying to get links in release notes to work
* trying to get links in release notes to work
* pass over documentation
* improving tests
* treating other review comments
* removing accidentally added param
* Use specific testing assertions
* Specific assertion stragglers
* changing the assert
Co-authored-by: Kevin Krsulich <kevin@krsulich.net>
Co-authored-by: Jake Lishman <jake@binhbar.com>
* Use VF2Layout in all preset passmanagers
With the introduction of the VF2Layout pass we now have a very fast
method of searching for a perfect layout. Previously we only had the
CSPLayout method for doing this which could be quite slow and we only
used in level 2 and level 3. Since VF2Layout operates quickly adding the
pass to each preset pass manager makes sense so we always use a perfect
layout if available (or unless a user explicitly specifies an initial
layout or layout method). This commit makes this change and adds
VF2Layout to each optimization level and uses a perfect layout if found
by default.
Fixes#7156
* Revert changes to level 0
For optimization level 0 we don't actually want to use VF2Layout because
while it can find a perfect layout it would be potentially surprising to
users that the level which is supposed to have no optimizations picks a
non-trivial layout.
* Set seed on perfect layout test
* Fix test failures and unexpected change in behavior
This commit makes several changes to fix both unexpected changes in
behavior and also update the default behavior of the vf2 layout pass.
The first issues is that the vf2 pass was raising an exception if it was
run with >2 qubit gates. This caused issues if we run with calibrations
(or backends that support >2q gates) as vf2 layout is used as an
opportunistic thing and if there is for example a 5q gate being used
we shouldn't fail the entire transpile just because vf2 can't deal with
it. It's only an issue if the later passes can't either, it just means
vf2 won't be able to find a perfect layout. The second change is around
the default seeding. For the preset pass managers to have a consistent
output this removed the randomization if no seed is specified and just
use an in order comparison. This is necessary to have a consistent
layout for testing and reproducability, while we can set a seed
everywhere, the previous behavior was more stable as it would default to
trivial layout most of the time (assuming that was perfect). When we add
multiple vf2 trials and are picking the best choice among those for a
given time budget we can add back in the default seed randomization. The
last change made here is that several tests implicitly expected a
trivial layout (mainly around device aware transpilation or
calibrations). In those cases the transpile wasn't valid for an
arbitrary layout, for example if a calibrated gate is only defined on a
single qubit. Using vf2 layout in those cases doesn't work because the
gate is only defined on a single qubit so picking a non-trivial layout
correctly errors. To fix these cases the tests are updated to explicitly
state they require a trivial layout instead of assuming the transpiler
will implicitly give them that if it's a perfect layout.
* Add missing release note
* Apply suggestions from code review in release note
Co-authored-by: Jake Lishman <jake@binhbar.com>
* Make vf2 layout stop reason an Enum
* Update releasenotes/notes/vf2layout-preset-passmanager-db46513a24e79aa9.yaml
Co-authored-by: Kevin Krsulich <kevin@krsulich.net>
* Add back initial layout to level1
Since there seems to be a pretty baked in asumption for level 1 that it
will use the trivial layout by default if it's a perfect mapping. This
was causing the majority of the test failures and might be an unexpected
breakage for people. However, in a future release we should remove this
(likely when vf2layout is made noise aware). To anticipate this a
FutureWarning is emitted when a trivial layout is used to indicate that
this behavior will change in the future for level 1 and if you're
relying on it you should explicitly set the layout_method='trivial'.
* Tweak seeds to reduce effect of noise on fake_yorktown with aer
* Update release note
* Use id_order=True on vf2_mapping() for VF2Layout pass
Using id_order=False orders the nodes by degree which biases the
mapping found by vf2 towards nodes with higher connectivity which
typically have higher error rates. Until the pass is made noise aware
to counter this bias we should just use id_order=True which uses the
node id for the order (which roughly matches insertion order) which
won't have this bias in the results.
* Revert "Use id_order=True on vf2_mapping() for VF2Layout pass"
VF2 without the Vf2++ heuristic is too slow for some common use cases
that we probably don't want to use it by default. Instead we should use
some techniques to improve the quality of the results. The first
approach will be applying a score heuristic to a found mapping. A
potential follow on after that could be to do some pre-filtering of
noisy nodes.
This reverts commit 53d54c3a3648288e40d3ae21e76206f88cb7b981.
* Set real limits on calling vf2 and add quality heuristic
This commit adds options to set limits on the vf2 pass, both the
internal call limit for the vf2 execution in retworkx, a total time
spent in the pass trying multiple layouts, and the number of trials to
attempt. These are then set in the preset pass manager to ensure we
don't sit spinning on vf2 forever in the real world. While the pass is
generally fast there are edge cases where it can get stuck. At the same
time this adds a rough quality heuristic (based on readout error falling
back to connectivity) to select between multiple mappings found by
retworkx. This addresses the poor quality results we were getting with
vf2++ in earlier revisions as we can find the best from multiple
mappings.
* Remove initial layout default from level 1
Now that vf2 layout has limited noise awareness and multiple trials it
no longer will be defaulting to the worst qubits like it was with only a
single sample when vf2++ is used. This commit removes the implicit
trivial layout attempt as it's no longer needed.
* Remove unused imports
* Use vigo instead of yorktown for oracle tests
* Revert "Remove initial layout default from level 1"
This is breaking the pulse tutorials, as fixing that is a more
involved change and probably indicates we should continue to
assume trivial layout by default if perfect for level 1 and raise a
warning to users that it's going away to give everyone who needs a
default trivial layout time to adjust their code. This commit reverts
only using vf2 for level 1 and adds back in a trivial layout stage.
This reverts commit df478357b9.
This reverts commit 65ae6ee0da.
* Fix warning and update docs to not emit one
This commit fixes the warning so it's only emitted if the trivial layout
is used, previously it would also be emitted if an initial layout was
set. The the docs are updated to not emit the warning, in some cases
code examples are updated to explicitly use a trivial layout if that's
what's needed. In others there were needless jupyter-execute directives
being used when there was no visualization and a code-block is just as
effective (which avoids the execution during doc builds).
* Add debug logging and fix heurstic usage
* Add better test coverage of new pass features
* Only run a single trial if the graphs are the same size
If the interaction graph and the coupling graph are the same size
currently the score heuristic will produce the same results since they
just look at the sum of qubit noise (or degree). We don't need to run
multiple trials or bother scoring things since we'll just pick the first
mapping anyway.
* Fix rebase error
* Use enum type for stop reason condition
* Add comment about call_limit value
* Undo unecessary seed change
* Add back default seed randomization
After all the improvements to the VF2Layout pass in #7276 this was no
longer needed. It was added back prior to #7276 where the vf2 layout
pass was not behaving well for simple cases.
* Permute operator bits based on layout
The backendv2 transpilation tests were failing with vf2 layout enabled
by default because we were no longer guaranteed to get an initial layout
by default. The tests were checking for an equivalent operator between
the output circuit and the input one. However, the use of vf2layout was
potentially changing the bit order (especially at higher optimization
levels) in the operator because a non trivial layout was selected. This
caused the tests to fail. This commit fixes the test failures by adding
a helper function to permute the qubits back based on the layout
property in the transpiled circuit.
* Fix docs build
* Remove warning on use of TrivialLayout in opt level 1
The warning which was being emitted by an optimization level 1 transpile()
if a trivial layout was used was decided to be too potentially noisy for
users, especially because it wasn't directly actionable. For the first
step of using vf2 layout everywhere we decided to leave level1 as trying
a trivial layout first and then falling back to vf2 layout if the
trivial layout isn't a perfect match. We'll investigate whether it makes
sense in the future to change this behavior and come up with a migration
plan when that happens.
* Apply suggestions from code review
Co-authored-by: Jake Lishman <jake@binhbar.com>
* Cleanup inline comment numbering in preset pass manager modules
* Fix lint
* Remove operator_permuted_layout() from backendv2 tests
This commit removes the custom operator_permuted_layout() function from
the backendv2 tests. This function was written to permute the qubits
based on the output layout from transpile() so it can be compared to the
input circuit for equivalence. However, since this PR was first opened a
new constructor method Operator.from_circuit() was added in #7616 to
handle this directly in the Operator construction instead of doing it
out of band. THis commit just leverages the new constructor instead of
having a duplicate local test function.
* Remove unused import
Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: Kevin Krsulich <kevin@krsulich.net>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* Update fake_bogota.py
- refer to the defs file and turn it into a FakePulseBackend
* Update fake_manila.py
- refer to the defs file and turn it into a FakePulseBackend
* Update fake_rome.py
- refer to the defs file and turn it into a FakePulseBackend
* Update fake_santiago.py
- refer to the defs file and turn it into a FakePulseBackend
* Update fake_bogota.py
- make sure we are using FakePulseLegacyBackend where it is needed.
* Create bogota-manila-rome-santiago-as-fakepulsebackends-2907dec149997a27.yaml
- add release notes
* Update releasenotes/notes/bogota-manila-rome-santiago-as-fakepulsebackends-2907dec149997a27.yaml
no need for prelude 🎹🎵😕
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
* Update releasenotes/notes/bogota-manila-rome-santiago-as-fakepulsebackends-2907dec149997a27.yaml
🧐 using proper notation.
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
* Fix typo
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
The previous resolver of indices for bits involved catching several
exceptions even when resolving a valid specifier. This is comparatively
slow for inner-loop code. The implementation also assumed that if a
type could be cast to an integer, the only way it could be a valid
specifier was as an index. This broke for size-1 Numpy arrays, which
can be cast to `int`, but should be treated as iterables.
Since `QuantumCircuit.append` necessarily checks the types of all its
arguments, it is unnecessary for `QuantumCircuit._append` to do so as
well. This also allows anywhere that is constructing a `QuantumCircuit`
from known-safe data (such as copying from an existing circuit, or
building templates) to do so without the checks. This is now
documented as its contract.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* Move qpy to own module.
qpy_serialization.py is splint into several files for maintenability. This commit also adds several bytes Enum classes for type keys in the header, and provides several helper functions. Some namedtuple class names are updated because, for example, INSTRUCTION will be vague when we add schedule, i.e. it's basically different program and has own instruction that has different data format. Basically CIRCUIT_ prefix is added to them.
* manually cherry-pick #7584 with some cleanup
- change qiskit.qpy.objects -> qiskit.qpy.binary_io
- TUPLE -> SEQUENCE (we may use this for list in future)
- add QpyError
- add _write_register in circuit io to remove boilerplate code
* respond to review comments
- expose several private methods for backward compatibility
- use options for symengine
- rename alphanumeric -> value
- rename write, read methods and remove alias
- improve container read
* remove import warning
* replace alphanumeric with value in comments and messages.
* private functions import
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* fix scheduling pass
This commit updates both ASAP and ALAP passes not to allow measurement instructions to simultaneously write in the same register.
In addition, delay appended after end of circuit is removed since this instruction has no effect.
* Update behavior of passes
Added `BaseScheduler` as a parent class of ASAP and ALAP passes. These scheduler can take two control parameters `clbit_write_latency` and `conditional_latency`. These represent I/O latency of clbits.
In addition, delays in the very end of the scheduled circuit is readded because Dynamical Decoupling passes inserts echo sequence there. More unittests and reno are also added.
* fix ASAP conditional bug
The conditional bit start time was only looking cregs. But this should start right before the gate.
Co-authored-by: Toshinari Itoko <itoko@jp.ibm.com>
* respond to review comments
- fix typo
- update model drawing in comment
- more comment
* update documentation and add todo comment
* update logic to insert conditional bit assert
* add more docs on topological ordering
* Update qiskit/transpiler/passes/scheduling/base_scheduler.py
Co-authored-by: Toshinari Itoko <15028342+itoko@users.noreply.github.com>
* lint fix
Co-authored-by: Toshinari Itoko <itoko@jp.ibm.com>
Co-authored-by: Toshinari Itoko <15028342+itoko@users.noreply.github.com>
Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
* Bump minimum supported symengine version for built-in pickle support
The new symengine 0.9 release added native support in the package for
pickling symengine objects. Previously we had been converting symengine
objects to sympy objects so we could pickle them. With native support
for pickle in symengine now we no longer need this which besides
removing unnecessary should hopefully make pickling (which we do
internally as part of using multiprocessing) more reliable.
This also seems to fix the hanging we were seeing with multiprocessing
with Python 3.9 on Linux. While investigating that issue it points to
the underlying cause being a bug in cPython with the `fork()` based
start method, but we were only able to reliably trigger it after
switching to symengine in #6270 and having to rely on importing
symengine to pickle the symengine objects. Since we're no longer doing
that after bumping the minimum symengine version this removes the
default disabling of parallel dispatch with Python 3.9. While I'm not
100% confident this fixes the bug, in my testing locally I haven't been
able to reproduce the hang we were encountering (but this is ancedotal
at best). If we do encounter issues with multiprocess hanging in the
future we can look at rewriting the internals of `parallel_map()` or
switching it back to disabled by default.
Fixes#6188
* Fix typos in release notes
Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* Add fake backends for new IBM Quantum systems (#6808)
This commit adds new fake backend classes for new IBM Quantum systems:
Cairo, Hanoi, Kolkata, Nairobi, and Washington. Just as with the other
fake backends these new classes contain snapshots of calibration and error
data taken from the real system, and can be used for local testing,
compilation and simulation.
Legacy backends are not added for these new fake backends as the
legacy backend interface is deprecated and will be removed in a future
release so there is no need to expose that for the new backends (it was
only added for compatibility testing on the old fake backends).
* Update qiskit/test/mock/backends/washington/fake_washington.py
Co-authored-by: Ali Javadi-Abhari <ajavadia@users.noreply.github.com>
* Update releasenotes/notes/new-fake-backends-04ea9cb26374e385.yaml
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
* Fix lint
Co-authored-by: Ali Javadi-Abhari <ajavadia@users.noreply.github.com>
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* Fix `partial_trace` when not removing subsystems
There were places, such as in `quantum_info.partial_trace` where an
`OpShape` output of `OpShape.remove` was mutated. In cases where
nothing was removed from the `OpShape`, it returned itself, and
consequently caused lingering issues with the object that owned the
shape. This ensures a copy is always produced, and adds in a special
case to handle `DensityMatrix` with nothing traced out.
* Check qargs not shape equality
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* expose atol and rtol to Z2Symmetries
* add reno
* add test
* default tolerances: 0
* introduce chop
* fix all-chop
* fix test and reno
* reno layout
* Apply suggestions from code review
Co-authored-by: Max Rossmannek <max.rossmannek@uzh.ch>
* test case where chop does not accumulate
* chop more efficient
... and other review comments!
Co-authored-by: Max Rossmannek <max.rossmannek@uzh.ch>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* Bugfix in local mitigator and a relevant test
* Name fix
* Refactoring mitigation tests
* Cleanup
* Linting
* Linting
* Small bugfix and a test for it
* Fix to rng usage
* Linting
* Changing test sensitivity
* Added settings property to mitigator to allow JSON encoding when given as experiment result
* Linting
* Linting
* Linting
* Release note relating to the bugfix
* Get the assignment matrices implicitly
* Using more specific assertions
* Removing magic constants
* Make initialization conform to settings
* Linting
* Removing testcase which might fail on some machines
* Removing testcase which might fail on some machines
* Linting
* Reword release note
* Reword release note
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* fix a bug in expectation value. Handle Pauli phases.
* extend the test to include Pauli expectation vals with phases.
* add release notes
* add more 1-qubit and 2-qubit tests for exp_val=-1,i,-i
* more ddt
* Fix typo in error message
* Add comment on phase
Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Replaces the bulk of the implementation of DAGCircuit.replace_block_with_op with the
generic retworkx method PyDiGraph.contract_nodes, added in Qiskit/retworkx#487.
At a minimum, this change rids Terra of graph manipulation code that is more widely
applicable in a generic graph library. For our current use cases (the ConsolidateBlocks
pass) this change might improve performance slightly, but the primary speedup would
be observed by replace_block_with_op callers that pass cycle_check=True, since
retworkx's contract_nodes performs this check prior to modifying the DAG, and without
making a backup copy.
Resolves#7418
* Use contract_nodes from rx in DAGCircuit.
* Bump min retworkx version and add release note.
* Remove temp variable.
* Add circuit diagrams to test_hoare_opt.py
* Add circuit diagram to equivalence_library.py
* Add a TODO in equivalence_library.py
* Add circuit diagram to echo_rzx_weyl_decomposition
* Add circuit diagram to merge_adjacent_barriers.py
* Add circuit diagram to test/python/circuit/library
* Add circuit diagram to test/python/circuit
* Add circuit diagram to test/python/compiler
* Add circuit diagram to test/python/dagcircuit
* Add circuit diagram to test/python
* Format source code with black
* Chang block comment from string to line annotation
* Add circuit diagram to standard_gates
* Add parsed-literal before circuit diagram
* fix a lint problem
* Update circuit diagram in test
* Fix a lint problem
* Add blank lines before and after parsed-literal
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
In the recently merged #7615 we bumped the black version and constrained
it to a stable version to keep us on a fixed set of formatting rules but
also receiving bug fixes. In doing this some new formatting rules were
applied to the repo (mainly `x ** y` was changed to `x**y`) by the new
version of black. To reduce noise in the git blame this commit updates
the .git-blame-ignore-revs file (which was added after we started using
black in #6362) to include the sha1 for this commit. This means that
when running git blame on files this commit will be ignored (assuming
the local git environment is configured correctly).
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
This explicitly overrides the release notes directory to have no code
owner, so anybody with write access can give an approving review to
changes within, without GitHub requesting review from everyone.
This commit bumps the black version we pin to the latest release,
22.1.0. This release is also the first release not marked as beta and
with that black has introduced a stability policy where no formatting
changes will be introduced on a major version release (which is the
year). [1] With this new policy in place we no longer need to pin to a
single version and can instead constrain the requirement to just the
major version without worrying about a new release breaking ci or local
development. This commit does that and sets the black version
requirement to be any 22.x.y release so that we'll continue to get
bugfixes moving forward without having to manually bump a pinned version.
[1] https://black.readthedocs.io/en/latest/the_black_code_style/index.html#stability-policy
* Add Operator.from_circuit constructor method
This commit adds a new constructor method .from_circuit() to the
Operator class. This allows users to construct Operator objects from
QuantumCircuits and have additional features than the default __init__
constructor allows. To start this just involves permuting the qubit
order based on the specified layout. The permutation based on layout has
come before, especially in testing with transpilation (see #7213 for
where equivalent functionality was done inline in the unittests).
Exposing this as an api will provide a single place to build an Operator
with a layout. It also will provide us a dedicated location if we need
to offer additional options to the construction of Operators from
circuits.
* Update releasenotes/notes/Operator-from_circuit-25b20d4b3ad5c398.yaml
Co-authored-by: Jake Lishman <jake@binhbar.com>
* Rename use_layout kwarg to ignore_set_layout
The use_layout kwarg was a bit confusing as it was previously
implemented. Since it could lead to situations like:
Operator.from_circuit(circuit, use_layout=False, layout=Layout(...))
which wasn't the clearest. To address this issue this commit renames the
kwarg ignore_set_layout so that it's clear the flag controls any layout
set on the circuit. This should hopefully make the purpose of the flag
and it's behavior when used with the "layout" kwarg clearer.
Co-authored-by: Jake Lishman <jake@binhbar.com>
This commit makes some updates to the codeowners file to cleanup both
the membership lists as well as better outline the purpose of the file.
The first change it makes is to remove duplicate membership in the code
owners list. Several people were both listed explicitly as a codeowner
on a subsection of the repository and are current members of the
terra-core github group. The two should be treated as mutually exclusive
as membership in terra-core means you have permission to merge any PR,
not just those in the listed sections. The second change is to remove
some inactive members of the project who are no longer actively engaged
in work on qiskit-terra but were still listed in the codeowners file.
The third thing that is done here is to improve the comments around the
purpose for the file. The previous comments previously made it seem like
this was more about assigning reviewers/notification for PRs. But, the
primary purpose of the file should be to be able to assign an
expectation and responsibility for maintaining and reviewing subsections
of the code to members of the project who are not part of the terra-core
group. This enables individuals to take on ownership of these sections
and reduce the burden on the core team. The final change made in this PR
is to make the release notes approvable by any user listed in the
code owners, not just terra-core as was previously. Most PRs have
release notes, but by restricting the contributors empowered to merge
them to just terra-core this in practice eliminated the usefulness of
having codeowners because a core team member would need to approve it
anyway. By enabling any codeowner to approve/merge release notes a
change isolated to a subsection of code with the exception of a release
note can now individually approve and merge that PR.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* Pauli I should not insert an IGate
* correct tase of PauliOp("I")
* add test for plain Pauli identity to circuit
* Update releasenotes/notes/opflow-igate-97df9a8b809116f1.yaml
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* Unify lazy handling of optional dependencies
This introduces new `HAS_X` variables for each of Qiskit's optional
dependencies, and provides a simple unified interface to them from
`qiskit.utils.optionals`. These objects lazily test for their
dependency when evaluated in a Boolean context, and have two `require_`
methods to unify the exception handling. `require_now` tests
immediately for the dependency and raises `MissingOptionalLibraryError`
if it is not present, and `require_in_call` is a decorator that lazily
tests for the dependencies when the function is called.
These remove the burden of raising nice exceptions from the usage
points; a function marked `HAS_MATPLOTLIB.require_in_call` can now
safely `import matplotlib` without special handling, for example. This
also provides a unified way for consumers of `qiskit` (such as the test
suite) to query the presence of libraries.
All tests are now lazy, and imports are moved to the point of usage, not
the point of import of the module. This means that `import qiskit` is
significantly faster for people who have many of the optional
dependencies installed; rather than them all being loaded at initial
import just to test their presence, they will now be loaded on demand.
* Optimise time taken for `import qiskit`
This makes several imports lazy, only being imported when they are
actually called and used. In particular, no component of `scipy` is
imported during `import qiskit` now, nor is `pkg_resources` (which is
surprisingly heavy).
No changes were made to algorithms or opflow, since these are not
immediately imported during `import qiskit`, and likely require more
significant work than the rest of the library.
* Import missing to-be-deprecated names
* Convert straggler tests to require_now
* Correct requirements in test cases
* Add `require_in_instance` class decorator
Effectively this is just a wrapper around `__init__`, except that this
class-decorator form will do the right thing even if `__init__` isn't
explicitly defined on the given class.
The implementation of `wrap_method` is a replacement for the older
`test.decorators._wrap_method`, which didn't handle all the possible
special cases as well, and messed up the documentation of its wrapped
functions. That wasn't so important when it was just a private
function, but now it has become public (so that
`test.decorators.enforce_subclasses_call` can still use it from
`qiskit.utils`), it needed reworking to be more polished.
* Privatise non-public names rather than del
* Add tests of `require_in_instance`
* Fix typos in documentation
* Add section on requirements to CONTRIBUTING
* Update documentation on HoareOptimizer error
* Remove UK localisation
* Mention more uses of PIL in documentation
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* Fix QPY support for control flow instructions
This commit fixes support for using qpy serialization with circuits that
contained control flow instructions. Previously, if you attempted to
serialize a circuit that contained control flow instructions it would
error as qpy didn't support embedding the circuit blocks as parameters
for the instructions. This has been fixed and other missing pieces for
fully reconstructing circuits with control flow were added. This
requires bumping the version string to v4 because to accurately
reconstruct control flow circuits we needed to be able to represent
registers that were only partially present in a circuit fully. V4 also
adds a representation for ranges which didn't exist in earlier versions.
Fixes#7583
* Fix handling of standalone register bits in circuit
This commit fixes the failing tests by correcting the handling of
standalone register bits that are part of a circuit (without their
register). Previosuly, if the circuit was solely composed of register
bits but the circuit didn't contain that register, qpy deserialization
would incorrectly add the register to the circuit. This corrects this by
having the deserialization function treat a register that's not in
circuit in the same manner as if it were an out of order register and
add each bit one at a time.
* Add test for for loop with iterator
* Add backwards compat tests for control flow
* Finish release notes
* Fix docs build
* Apply suggestions from code review
Co-authored-by: Jake Lishman <jake@binhbar.com>
* Add handling for ContinueLoopOp too
* Cleanup note about register index array type change
* Use unique doc reference names for qpy module
* Update qpy version 3 ref in release note
* Switch tuple to support any qpy type instead of just ints
This commit changes the tuple formatting to work with any parameter type
that qpy currently supports, not just ints which is what for loop
currently uses a tuple for. This should provide some future proofing as
it enables tuples of other types, which is being considered in the
future, to work without modification.
* Update another broken sphinx ref
* Fix last dangling broken sphinx ref
* Fix yet another dangling sphinx ref
* Fix failing test
* Set fixed Python hash seed in qpy compat tests
This commit updates the qpy compat test runner script to set a fixed
hash seed. The control flow builders internally use sets which means
that to get consistent ordering between python processes we need to
ensure the hash seeds are the same. Without this the argument order
might differ between creating the circuit for qpy serialization and
reconstructing the circuit in another process by deserializing that
qpy data. Setting a fixed hash seed between processes ensures this type
of failure won't happen as the set order will be consistent between
multiple runs of Python.
* Update qiskit/circuit/qpy_serialization.py
Co-authored-by: Jake Lishman <jake@binhbar.com>
* Fix docstring for TUPLE payload
This commit fixes an oversight in earlier commits where the
documentation for the TUPLE payload wasn't actually finished wasn't
really coherent. This commit finishes the documentation to explain how a
tuple is serialized in qpy.
* Add test case using nested tuples
* Directly construct WhileLoopOp in qpy compat tests
In an earlier commit we set a fixed (but still randomly generated) python
hash seed to force multiple invocations of python to use the same hash
seed. This was done in order to fix a non-deterministic failure we were
seeing with the while loop circuit. The while loop context builder uses
a set internally and is sensitive to argument ordering. However, just
hash seed randomization doesn't fix the issue in practice because the
difference isn't between set order strictly, but also that set order
matches the circuit insertion order for what gets written in QPY. To
address this and make the tests work stably this commit switches away
from using the builder interface and instead manually constructs the
while loop and appends it to the circuit with a fixed argument order.
This should hopefully make the compat tests run consistently and fix the
occasional failures we're seeing in CI.
* Fix lint
* Add test case for serializing an empty tuple instruction parameter
* Avoid builder interface for qpy compat tests
Just as was changed for the while loop generator function in the qpy
compat tests this commit updates the other control flow example circuits
to not use the builder interface. The same non-determinism in the
ordering of arguments applies to all the control flow instructions. So
to avoid spurious failures we should explicitly set a arg order and not
use the builder interface.
Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* Fix QuantumInstance compatibility with BackendV2
This commit fixes compatibility with the QuantumInstance class when
targetting a BackendV2 backend. When BackendV2 was introduced testing
with the QuantumInstance (and therefore all of qiskit.algorithms) was
neglected and there are several compatibility issues with the class
around hard coded assumptions that the backend being wrapped was a
BaseBackend or BackendV1 object.
* Remove invalid test
The tests for the QuantumInstance with BackendV2 were just ported over
from the equivalent testing with BackendV1. However, one of those tests
was to test error mitigation with a custom noise model which requires
qiskit-aer. We can't test this without aer having been migrated to
BackendV2. Until qiskit-aer is backendv2 enabled this commit just
removes the test.
* Remove unused imports
* Remove duplicate backend version checks from QuantumInstance
The QuantumInstance's is_simulator() and is_local() methods had
duplicate checking on the backend version. This was an artifact of
testing where in earlier revisions of this PR branch it was done in the
QuantumInstance before being moved to the inner helper functions. Since
the helper functions are updated to handle the version checking now we
no longer need it in the QuantumInstance methods too. This commit
removes this and just relies on the helper functions to do the version
checking.
* Use more descriptive local variable name for backend version
This commit attempts to clarify how the version fields are used by
naming the local variables backend_interface_version to make it clear
what we're checking in that context.
* Fix handling of statevector simulator check for BackendV2
This commit fixes the check for whether we're running with a statevector
simulator (to determine whether we need to insert save_statevector
instructions) to be BackendV2 compatible. This isn't strictly needed yet
as qiskit-aer is still based on BackendV1, but this would cause a bug
when it does switch to the newer backend version. This commit just gets
in front of that potential issue so we don't have a bug in the future.
* Use backend_interface_version everywhere
There were a few straglers still using backend_version instead of
backend_interface_version in the code. This commit updates all these
instances so there is consistent naming around this inside the utils
subpackage.
* Fix handling of statevector simulator detection
This commit fixes an oversight from an earlier development version that
slipped through later revisions where the statevector simulator backend
detection in the QuantumInstance class was hard coded to False for
BackendV2. This was just meant as a temporary debugging step while
getting the initial commit ready, but was never updated to remove this
until now.
* Actually commit local variable backend_interface_version renames
* Fix typo
Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
* fix cvar for paulisumop
* comments from code review
- fix pylint in _is_diagonal
- use elif in _is_diagonal to avoid unnecessary iterations
- fix reno
- add assertion to test
* cleanup test properly
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
dependabot[bot]