* Initial: implement fixed capacity constructor for DAGCircuit
- Implement `DAGCircuit` with `with_capacity` to create an initially allocated instance, for rust only.
- Implement `with_capacity` for `BitData` and `IndexInterner` that creates an initally allocated instance of each type.
- Other small tweaks and fixes.
- Add num_edges optional argument. If known, use `num_edges` argument to create a `DAGCircuit` with the exact number of edges.
- Leverage usage of new method in `copy_empty_like`.
- Use `..Default::default()` for `op_names` and `calibrations` fields in `DAGCircuit::with_capacity()`
* Fix: Adapt to #13033
* Fix: Re-arrange the function arguments as per @mtreinish's review.
- The order now follows: qubits, clbits, vars, num_ops, edges. As per [Matthew's comment](https://github.com/Qiskit/qiskit/pull/12975#discussion_r1742426093).
The default value for `Instruction.unit` is `"dt"`. Previously, the
`OperationFromPython` extraction logic would only suppress allocation of
the extra instruction attributes if all the contained fields were
`None`, but `None` is not actually a valid value of `Instruction.unit`
(which must be a string). This meant that `OperationFromPython` would
always allocate and store extra attributes, even for the default cases.
This did not affect standard gates appended using their corresponding
`QuantumCircuit` methods (since no Python-space extraction is
performed in that case), but did affect standard calls to `append`, or
anything else that entered from Python space.
This drastically reduces the memory usage of circuits built by
`append`-like methods. Ignoring the inefficiency factor of the
heap-allocation implementation, this saves 66 bytes plus
small-allocation overhead for 2-byte heap allocations (another 14 bytes
on macOS, but will vary depending on the allocator) per standard
instruction, which is on the order of 40% memory-usage reduction.
This commit removes benchmarks for the deprecated assemble module and
the soon to be (see #13063) deprecated pulse module. While these
features are still supported it's not worth the spending the limited
CPU time we hacve on the benchmarking system to track the performance of
these components anymore.
The `mutable` check in the controlled-gate `OperationFromPython`
extraction logic to check for a mutated `base_gate` was overzealous,
and would return false positives for parametric controlled gates. The
only modification to `base_gate` of a standard-library gate that would
not have caused data-model problems from Python space would be setting
the base-gate label, which is used for a public feature of the circuit
visualisers.
The change to `get_standard_gate_name_mapping` is just a minor
convenience to make the gate objects directly appendable to a circuit;
previously, each `Parameter` object was distinct and had a UUID clash
with others of the same name, so could not be used together. The new
behaviour is purely a convenience for tests; it largely should not be
useful for users to directly append these gates.
* init
* up
* lint
* .
* up
* before cache
* with cache
* correct
* cleaned up
* lint reno
* Update Cargo.lock
* .
* up
* .
* revert op
* .
* .
* .
* .
* Delete Cargo.lock
* .
* corrected string comparison
* removed Operator class from operation.rs
* .
* Apply suggestions from code review
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* comments from code review
* Apply suggestions from code review
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* code review
* remove with_gil in favor of passing python tokens as params
* Apply suggestions from code review
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* fmt
* python serialization
* deprecation
* Update commutation_checker.py
* heh
* let Pytuple collect
* lint
* First set of comments
- use Qubit/Clbit
- more info on unsafe
- update reno
- use LazySet less
- use OperationRef, avoid CircuitInstruction creation
* Second part
- clippy
- no BigInt
- more comments
* Matrix speed & fix string sort
-- could not use op.name() directly since sorted differently than Python, hence it's back to BigInt
* have the Python implementation use Rust
* lint & tools
* remove unsafe blocks
* One more try to avoid segfaulty windows
-- if that doesn't work maybe revert the change the the Py CommChecker uses Rust
* Debug: disable cache
trying to figure out why the windows CI fails (after being unable to locally reproduce we're using CI with a reduced set of tests)
* ... second try
* Update crates/accelerate/src/commutation_checker.rs
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* Restore azure config
* Remove unused import
* Revert "Debug: disable cache"
This reverts commit c564b806c9.
* Don't overallocate cache
We were allocating a the cache hashmap with a capacity for max cache
size entries every time we instantiated a new CommutationChecker. The
max cache size is 1 million. This meant we were allocating 162MB
everytime CommutationChecker.__new__ was called, which includes each
time we instantiate it manually (which happens once on import), the
CommutationAnalysis pass gets instantiated (twice per preset pass
manager created with level 2 or 3), or a commutation checker instance is
pickle deserialized. This ends up causing a fairly large memory
regression and is the source of the CI failures on windows.
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
* Cleanup parameter key type to handle edge conditions better
This commit cleans up the ParameterKey type and usage to make it handle
edge conditions better. The first is that the type just doesn't do the
right thing for NaN, -0, or the infinities. Canonicalization is added
for hash on -0 and the only constructor of the newtype adds a runtime
guard against NaN and inifinity (positive or negative) to avoid that
issue. The approach only makes sense as the cache is really there to
guard us against unnecessary re-computing when we reiterate over the
circuit > 1 time and nothing has changed for gates. Otherwise comparing
floats like done in this PR does would not be a sound or an effective
approach.
* Remove unnecessary cache hit rate tracking
* Undo test assertion changes
* Undo unrelated test changes
* Undo pending deprecation and unify commutation classes
This commit removes the pending deprecation decorator from the python
class definition as the Python class just internally is using the rust
implementation now. This also removes directly using the rust
implementation for the standard commutation library global as using the
python class is exactly the same now.
We can revisit if there is anything we want to deprecate and remove in
2.0 in a follow up PR. Personally, I think the cache management methods
are all we really want to remove as the cache should be an internal
implementation detail and not part of the public interface.
* Undo gha config changes
* Make serialization explicit
This commit makes the pickling of cache entries explicit. Previously it
was relying on conversion traits which hid some of the complexity but
this uses a pair of conversion functions instead.
* Remove stray SAFETY comment
* Remove ddt usage from the tests
Now that the python commutation checker and the rust commutation checker
are the same thing the ddt parameterization of the commutation checker
tests was unecessary duplication. This commit removes the ddt usage to
restore having a single run of all the tests.
* Update release note
* Fix CommutationChecker class import
* Remove invalid test assertion for no longer public attribute
* Ray's review comments
Co-authored-by: Raynel Sanchez <raynelfss@hotmail.com>
* Handle ``atol/rtol``, more error propagation
* re-use expensive quantities
such as the relative placement and the parameter hash
---------
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Julien Gacon <jules.gacon@googlemail.com>
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: Raynel Sanchez <raynelfss@hotmail.com>
* Added a get_entangler_map() and _selective_entangler_map() functions to PauliFeatureMap
* Added some logic to handle the List[List[List[List[int]]]] case. Refer to the comments in the code.
* `entanglement` can now be specified as dictionary `{num_qubits: entanglement}`
* Added logic to handle cases when entanglement is not specified for some pauli blocks.
* Added tests cases for `PauliFeatureMap`
* Added test and releasenotes. Also modified the typing hints and doc strings to reflect new changes.
* Replaced all tab characters with spaces to fix the linting error.
* Formatted releasenotes as per the suggestion to fix the issue in reno.
* Added `Callable[[int], Union[str | Dict[...]]]` to the typing hints and included some logic to handle that.
* Removed `List[List[int]]` from typing hints as this is equivalent to List[Tuple[int]]
Co-authored-by: Julien Gacon <gaconju@gmail.com>
* updated the release notes
Co-authored-by: Julien Gacon <gaconju@gmail.com>
* Updated typing hints in `ZZFeatureMap`
---------
Co-authored-by: Julien Gacon <gaconju@gmail.com>
* Fully port Optimize1qGatesDecomposition to Rust
This commit builds off of #12550 and the other data model in Rust
infrastructure and migrates the Optimize1qGatesDecomposition pass to
operate fully in Rust. The full path of the transpiler pass now never
leaves rust until it has finished modifying the DAGCircuit. There is
still some python interaction necessary to handle parts of the data
model that are still in Python, mainly calibrations and parameter
expressions (for global phase). But otherwise the entirety of the pass
operates in rust now.
This is just a first pass at the migration here, it moves the pass to be
a single for loop in rust. The next steps here are to look at operating
the pass in parallel. There is no data dependency between the
optimizations being done by the pass so we should be able to the
throughput of the pass by leveraging multithreading to handle each run
in parallel. This commit does not attempt this though, because of the
Python dependency and also the data structures around gates and the
dag aren't really setup for multithreading yet and there likely will
need to be some work to support that (this pass is a good candidate to
work through the bugs on that).
Part of #12208
* Tweak control_flow_op_nodes() method to avoid dag traversal when not necessary
* Store target basis set without heap allocation
Since we only are storing 12 enum fields (which are a single byte) using
any heap allocated collection is completely overkill and will have more
overhead that storing a statically sized array for all 12 variants. This
commit adds a new struct that wraps a `[bool; 12]` to track which
basis are supported and an API for tracking this. This simplifies the
tracking of which qubit supports which EulerBasis, it also means other
internal users of the 1q decomposition have a simplified API for working
with the euler basis.
* Remove From trait for Qubit->PhysicalQubit conversion
* Fix merge conflict
* Use new DAGCircuit::has_control_flow() for control_flow_op_nodes() pymethod
* Move _basis_gates set creation to __init__
* Update releasenotes/notes/optimize-1q-gates-decomposition-ce111961b6782ee0.yaml
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
---------
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
* Add no-hash-lookup way to retrieve the default interned value
This makes it possible to do `Interner::get_default()` without any
value, in order to retrieve a key pointing to the default allocation
without making any hash lookups. While the hashing and equality check
of the default allocation is typically very cheap (like the empty
slice), acquiring it still generally required a function call, which
often needed to be paid frequently.
* Add symmetric method on `Interned`
* Tweak documentation wording
Co-authored-by: Kevin Hartman <kevin@hart.mn>
* Re-remove `Interned::of_default` following review
---------
Co-authored-by: Kevin Hartman <kevin@hart.mn>
We needed this back when `setuptools` first introduced the new editable
installations, but at this point it should work more correctly without
it; our non-CI configurations haven't included it for some time.
* Initial: Add `assign_parameter` methods that can be used from Rust.
* Add: Error message in `from_slice` method.
- Accept owned mapping for `from_mapping`.
* Fix: Use `assign_parameters_from_slice` as inner method for `assign_parameters_sequence`
* Fix: Use `ParameterUuid` as keys for `assign_parameter_from_mapping`
* Fix: Accept Param references in `assign_parameters_inner`
* Fix: Ownership issues
- Remove initial allocation of `Param` instances within a vec in `assign_parameters_iterable`.
- Revert changes in `assign_parameters_mapping`.
- Fix error message in `assign_parameter_from_slice`.
- Return an error if a `uuid` is not found in `assign_parameters from mapping. Also, return error if a valid uuid is not found to avoid panicking.
- Add `clone_ref(py)` method to `Param` to ensure that we use the most efficient cloning methods (either `clone_ref` with gil or copying).
- Implement trait `AsRef<Param>` for `Param` to be able to send both owned and non-owned instances to `assign_parameters_inner`.
* Docs: Add comment on `AsRef<Param>` impl block.
This is generally useful for Qiskit's built-in transpiler passes, and
we've had some interest in using it for the same reasons from outside
the library. While the control-flow structure from within Rust might
change in the future, in the immediate term and from Python space, it
feels fair to expose this as public API, since all the information
contained in it is certainly already public.
* Avoid isinstance check on every node in DAG during
recursive depth and size calls.
* Avoid unnecessary downcast.
* Check if there is control flow before doing loops.
* Avoid py_op_nodes in DAGCircuit::count_ops.
* Move interners from trait to generic structs
This rewrites the interner mechanisms be defined in terms of two generic
structs (`Interner` and `Interned`) that formalise the idea that the
interned values are owned versions of a given reference type, and move
the logically separate intern-related actions (get a key from the value,
and get the value from the key) into separate functions (`get` and
`insert`, respectively).
This has a few advantages:
1. we now have both `insert` and `insert_owned` methods, which was
awkward to add within the trait-based structure. This allows a more
efficient path when the owned variant has already necessarily been
constructed.
2. additionally, the standard `insert` path now takes only a reference
type. For large circuits, most intern lookups will, in general, find
a pre-existing key, so in situations where the interned value is
sufficiently small that it can be within a static allocation (which
is the case for almost all `qargs` and `cargs`), it's more efficient
not to construct the owned type on the heap.
3. the type of the values retrieved from an interner are no longer
indirected through the owned type that's stored. For example,
where `IndexedInterner<Vec<Qubit>>` gave out `&Vec<Qubit>`s as its
lookups, `Interner<[Qubit]>` returns the more standard `&[Qubit]`,
which is only singly indirect rather than double.
The following replacements are made:
1. The `IndexedInterner` struct from before is now just called
`Interner` (and internally, it uses an `IndexSet` rather than
manually tracking the indices). Its generic type is related to the
references it returns, rather than the owned value it stores, so
`IndexedInterner<Vec<Qubit>>` becomes `Interner<[Qubit]>`.
2. The `Interner` trait is now gone. Everything is just accessed as
concrete methods on the `Interner` struct.
3. `<&IndexedInterner as Interner>::intern` (lookup of the value from an
interner key) is now called `Interner::get`.
4. `<&mut IndexedInterner as Interner>::intern` (conversion of an owned
value to an interner key) is now called `Interner::insert_owned`.
5. A new method, `Interner::insert`, can now be used when one need not
have an owned allocation of the storage type; the correct value will
be allocated if required (which is expected to be less frequent).
6. The intern key is no longer called `interner::Index`, but
`Interned<T>`, where the generic parameter `T` matches the generic of
the `Interner<T>` that gave out the key.
* Improve internal documentation
* Match names of interners between `CircuitData` and` DAGCircuit`
* Initial: Expose `CircuitData` interners and registers to the `qiskit-circuit` crate.
- Make the `CircuitData` iter method be an exact-size iterator.
* FIx: Expose immutable views of interners, registers and global phase.
- Revert the changes making the interners and registers visible to the crate `qiskit-circuit`.
- Create methods to expose immutable borrowed views of the interners, registers and global_phase to prevent from mutating the DAGCircuit.
- Add `get_qargs` and `get_cargs` to unpack interned qargs ans cargs.
- Other tweaks and fixes.
* Format: Fix typo in comment
Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>
* Refactor: Use naming convention for getters.
* Docs: Apply suggestions from code review
- Correct incorrect docstrings for `qubits()` and `clbits()`
Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>
---------
Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>
* fix: deal with special case for `UnitaryGate`
* add test
* add release note
* typo
* generalize the fix to other gates
* fix seeds
* update unittests
* Add missing unittests
---------
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
* Port DAGCircuit to Rust
This commit migrates the entirety of the `DAGCircuit` class to Rust. It
fully replaces the Python version of the class. The primary advantage
of this migration is moving from a Python space rustworkx directed graph
representation to a Rust space petgraph (the upstream library for
rustworkx) directed graph. Moving the graph data structure to rust
enables us to directly interact with the DAG directly from transpiler
passes in Rust in the future. This will enable a significant speed-up in
those transpiler passes. Additionally, this should also improve the
memory footprint as the DAGCircuit no longer stores `DAGNode`
instances, and instead stores a lighter enum NodeType, which simply
contains a `PackedInstruction` or the wire objects directly.
Internally, the new Rust-based `DAGCircuit` uses a `petgraph::StableGraph`
with node weights of type `NodeType` and edge weights of type `Wire`. The
NodeType enum contains variants for `QubitIn`, `QubitOut`, `ClbitIn`,
`ClbitOut`, and `Operation`, which should save us from all of the
`isinstance` checking previously needed when working with `DAGNode` Python
instances. The `Wire` enum contains variants `Qubit`, `Clbit`, and `Var`.
As the full Qiskit data model is not rust-native at this point while
all the class code in the `DAGCircuit` exists in Rust now, there are
still sections that rely on Python or actively run Python code via Rust
to function. These typically involve anything that uses `condition`,
control flow, classical vars, calibrations, bit/register manipulation,
etc. In the future as we either migrate this functionality to Rust or
deprecate and remove it this can be updated in place to avoid the use
of Python.
API access from Python-space remains in terms of `DAGNode` instances to
maintain API compatibility with the Python implementation. However,
internally, we convert to and deal in terms of NodeType. When the user
requests a particular node via lookup or iteration, we inflate an ephemeral
`DAGNode` based on the internal `NodeType` and give them that. This is very
similar to what was done in #10827 when porting CircuitData to Rust.
As part of this porting there are a few small differences to keep in
mind with the new Rust implementation of DAGCircuit. The first is that
the topological ordering is slightly different with the new DAGCircuit.
Previously, the Python version of `DAGCircuit` using a lexicographical
topological sort key which was basically `"0,1,0,2"` where the first
`0,1` are qargs on qubit indices `0,1` for nodes and `0,2` are cargs
on clbit indices `0,2`. However, the sort key has now changed to be
`(&[Qubit(0), Qubit(1)], &[Clbit(0), Clbit(2)])` in rust in this case
which for the most part should behave identically, but there are some
edge cases that will appear where the sort order is different. It will
always be a valid topological ordering as the lexicographical key is
used as a tie breaker when generating a topological sort. But if you're
relaying on the exact same sort order there will be differences after
this PR. The second is that a lot of undocumented functionality in the
DAGCircuit which previously worked because of Python's implicit support
for interacting with data structures is no longer functional. For
example, previously the `DAGCircuit.qubits` list could be set directly
(as the circuit visualizers previously did), but this was never
documented as supported (and would corrupt the DAGCircuit). Any
functionality like this we'd have to explicit include in the Rust
implementation and as they were not included in the documented public
API this PR opted to remove the vast majority of this type of
functionality.
The last related thing might require future work to mitigate is that
this PR breaks the linkage between `DAGNode` and the underlying
`DAGCirucit` object. In the Python implementation the `DAGNode` objects
were stored directly in the `DAGCircuit` and when an API method returned
a `DAGNode` from the DAG it was a shared reference to the underlying
object in the `DAGCircuit`. This meant if you mutated the `DAGNode` it
would be reflected in the `DAGCircuit`. This was not always a sound
usage of the API as the `DAGCircuit` was implicitly caching many
attributes of the DAG and you should always be using the `DAGCircuit`
API to mutate any nodes to prevent any corruption of the `DAGCircuit`.
However, now as the underlying data store for nodes in the DAG are
no longer the python space objects returned by `DAGCircuit` methods
mutating a `DAGNode` will not make any change in the underlying
`DAGCircuit`. This can come as quite the surprise at first, especially
if you were relying on this side effect, even if it was unsound.
It's also worth noting that 2 large pieces of functionality from
rustworkx are included in this PR. These are the new files
`rustworkx_core_vnext` and `dot_utils` which are rustworkx's VF2
implementation and its dot file generation. As there was not a rust
interface exposed for this functionality from rustworkx-core there was
no way to use these functions in rustworkx. Until these interfaces
added to rustworkx-core in future releases we'll have to keep these
local copies. The vf2 implementation is in progress in
Qiskit/rustworkx#1235, but `dot_utils` might make sense to keep around
longer term as it is slightly modified from the upstream rustworkx
implementation to directly interface with `DAGCircuit` instead of a
generic graph.
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>
Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>
Co-authored-by: John Lapeyre <jlapeyre@users.noreply.github.com>
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
* Update visual mpl circuit drawer references
Right now there is a bug in the matplotlib circuit visualizer likely
caused by the new `__eq__` implementation for `DAGOpNode` that didn't
exist before were some gates are missing from the visualization. In the
interest of unblocking this PR this commit updates the references for
these cases temporarily until this issue is fixed.
* Ensure DAGNode.sort_key is always a string
Previously the sort_key attribute of the Python space DAGCircuit was
incorrectly being set to `None` for rust generated node objects. This
was done as for the default path the sort key is determined from the
rust domain's representation of qubits and there is no analogous data in
the Python object. However, this was indavertandly a breaking API change
as sort_key is expected to always be a string. This commit adds a
default string to use for all node types so that we always have a
reasonable value that matches the typing of the class. A future step is
likely to add back the `dag` kwarg to the node types and generate the
string on the fly from the rust space data.
* Make Python argument first in Param::eq and Param::is_close
The standard function signature convention for functions that take a
`py: Python` argument is to make the Python argument the first (or
second after `&self`). The `Param::eq` and `Param::is_close` methods
were not following this convention and had `py` as a later argument in
the signature. This commit corrects the oversight.
* Fix merge conflict with #12943
With the recent merge with main we pulled in #12943 which conflicted
with the rust space API changes made in this PR branch. This commit
updates the usage to conform with the new interface introduced in this
PR.
* Add release notes and test for invalid args on apply methods
This commit adds several release notes to document this change. This
includes a feature note to describe the high level change and the user
facing benefit (mainly reduced memory consumption for DAGCircuits),
two upgrade notes to document the differences with shared references
caused by the new data structure, and a fix note documenting the fix
for how qargs and cargs are handled on `.apply_operation_back()` and
`.apply_operation_front()`. Along with the fix note a new unit test is
added to serve as a regression test so that we don't accidentally allow
adding cargs as qargs and vice versa in the future.
* Restore `inplace` argument functionality for substitute_node()
This commit restores the functionality of the `inplace` argument for
`substitute_node()` and restores the tests validating the object
identity when using the flag. This flag was originally excluded from
the implementation because the Rust representation of the dag is not
a shared reference with Python space and the flag doesn't really mean
the same thing as there is always a second copy of the data for Python
space now. The implementation here is cheating slighty as we're passed
in the DAG node by reference it relies on that reference to update the
input node at the same time we update the dag. Unlike the previous
Python implementation where we were updating the node in place and the
`inplace` argument was slightly faster because everything was done by
reference. The rust space data is still a compressed copy of the data
we return to Python so the `inplace` flag will be slightly more
inefficient as we need to copy to update the Python space representation
in addition to the rust version.
* Revert needless dict() cast on metadata in dag_to_circuit()
This commit removes an unecessary `dict()` cast on the `dag.metadata`
when setting it on `QuantumCircuit.metadata` in
`qiskit.converters.dag_to_circuit()`. This slipped in at some point
during the development of this PR and it's not clear why, but it isn't
needed so this removes it.
* Add code comment for DAGOpNode.__eq__ parameter checking
This commit adds a small inline code comment to make it clear why we
skip parameter comparisons in DAGOpNode.__eq__ for python ops. It might
not be clear why the value is hard coded to `true` in this case, as this
check is done via Python so we don't need to duplicate it in rust space.
* Raise a ValueError on DAGNode creation with invalid index
This commit adds error checking to the DAGNode constructor to raise a
PyValueError if the input index is not valid (any index < -1).
Previously this would have panicked instead of raising a user catchable
error.
* Use macro argument to set python getter/setter name
This commit updates the function names for `get__node_id` and
`set__node_id` method to use a name that clippy is happy with and
leverage the pyo3 macros to set the python space name correctly instead
of using the implicit naming rules.
* Remove Ord and PartialOrd derives from interner::Index
The Ord and PartialOrd traits were originally added to the Index struct
so they could be used for the sort key in lexicographical topological
sorting. However, that approach was abandonded during the development of
this PR and instead the expanded Qubit and Clbit indices were used
instead. This left the ordering traits as unnecessary on Index and
potentially misleading. This commit just opts to remove them as they're
not needed anymore.
* Fix missing nodes in matplotlib drawer.
Previously, the change in equality for DAGNodes was causing nodes
to clobber eachother in the matplotlib drawer's tracking data
structures when used as keys to maps.
To fix this, we ensure that all nodes have a unique ID across
layers before constructing the matplotlib drawer. They actually
of course _do_ in the original DAG, but we don't really care
what the original IDs are, so we just make them up.
Writing to _node_id on a DAGNode may seem odd, but it exists
in the old Python API (prior to being ported to Rust) and
doesn't actually mutate the DAG at all since DAGNodes are
ephemeral.
* Revert "Update visual mpl circuit drawer references"
With the previous commit the bug in the matplotlib drawer causing the
images to diverge should be fixed. This commit reverts the change to the
reference images as there should be no difference now.
This reverts commit 1e4e6f3862.
* Update visual mpl circuit drawer references for control flow circuits
The earlier commit that "fixed" the drawers corrected the visualization
to match expectations in most cases. However after restoring the
references to what's on main several comparison tests with control flow
in the circuit were still failing. The failure mode looks similar to the
other cases, but across control flow blocks instead of at the circuit
level. This commit temporarily updates the references of these to the
state of what is generated currently to unblock CI. If/when we have a
fix this commit can be reverted.
* Fix edge cases in DAGOpNode.__eq__
This commit fixes a couple of edge cases in DAGOpNode.__eq__ method
around the python interaction for the method. The first is that in
the case where we had python object parameter types for the gates we
weren't comparing them at all. This is fixed so we use python object
equality for the params in this case. Then we were dropping the error
handling in the case of using python for equality, this fixes it to
return the error to users if the equality check fails. Finally a comment
is added to explain the expected use case for `DAGOpNode.__eq__` and why
parameter checking is more strict than elsewhere.
* Remove Param::add() for global phase addition
This commit removes the Param::add() method and instead adds a local
private function to the `dag_circuit` module for doing global phase
addition. Previously the `Param::add()` method was used solely for
adding global phase in `DAGCircuit` and it took some shortcuts knowing
that context. This made the method implementation ill suited as a
general implementation.
* More complete fix for matplotlib drawer.
* Revert "Update visual mpl circuit drawer references for control flow circuits"
This reverts commit 9a6f9536a3.
* Unify rayon versions in workspace
* Remove unused _GLOBAL_NID.
* Use global monotonic ID counter for ids in drawer
The fundamental issue with matplotlib visualizations of control flow is
that locally in the control flow block the nodes look the same but are
stored in an outer circuit dictionary. If the gates are the same and on
the same qubits and happen to have the same node id inside the different
control flow blocks the drawer would think it's already drawn the node
and skip it incorrectly. The previous fix for this didn't go far enough
because it wasn't accounting for the recursive execution of the drawer
for inner blocks (it also didn't account for LayerSpoolers of the same
length).
* Re-add missing documentation
* Remove unused BitData iterator stuff.
* Make types, dag, and bit count methods public
This commit makes some attributes of the dag circuit public as they will
need to be accessible from the accelerate crate to realistically start
using the DAGCircuit for rust transpiler passes.
* Make Wire pickle serialization explicit
This commit pivots away from using the PyO3 crate's conversion traits
for specialized pickle serialization output of Wire objects. The output
of the previous traits wasn't really intended for representing a Wire in
Python but only for pickle serialization. This commit migrates these to
custom methods, without a trait, to make it clear they're only for
pickle.
* Make py token usage explicit in _VarIndexMap
The _VarIndexMap type was designed to look like an IndexMap but is
actually an inner python dictionary. This is because `Var` types are still
defined in python and we need to use a dictionary if we have `Var`
objects as keys in the mapping. In the interest of looking like an
IndexMap all the methods (except for 1) used `with_gil` internally to
work with the dictionary. This could add unecessary overhead and to make
it explicit that there is python involvement with this struct's methods
this commit adds a py: Python argument to all the methods and removes
the `with_gil` usage.
* Make all pub(crate) visibility pub
* Remove unused method
* Reorganize code structure around PyVariableMapper and BitLocations
* Add missing var wires to .get_wires() method
In the porting of the get_wires() method to Rust the handling of Var
wires was missed in the output of the method. This commit corrects the
oversight and adds them to the output.
* Raise TypeError not ValueError for invalid input to set_global_phase
* De-duplicate check logic for op node adding methods
The methods for checking the input was valid on apply_operation_back,
apply_operation_front, and _apply_op_node_back were all identical. This
combines them into a single method to deduplicate the code.
* Improve collect_1q_runs() filter function
The filter function for collect_1q_runs() was needlessly building a
matrix for all the standard gates when all we need to know in that case
is whether the standard gate is parameterized or not. If it's not then
we're guaranteed to have a matrix available. This commit updates the
filter logic to account for this and improve it's throughput on standard
gates.
* Use swap_remove instead of shift_remove
* Combine input and output maps into single mapping
This commit combines the `DAGCircuit` `qubit_input_map` and
`qubit_output_map` fields into a single `IndexMap` `qubit_io_map` (and
the same for `clbit_input_map` and `clbit_output_map` going to
`clbit_io_map`). That stores the input and output as 2 element array
where the first element is the input node index and the second element
is the output node index. This reduces the number of lookups we need to
do in practice and also reduces the memory overhead of `DAGCircuit`.
* Ensure we account for clbits in depth() short circuit check
* Also account for Vars in DAGCircuit.width()
The number of vars should be included in the return from the width()
method. This was previously missing in the new implementation of this
method.
* Remove duplicated _get_node() method
The `_get_node()` method was duplicated with the already public
`node()` method. This commit removes the duplicate and updates it's only
usage in the code base.
* Handle Var wires in classical_predecessors
This method was missing the handling for var wires, this commit corrects
the oversight.
* Remove stray comment
* Use Operation::control_flow() instead of isinstance checking
* Use &str for increment_op and decrement_op
This commit reworks the interface for the increment_op and decrement_op
methods to work by reference instead of passing owned String objects to
the methods. Using owned String objects was resulting in unecessary
allocations and extra overhead that could be avoided. There are still a
few places we needed to copy strings to ensure we're not mutating things
while we have references to nodes in the dag, typically only in the
decrement/removal case. But this commit reduces the number of String
copies we need to make in the DAGCircuit.
* Also include vars in depth short circuit
* Fix typing for controlflow name lookup in count_ops
* Fix .properties() method to include operations field
The .properties() method should have included the output of .count_ops()
in its dictionary return but this was commented out temporarily while
other pieces of this PR were fixed. This commit circles back to it and
adds the missing field from the output.
As an aside we should probably deprecate the .properties() method for
removal in 2.0 it doesn't seem to be the most useful method in practice.
* Add missing Var wire handling to py_nodes_on_wire
* Add back optimization to avoid isinstance in op_nodes
This commit adds back an optimization to the op_nodes dag method to
avoid doing a python space op comparison when we're filtering on
non-standard gates and evaluating a standard gate. In these cases we
know that the filter will not match purely from rust without needing a
python space op object creation or an isinstance call so we can avoid
the overhead of doing that.
* Simplify/deduplicate __eq__ method
This commit reworks the logic in the DAGCircuit.__eq__ method
implementation to simplify the code a bit and make it less verbose and
duplicated.
* Invalidate cached py op when needed in substitute_node_with_dag
This commit fixes a potential issue in substitute_node_with_dag() when
the propagate_condition flag was set we were not invalidating cached py
ops when adding a new condition based on a propagated condition. This
could potentially cause the incorrect object to be returned to Python
after calling this method. This fixes the issues by clearing the cached
node so that when returning the op to python we are regenerating the
python object.
* Copy-editing suggestions for release notes
Co-authored-by: John Lapeyre <jlapeyre@users.noreply.github.com>
* Fix and simplify separable_circuits()
This commit fixes and simplifies the separable_circuits() method. At
it's core the method is building a subgraph of the original dag for each
weakly connected component in the dag with a little bit of extra
tracking to make sure the graph is a valid DAGCircuit. Instead of trying
to do this manually this commit updates the method implementation to
leverage the tools petgraph gives us for filtering graphs. This both
fixes a bug identified in review but also simplifies the code.
* Add clbit removal test
* Move to using a Vec<[NodeIndex; 2]> for io maps
This commit migrates the qubit_io_map and clbit_io_map to go from a type
of `IndexMap<Qubit, [NodeIndex; 2], RandomState>` to
`Vec<[NodeIndex; 2]>`. Our qubit indices (represented by the `Qubit`
type) must be a contiguous set for the circuit to be valid, and using an
`IndexMap` for these mappings of bit to input and output nodes only
really improved performance in the removal case, but at the cost of
additional runtime overhead for accessing the data. Since removals are
rare and also inefficient because it needs to reindex the entire dag
already we should instead optimize for the accessing the data. Since we
have contiguous indices using a Vec is a natural structure to represent
this mapping.
* Make add_clbits() signature the same as add_qubits()
At some point during the development of this PR the function signatures
between `add_qubits()` and `add_clbits()` diverged between taking a
`Vec<Bound<PyAny>>` and `&Bound<PySequence>`. In general they're are
comprable but since we are going to be working with a `Vec<>` in the
function body this is a better choice to let PyO3 worry about the
conversion for us. Additionally, this is a more natural signature for
rust consumption. This commit just updates `add_clbits()` to use a Vec
too.
* Add attribution comment to num_tensor_factors() method
* Add py argument to add_declared_var()
* Remove unnecessarily Python-space check
* Correct typo in `to_pickle` method
---------
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>
Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>
Co-authored-by: John Lapeyre <jlapeyre@users.noreply.github.com>
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
* Get rid of encoded assumption in TwoQubitBasisDecomposer that the given KAK gate is a Rust-space StandardGate
* Improve unit test
* Update test/python/synthesis/test_synthesis.py
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
---------
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
* moving high-level-synthesis plugins to a separate file
* Adding the remaining MCX synthesis functions and exposting all of them as HLS synthesis plugins.
* adding entry points for MCX plugins
* adding documentation section for MCX plugins
* renaming file
* Adding pending deprecation warnings
* placeholder for MCX plugin tests
* adding flag pending=True to deprecate
* changing checks from isinstance to name-based: a CCCX gate is called mcx but is not an MCXGate
* futher exposing C3X and C4X synthesis
* updating MCX synthesis functions to avoid returning C3X and C4X gates
For better or for worse, C3X and C4X gates have name 'mcx', and
therefore are caught by mcx plugins. We need to avoid recursion
with HLS calling an MCX-synthesis plugin for a C3X-gate, which
in turns returns a C3X-gate.
* fix compose to append
* renaming synthesized circuits for c3x and for c4x back to 'mcx' to avoid qasm changes
* test qasm fixes
* randomly spotted typo
* fixing how QuantumCircuit.decompose works in the presence of gates_to_decompose
* updating MCX plugins to check isinstance
* fixing test
* pylint fixes
* properly fixing test
* additional tests
* adding new synthesis functions to synthesis docs
* release notes
* docstrings improvements followin review
* Adding refernce to Vale et al paper for the MCXPhase gate implementation
* fixes to deprecation warnings and adding deprecation for get_num_ancilla_qubits
* docstring fixes
* renaming mcphase to v24
* removing unncessary checks
* addressing the rest of review comments
* and of course updating qasm checking after we've slightly changed the decomposition
* yet another renaming
* Update qiskit/circuit/library/standard_gates/x.py
Co-authored-by: Julien Gacon <gaconju@gmail.com>
* Update qiskit/circuit/library/standard_gates/x.py
Co-authored-by: Julien Gacon <gaconju@gmail.com>
* Update releasenotes/notes/add-mcx-plugins-85e5b248692a36db.yaml
Co-authored-by: Julien Gacon <gaconju@gmail.com>
* release notes
* formatting
* fixing docs
* removing references from the first sentence of plugin descriptions
---------
Co-authored-by: Julien Gacon <gaconju@gmail.com>
* Extended the `from_backend` method of `InstructionDurations` to support `GenericBackendV2`
* Simplified the `from_backend` method to allow using `BackendV2`. Added a test and a releasenote.
* Made changes to the releasenote.
* add back files needed in Initialize.gates_to_uncompute()
* add a test for Initialize.gates_to_uncompute() method
* fix a comment
* add release notes
* update gates_to_uncompute such that it will call Isometry
* remove unused imports
* transfer code from StatePreparation to Initialize.gates_to_uncompute
* update code following review
* first attempt to fix issue 12552
* first attempt to fix issue 12552
* fixed issue 12552 and unittest
* formatted and completed 12552
* formatted and completed 12552 documentation
* fixed unit tests 12552
* Update qiskit/transpiler/passes/routing/stochastic_swap.py
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
* linted
* passed all tests, including compiler test
* final linting and unittest passing - hopefully
* Update qiskit/transpiler/passes/routing/stochastic_swap.py
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
* Update releasenotes/notes/deprecate-StochasticSwap-451f46b273602b7b.yaml
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
* added test
* Apply suggestions from code review
---------
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
* Example code for Store function
* update shortened line
* Update qiskit/circuit/__init__.py
---------
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
* absorb `phase` into `coeffs` in `paulis()` setter
* do not mutate input array
* add test that paulis setter absorbs phase
* lint
* If input paulis have phase, mutate and warn
* add release note
* release-note formatting
* add test with `simplify()`
* remove phase-warning from paulis setter
* lint
* remove unused import
* update reno
We were previously using `RefCell` to handle the interior mutability for
the Python-operation caches. This works fine, but has two problems:
- `RefCell<Py<PyAny>>` is two pointers wide
- we had to do heavier dynamic runtime checks for the borrow status on
every access, including simple gets.
This commit moves the caching to instead use `OnceCell`. The internal
tracking for a `OnceCell<T>` is just whether an internal `Option<T>` is
in the `Some` variant, and since `Option<Py<PyAny>>` will use the
null-pointer optimisation for space, this means that
`OnceCell<Py<PyAny>>` is only a single pointer wide. Since it's not
permissible to take out a mutable reference to the interior, there is
also no dynamic runtime checking---just the null-pointer check that the
cell is initialised---so access is faster as well.
We can still clear the cache if we hold a mutable reference to the
`OnceCell`, and since every method that can invalidate the cache also
necessarily changes Rust-space, they certainly require mutable
references to the cell, making it safe.
There is a risk here, though, in `OnceCell::get_or_init`. This function
is not re-entrant; one cannot attempt the initialisation while another
thread is initialising it. Typically this is not an issue, since the
type isn't `Sync`, however PyO3 allows types that are only `Send` and
_not_ `Sync` to be put in `pyclass`es, which Python can then share
between threads. This is usually safe due to the GIL mediating access
to Python-space objects, so there are no data races. However, if the
initialiser passed to `get_or_init` yields control at any point to the
Python interpreter (such as by calling a method on a Python object),
this gives it a chance to block the thread and allow another Python
thread to run. The other thread could then attempt to enter the same
cache initialiser, which is a violation of its contract.
PyO3's `GILOnceCell` can protect against this failure mode, but this is
inconvenient to use for us, because it requires the GIL to be held to
access the value at all, which is problematic during `Clone` operations.
Instead, we make ourselves data-race safe by manually checking for
initialisation, calcuting the cache value ourselves, and then calling
`set` or `get_or_init` passing the already created object. While the
initialiser can still yield to the Python interpreter, the actual
setting of the cell is now atomic in this sense, and thus safe. The
downside is that more than one thread might do the same initialisation
work, but given how comparitively rare the use of Python threads is,
it's better to prioritise memory use and single-Python-threaded access
times than one-off cache population in multiple Python threads.
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
* Allow immutable borrow to access `QuantumCircuit.parameters`
`QuantumCircuit.parameters` is logically a read-only operation on
`QuantumCircuit`. For efficiency in multiple calls to
`assign_parameters`, we actually cache the sort order of the internal
`ParameterTable` on access. This is purely a caching effect, and should
not leak out to users.
The previous implementation took a Rust-space mutable borrow out in
order to (potentially) mutate the cache. This caused problems if
multiple Python threads attempted to call `assign_parameters`
simultaneously; it was possible for one thread to give up the GIL during
its initial call to `CircuitData::copy` (so an immutable borrow was
still live), allowing another thread to continue on to the getter
`CircuitData::get_parameters`, which required a mutable borrow, which
failed due to the paused thread in `copy`.
This moves the cache into a `RefCell`, allowing the parameter getters to
take an immutable borrow as the receiver. We now write the cache out
only if we *can* take the mutable borrow out necessary. This can mean
that other threads will have to repeat the work of re-sorting the
parameters, because their borrows were blocking the saving of the cache,
but this will not cause failures.
The methods on `ParameterTable` that invalidate the cache all require a
mutable borrow on the table itself. This makes it impossible for an
immutable borrow to exist simultaneously on the cache, so these methods
should always succeed to acquire the cache lock to invalidate it.
* Use `RefCell::get_mut` where possible
In several cases, the previous code was using the runtime-checked
`RefCell::borrow_mut` in locations that can be statically proven to be
safe to take the mutable reference. Using the correct function for this
makes the logic clearer (as well as technically removing a small amount
of runtime overhead).
* Use `OnceCell` instead of `RefCell`
`OnceCell` has less runtime checking than `RefCell` (only whether it is
initialised or not, which is an `Option` check), and better represents
the dynamic extensions to the borrow checker that we actually need for
the caching in this method.
All methods that can invalidate the cache all necessarily take `&mut
ParameterTable` already, since they will modify Rust-space data. A
`OnceCell` can be deinitialised through a mutable reference, so this is
fine. The only reason a `&ParameterTable` method would need to mutate
the cache is to create it, which is the allowed set of `OnceCell`
operations.
Raynel Sanchez