Adding `Target` and `InstructionProperties` representations to rust #12292

raynelfss · 2024-04-23T20:34:06Z

Summary

Fixes #12052

~~Currently blocked by #12459~~ thanks @mtreinish!

As we continue moving more parts of the transpiler to Rust, it is important that some of the important data about the devices we're compiling for is easily accessible from the Rust side of things. These commits aim to implement representations of both the Target and InstructionProperties classes, allowing most of their data to live inside of Rust but still be usable in Python.

Details and comments

qiskit._accelerate now has a target module which includes both the BaseTarget and BaseInstructionProperties structs.
Here's a more detailed description of what has changed:

Rust

BaseTarget will include the main data that the Target has in python. This will include all of the publicly available attributes of the existing Target class, except for the following:
- _coupling_graph: of type rustworkx.PyDiGraph
- _instruction_durations: of type InstructionDurations
- _instruction_schedule_map: of type InstructionScheduleMap
Due to these properties are not usable through Rust without using python api's, they were left in python.
- This representation is purely additive through Python and non mutable through Rust.
BaseInstructionProperties represents the main data of `InstructionProperties, that being:
- Error: f64,
- Duratlon: f64

Python:

Target: this is a subclass of BaseTarget and has the missing attributes + functions from the previous version.
- In addition it stores a copy of the _gate_map attribute from the rust side for easy access in python without conversions from rust.
- We ensure that the data from Target._gate_map is identical to the data in super().gate_map by calling the super()'s equivalent function when adding or updating an instruction.
InstructionProperties: is a subclass of BaseInstructionProperties
- The attribute of calibration to store the instruction's calibration object is present in this representation along with it's custom setter and getter.

Possible breaking changes:

- InstructionProperties is now PyO3 class which means it cannot be subclassed by overloading __init__() alone. The new architecture would require users to extend both __new__ and __init__ to add extra properties. These wouldn't necessarily be visible in rust. Fixed!

Broken tests

~~A procedure performed in Aer that attempts to erase the Target's internal data is no longer possible due to the these attributes being hidden from python.~~ Solved by Qiskit/qiskit-aer#2142

Blocks

Open to review!

Any suggestions, comments, feedback, or any request for changes are encouraged and welcome!

- Add `Target` class to test mobility between Rust and Python. - Add `add_instruction` method to test compatibility with instructions.

- Property check caused most cases to panic. - Will be commented out and restored at a later time.

- Instructions property returns all added to the target. - Similar behavior to source.

- Add comments to instruction property. - Use new_bound for new PyDicts.

- Remove redundant transformation of PyObject to PyTuple. - Remove debugging print statement.

- Add `InstructionProperties` class to process properties in rust. - Add `is_instance` and `is_class` to identify certain Python objects. - Modify logic of `add_instruction` to use class check. - Other tweaks and fixes.

- Partial addition from Target.py\ - Introduction of hashable qarg data structure. - Other tweaks and fixes.

- Complete missing procedures in function. - Rename `Qargs` to `HashableVec`. - Make `HashableVec` generic. - Separate `import_from_module_call` into call0 and call1. - Other tweaks and fixes.

- Remove stray print statements. - Other tweaks and fixes.

- Remove all unsafe unwraps

- Update `update_from_instruction_schedule_map to use PyResult and '?' operator. - Use Bound Python objects whenever possible. - Other tweaks and fixes.

- Add temporary _target module for testing. - Remove update_from_instruction_schedule_map function back to python. - Add python properties for all public attributes in rust - Other tweaks and fixes.

- Add identical method `qargs` to obtain the qargs of a target. - Other tweaks and fixes.

- Add function with identical behavior to the original in Target. - Other tweaks and fixes.

- Add target module to qiskit init file. - Remove is_instance method. - Modify set_calibration method in InstructionProperty to leave typechecking to Python. - Change rust Target alias to Target2. - Other tweaks and fixes,

- Fix wrong setters/getters for calibration in InstructionProperty object in rust.

- Add FromPyObject trait to Hashable vec to receive Tuples and transform them directly into this type. - Add operations_for_qargs for Target class in Rust side and Python. - Fix return dict keys for `qargs_for_operation_name`. - Add `timing_constrains` and `operation_from_name` to Python side. - Other tweaks and fixes.

- Fix wrong name for function operation_for_qargs. - Fix missing return value in the python side. - Other tweaks and fixes.

- Make `InstructionProperties` "_calibration" attribute visible. - Removed attribute "calibration", treat as class property. - Other tweaks and fixes

- Port class method to rust and connect to Python wrapper. - Other tweaks and fixes.

- Other tweaks and fixes.

- These changes break current functionality of other functions, butemulate intended behavior better. - Fixes coming soon.

mtreinish

Overall I think this is about ready to merge. I think there will definitely be an interative process where we refine this as we start using it more. But this has all the pieces we need to start and only will cost a bit extra memory for a rust space copy of the properties. I left a few inline comments, but the big things I think we must fix before moving forward here are the docstrings for BaseTarget should describe BaseTarget's functionality and what/how it is to be used. Even though it's a private class and only for Qiskit internals having the docstrings read like the python space target is confusing.

The other big one is I think dropping Mapping from the Python space Target is a breaking api change and we should be able to avoid that, I think metaclass=Mapping or something similar will work to fix that and let you inherit from BaseTarget and still register the class as a Mapping.

mtreinish · 2024-07-12T21:23:41Z

crates/accelerate/src/target_transpiler/mod.rs

+    _gate_name_map: IndexMap<String, TargetOperation, RandomState>,
+    global_operations: IndexMap<u32, HashSet<String>, RandomState>,
+    variable_class_operations: IndexSet<String, RandomState>,
+    qarg_gate_map: NullableIndexMap<Qargs, Option<HashSet<String>>>,
+    non_global_strict_basis: Option<Vec<String>>,
+    non_global_basis: Option<Vec<String>>,


This is more a comment for a future performance optimization to investigate in a follow up PR (we can convert this to a tracking issue after the PR merges)

All the String entries here are all the same set of strings. Ideally we should have only one owned string copy as the canonical name and the others are all just &str references to that single string. We might have issues with lifetimes on this as a pyclass, but we could do something like have a gate_names: Vec<String> and then replace all the strings by a usize index.

crates/accelerate/src/target_transpiler/mod.rs

mtreinish · 2024-07-15T15:13:16Z

crates/accelerate/src/target_transpiler/mod.rs

+    #[pyo3(signature = (instruction, name, properties=None))]
+    fn add_instruction(


Ok, that makes sense. My comment was more about the method name and the docstring identical to the python one. This is really a different method from the python space add_instruction it's a subset of the functionality for handling the rust data but everything is showing it looks like the python space.

crates/accelerate/src/target_transpiler/mod.rs

mtreinish · 2024-07-15T15:24:28Z

crates/accelerate/src/target_transpiler/nullable_index_map.rs

+/// **Warning:** This is an experimental feature and should be used with care as it does not
+/// fully implement all the methods present in `IndexMap<K, V>` due to API limitations.
+#[derive(Debug, Clone)]
+pub struct NullableIndexMap<K, V>


With this warning do we want to make this fully public, or can we constrain this to pub(crate)? This is really only used internally for the Target right, so we may return it to python (but that doesn't matter for this) and we can avoid exposing this directly for accessing the target from rust right? (although all the usage right now is potentially coming from the same crate)

Well, with this one it's a little complicated. Even though it is currently being used only in Target, it might be usable for other things in the future where we have to store a nullable value inside of a dict-like structure. And maybe someone could build on top of it.

However, the main reason why I made it public is that if we call Target.values the value that is returned is an instance of NullableIndexMap and, if we wanted to avoid that, we'd have to reconstruct the IndexMap entirely. Of course, there might be a better way to approach this so I'm curious as to which ideas you might have.

qiskit/transpiler/target.py

- Minor corrections from Matthew's review.

- Due to the private nature of `NullableIndexMap`, the `Target` has to be made crate private. - Add temporary`allow(dead_code)` flag for the unused `Target` and `NullableIndexMap` methods. - Fix docstring of `Target` struct. - Fix docstring of `add_instruction`. - Make several python-only operations public so they can be used with other `PyClass` instances as long as they own the gil. - Modify `py_instruction_supported` to accept bound objects. - Use rust-native functions for some of the instance properties. - Rewrite `instruction` to return parameters as slice. - `operation_names` returns an `ExactSizeIterator`. - All rust-native methods that return an `OperationType` object, will return a `NormalOperation` instance which includes the `OperationType` and the parameters.

mtreinish

I think this is ready to go now. I have a few very small nits inline. But for a 2k line change I think I can continue to look at this and find something forever. so after these are fixed I'll approve and we can merge this. I think there are some potential cleanups we'll want to do down the road, but this is a great start and gets us the data model for the target in rust to start leveraging in various places.

This will conflict with @jakelishman's PR #12730 I think because that removes OperationType but it shouldn't be too hard to update this with that or vice versa if this merges first.

mtreinish · 2024-07-23T17:21:47Z

crates/accelerate/src/target_transpiler/mod.rs

+pub(crate) struct NormalOperation {
+    pub operation: OperationType,
+    pub params: SmallVec<[Param; 3]>,
+    op_object: PyObject,


Isn't this largely duplicative with the other fields? Is there something we gain by storing this too? Is it just so you don't have to construct it on the fly again if a user requests the operation object for a name?

Yes, that is exactly the purpose of this. So that we don't have to reconstruct the Operation object. However, I understand that Jake's changes in #12730 might reshape the way this is done. So storing the op_object might not be necessary after that merges, so maybe we should wait for that PR.

mtreinish · 2024-07-23T17:30:36Z

crates/accelerate/src/target_transpiler/mod.rs

+    /// Returns an iterator over all the instructions present in the `Target`
+    /// as pair of `&OperationType`, `&SmallVec<[Param; 3]>` and `Option<&Qargs>`.
+    // TODO: Remove once `Target` is being consumed.
+    #[allow(dead_code)]


Another note for a follow-up we should add rust space tests for these, since they're meant to be consumed from rust we should be able to test them without python.

mtreinish · 2024-07-23T17:35:11Z

qiskit/transpiler/target.py

+                        self._coupling_graph = (
+                            None  # pylint: disable=attribute-defined-outside-init
+                        )


Is this pylint disable still needed? https://github.com/Qiskit/qiskit/pull/12292/files#diff-00276708f86c726ae04a2c670e74177eaad6aad971e017951bb5792501d13bf5R338 seems to define it in __init__

mtreinish · 2024-07-23T17:48:46Z

qiskit/transpiler/target.py

@@ -1188,10 +893,12 @@ def __str__(self):
                prop_str_pieces = [f"\t\t{qarg}:\n"]
                duration = getattr(props, "duration", None)
                if duration is not None:
-                    prop_str_pieces.append(f"\t\t\tDuration: {duration} sec.\n")
+                    prop_str_pieces.append(
+                        f"\t\t\tDuration: {duration if duration > 0 else 0} sec.\n"


When is duration < 0?

This is so that Python doesn't display the floating point number at 0. Of course there might be a better way of doing this.

mtreinish · 2024-07-23T17:49:01Z

qiskit/transpiler/target.py

                error = getattr(props, "error", None)
                if error is not None:
-                    prop_str_pieces.append(f"\t\t\tError Rate: {error}\n")
+                    prop_str_pieces.append(f"\t\t\tError Rate: {error if error > 0 else 0}\n")


Same question here when is error < 0

This is so that Python doesn't display the floating point number at 0. Of course there might be a better way of doing this.

Same here.

mtreinish · 2024-07-23T17:56:59Z

crates/accelerate/src/target_transpiler/mod.rs

+The intent of this struct is to contain data that can be representable and
+accessible through both Rust and Python, so it can be used for rust-based
+transpiler processes.
+ */


The only thing I think we should mention here is around where we have copies of data or not and how we keep things in sync. Just so people will understand the split in responsibilities between the python space and the rust space. Otherwise this is much better.

- Mention duplication in docstring for rust Target. - Use f"{*:g}" to avoid printing the floating point for 0 in `Target`'s repr method. - Add note mentioning future unit-tests in rust.

mtreinish

LGTM, thanks for all the hard work on this @raynelfss. I think this is ready to merge. There are some small things I outlined in my earlier reviews that we can fix in follow ups. This is a new rust space representation so just like with everything we're migrating over there will likely be a longish tail of follow-ups we'll make to make incremental improvements as we start to use it more. But this is definitely in a state where I think it's good to start working with.

This all being said I'm going to hold off on pushing the automerge button right now, because I think we should wait until we branch 1.2.0 for this. Since nothing in 1.2.0 will use this I think it's better to just include this in 1.3.0.

…iskit#12292) * Initial: Add `Target` class to `_accelerate` - Add `Target` class to test mobility between Rust and Python. - Add `add_instruction` method to test compatibility with instructions. * Fix: Remove empty property check - Property check caused most cases to panic. - Will be commented out and restored at a later time. * Add: Instructions property - Instructions property returns all added to the target. - Similar behavior to source. * Chore: comments and deprecated methods - Add comments to instruction property. - Use new_bound for new PyDicts. * Chore: Remove redundant code - Remove redundant transformation of PyObject to PyTuple. - Remove debugging print statement. * Add: `InstructionProperties` class and type checkers - Add `InstructionProperties` class to process properties in rust. - Add `is_instance` and `is_class` to identify certain Python objects. - Modify logic of `add_instruction` to use class check. - Other tweaks and fixes. * Add: Setter and Getter for calibration in `InstructionProperty` * Add: `update_instruction_properties` to Target. * Add: Update_from_instruction_schedule_map - Partial addition from Target.py\ - Introduction of hashable qarg data structure. - Other tweaks and fixes. * Add: Complete `update_from_instruction_schedule_map1 - Complete missing procedures in function. - Rename `Qargs` to `HashableVec`. - Make `HashableVec` generic. - Separate `import_from_module_call` into call0 and call1. - Other tweaks and fixes. * Add: instruction_schedule_map property. - Remove stray print statements. - Other tweaks and fixes. * Fix: Key issue in `update_from_instruction_schedule_map` - Remove all unsafe unwraps * Fix: Use PyResult Value for void functon - Update `update_from_instruction_schedule_map to use PyResult and '?' operator. - Use Bound Python objects whenever possible. - Other tweaks and fixes. * Add: Python wrapping for Target - Add temporary _target module for testing. - Remove update_from_instruction_schedule_map function back to python. - Add python properties for all public attributes in rust - Other tweaks and fixes. * Add: `qargs` property - Add identical method `qargs` to obtain the qargs of a target. - Other tweaks and fixes. * Add: `qargs_for_operation_name` function. - Add function with identical behavior to the original in Target. - Other tweaks and fixes. * Add: durations method for Target - Add target module to qiskit init file. - Remove is_instance method. - Modify set_calibration method in InstructionProperty to leave typechecking to Python. - Change rust Target alias to Target2. - Other tweaks and fixes, * Add: InstructionProperties wrapper in python * Fix: InstructionProperties could not receive calibrations - Fix wrong setters/getters for calibration in InstructionProperty object in rust. * Add: more methods to Target in `target.rs` - Add FromPyObject trait to Hashable vec to receive Tuples and transform them directly into this type. - Add operations_for_qargs for Target class in Rust side and Python. - Fix return dict keys for `qargs_for_operation_name`. - Add `timing_constrains` and `operation_from_name` to Python side. - Other tweaks and fixes. * Fix: missing return value in `operations_for_args` - Fix wrong name for function operation_for_qargs. - Fix missing return value in the python side. - Other tweaks and fixes. * Fix: Bad compatibility with InstructionProperties - Make `InstructionProperties` "_calibration" attribute visible. - Removed attribute "calibration", treat as class property. - Other tweaks and fixes * Add: `operation_names_for_qargs` to Target - Port class method to rust and connect to Python wrapper. - Other tweaks and fixes. * Add: instruction_supported method to rust and python: - Other tweaks and fixes. * Add: changes to add_instruction function to increase functionality. - These changes break current functionality of other functions, butemulate intended behavior better. - Fixes coming soon. * Fix: Backwards compatibility with `add_instruction` - Fixed wrong additions to HashMaps in the rust side causing instructions to be missing. - Other tweaks and fixes. * Fix: Gate Map behavior didn't match Qiskit#11422 - Make GateMap use optional values to match behavior of Qiskit#11422. - Define GateMapType for complex type in self.gate_map. - Throw Python KeyError exceptions from the rust side in `update_instruction_properties` and other functions. - Modify logic in subsequent functions that use gate_map optional values. - Other tweaks and fixes. * Add: `has_calibration` method to Target * Add: `get_calibraton` method to Target * Add: `instruction_properties` method to Target * Add: `build_coupling_map` and helper methods - `build_coupling_map`will remain in Python for now, along with its helper functions. - Make `gate_name_map` visible to python. - Add `coupling_graph` attribute to Target in Rust. - Other tweaks and fixes. * Add: `get_non_global_operation_names` to Target. - Add attributes `non_global_strict_basis` and `non_global_basis` as Optional. - Other tweaks and fixes. * Add: Missing properties - Add properties: operations, operation_names, and physical_qubits. - Reorganize properties placement. - Other tweaks and fixes. * Add: `from_configuration` classmethod to Target. - Add method that mimics the behavior of the python method. - Change concurrent_measurements to 2d Vec instead of a Vec of sets. - Other tweaks and fixes. * Add: Magic methods to Rust and Python - Add docstring to __init__. - Add __iter__, __getitem__, __len__, __contains__, keys, values, and items methods to rust. - Add equivalen methods to python + the __str__ method. - Make description an optional attribute in rust. - Other tweaks and fixes. * Fix: Bugs when fetching qargs or operations - Fix qarg_for_operation_name logic to account for None and throw correct exceptions. - Stringify description before sending in case of numerical descriptors. - Fix qarg to account for None entry. - Other tweaks and fixes. * Chore: Prepare for Draft PR - Remove _target.py testing file. - Fix incorrect initialization of calibration in InstructionProperties. - Other tweaks and fixes. * Fix: target not being recognized as a module - Add target to the pyext crate. - Change placement of target import for alphabetical ordering. - Other tweaks and fixes. * Fix: Change HashMap to IndexMap - Change from f32 to f64 precision. - Other tweaks and fixes. * Fix: Move InstructionProperties fully to Rust - Move InstructionProperties to rust. - Modify gate_map to accept an InstructionProprties object instead of PyObjecy. - Change update_instruction_properties to use Option InstructionProprtyird. - Remove InstructionProperties from target.py - Other tweaks and fixes. * Fix: Make Target inherit from Rust - Make Target inherit from the rust-side Target by using subclass attribute, then extending its functionality using python. - Switch from __init__ to __new__ to adapt to the Target rust class. - Modify every trait that worked with `target._Target` to use `super()` or `self` instead. - Fix repr in InstructionProperties to not show `Some()` when a value exists. - Fix `__str__` method in `Target` to not display "None" if no description is given. - Assume `num_qubits` is the first argument when an integer is provided as a first argument and nothing else is provided for second (Target initializer). - Return a set in operation_names instead of a Vec. - Other tweaks and fixes. * Fix: Recognize None in `operation_for_qargs`. - Fix module labels for each class in target.rs. - Use py.is_instance instead of passing isinstance to `instruction_supported`. - Modify `operations_for_qargs` to accept optional values less aggressively. Allow it to find instructions with no qargs. (NoneType). - Other tweaks and fixes. * Fix: Make InstructionProperties subclassable. - Fix get_non_global_operation_names to accept optional values and fix search set to use sorted values. - Fix __repr__ method in InstructionProperties to add punctuation. - Fix typo in python durations method. - Modify test to overload __new__ method instead of just __init__ (Possible breaking change). -Other tweaks and fixes. * Fix: errors in `instruction_properties` and others: - Allow `instruction_properties` method to view optional properties. - Allow `operation_names_for_qargs` to select class instructions when None is passed as a qarg. - Modify __str__ method to display error and duration times as int if the value is 0. - Other tweaks and fixes. * Fix: call `isclass` from rust, instead of passing it from Python. * Fix: Move `update_from_instruction_schedule_map` to rust. * Fix: Move `durations` to rust. * Fix: Move `timing_constraints` to rust * Fix: Move operations_from_name fully to rust * Fix: `instruction_supported` method: - Rewrite the logic of instruction_supported due to previous errors in the method. - Move `check_obj_params` to Rust. - Other tweaks and fixes. * Fix: errors in `from_configuration` class method. - Fix some of the logic when retrieving gates from `name_mapping`. - Remove function arguments in favor of implementing counterpart functions in rust. - Add qubit_props_list_from_props function and return rust datatypes. - Fix wrong error handling procedures when retrieving attributes from backend_property. - Other tweaks and fixes. * Fix: Import `InstructionScheduleMap` directly instead of passing. - `instruction_schedule_map()` now imports the classtype directly from rust instead of needing it to be passed from python. - Remove unused imports in `target.py`. - Ignore unused arguments in `test_extra_props_str`. - Other tweaks and fixes. * Docs: Add docstrings to rust functions - Remove redundant redefinitions in python. - Fix text_signatures for some rust functions. - Added lint exceptions to some necessary imports and function arguments. - Other tweaks and fixes. * Add: Make `Target` and `InstructionProperties` pickleable. - Add `__getstate__` and `__setstate__` methods to make both rust subclasses pickleable. * Fix: Wrong calibration assignment in __setstate__ - Use set_calibration to set the correct calibration argument. - Fix wrong signature in get_non_global_operation_names. - Other tweaks and fixes. * Refactor: HashableVec is now Qarg - Use `PhysicalQubit` instead of u32 for qargs. - Use a `SmallVec` of size 4 instead of a dynamic Vec. - Default to using the `Hash()` method embedded in `SmallVec`. - Add a Default method to easily unwrap Qarg objects. - Other tweaks and fixes. * Add: `get` function to target. - Remove some redundant cloning in code. - Other small fixes. * Fix: Remove unnecessary Optional values in gate_map. - Update every gate_map call to use the new format. - Other small tweaks and fixes. * Refactor: `calibration` is for `InstructionProperties` - Use python `None` instead of option to store `calibration` in `InstructionProperties`. - Adapt code to these changes. - Remove redundant implementation of Hash in Qargs. - Other tweaks and fixes. * Fix: Temporary speedup for `gate_map` access - Added temporary speedups to access the gate_map by returning the values as PyObjects. - Convert qargs to rust tuples instead of initializing a `PyTuple`. - Store `InstructionProperties` as a python ref in gate_map. (Will be changed in future updates). - Other tweaks anf fixes. * Fix: Incorrect extractions for `InstructionProperties` - Fix incorrect conversion of `InstructionProperties` to `Py<InstructionProperties>` - Fix incorrect extraction of qargs in `update_from_instruction_schedule_map` * Fix: Hide all private attributes in `Target` - Hide all private attributes of the `Target` to prevent unecessary cloning. - Other small tweaks and fixes. * Add: New representation of gate_map using new pyclasses: - Make Qarg a sequence pyclass. - Make QargPropsMap the new representation of a GateMap value. - Adapt the code to new structure. - TODO: Add missing magic methods for sequence and mapping objects. - Other small tweaks and fixes. * Add: Use custom datatypes to return values to Python. - Add QargSet datatype to return a set of qargs. - Works as return type for `Target.qargs` - Object is has itertype of QargSetIter. - Rename QargPropMap to PropsMap - Use iterator type IterPropsMap - Other small tweaks and fixes. * Fix: Extend `InstructionProperties` to be subclassable using `__init__: - Made a subclass of `InstructionProperties` that can be extended using an `__init__`method. - Revert previous changes to `test_target.py`. - Other tweaks and fixes. * Refactor: Split target into its own module - Reestructure the files to improve readability of code. - `instruction_properties.rs` contaisn the `InstructionProperties` class. - `mod.rs` contains the `Target` class. - `qargs.rs` contains the Qargs struct to store quantum arguments. - `property_map` contains the Qarg: Property Mapping that will be stored in the gate_map. - Add missing methods to PropsMap: - Add `PropsMapKeys` object to store the qargs as a set. - Add methods to compare and access `PropsMapKey`. - Add QargsOrTuple enum in Qargs to parse Qargs instantly. * Fix: Rest of failing tests in Target - Modify the `InstructionProperties` python wrapper. - InstructionProperties was not assigning properties to rust side. - Make duration in `InstructionProperties` setable. - Add `__eq__` method for `PropMap` to compare with other dicts. - `PropMapKeys` can only be compared with a Set. - Remove `qargs_for_operation_name` from `target.py` - Other small tweaks and fixes. * Add: New GateMap Structure - GateMap is now its own mapping object. - Add `__setstate__` and `__getstate__` methods for `PropMap` and `GateMap`. - Other small tweaks and fixes. * Fix: Make new subclasses pickleable - Add module location to `PropsMap`, `GateMap`, and `Qargs`. - Added default method to PropMap. - Made default method part of class initializers. - Other smalls tweaks and fixes. * Fix: Remove redundant lookup in Target (Qiskit#12373) * Format: `mod.rs` qubit_comparison to one line. * Add: `GateMapKeys` object in GateMap: - Use IndexSet as a base to preserve the insertion order. - Other tweaks and fixes. * Add: __sub__ method to GateMapKeys * Fix: Modify `GateMap` to store values in Python heap. - Fix `GateMap.__iter__` to use an IndexKeys iterator. - Other small tweaks and fixes. * Fix: Remove duplicate import of `IndexSet::into_iter` in `GateMap`. - Make `__iter__` use the keys() method in `GateMap`. * Fix:: Adapt to target changes (Qiskit#12288) - Fix lint stray imports. * Fix: Incorrect creation of parameters in `update_from_instruction_schedule_map` - Add `tupelize` function to create tuples from non-downcastable items. - Fix creation of Parameters by iterating through members of tuple object and mapping them to parameters in `update_from_instruction_schedule_map`. - Add missing logic for creating a Target with/without `qubit_properties`. - Add tuple conversion of `Qargs` to store items in a dict in `BasisTranslator` and `UnitarySynthesis` passes. - Cast `PropsMap` object to dict when comparing in `test_fake_backends.py`. - Modify logic of helper functions that receive a bound object reference, a second `py` not required as an argument. - Add set operation methods to `GateMapKeys`. - Other tweaks and fixes. * Fix: More failing tests - Fix repeated erroneous calls to `add_instruction` in `update_from_instruction_schedule_map` - Add missing condition in `instruction_supported` - Use `IndexSet` instead of `HashSet` for `QargsSet`. - Other small tweaks and fixes. * Add: Macro rules for qargs and other sequences. - Create `QargSet` and `PropsMap` using the new macros. - Return a `TargetOpNames` ordered set to python in `operation_names`. - Remove the Python side `operation_names.` - Fix faulty docstring in `target.py`. - Other tweaks and fixes. * Docs: Add necessary docstrings to all new rust functions. - Remove duplicate Iterator in GateMap. - Other small tweaks and fixes. * Fix: Use `GILOneCell` and remove `Qargs` - Use `GILOneCell` to import python modules only once at initialization. - Remove the custom data structure `Qargs` to avoid conversion overhead. - `Qargs` does not use `PhysicalQubits`, `u32` is used instead. - Fix `__setstate__ `and `__getstate__` methods for `PropsMap`, `GateMap`, and `key_like_set_iterator` macro_rule. - Update code to use the new structures. - TODO: Fix broken tests. * Fix: Cast `Qargs` to `Tuple` in specific situations - Use tupleize to cast `Qargs` to `Tuple` in `instructions`. - Use downcast to extract string in `add_instruction`. - Other tweaks and fixes. * Add: Make `Target` Representable in Rust - Rename `InstructionProperties` as `BaseInstructionProperties`. - Remove `Calibration` from the rust space. - Restore `gate_map`, `coupling_map`, `instruction_schedule_map`, and `instruction_durations` to rust. - Remove all unnecessary data structures from rust space. - Other tweaks and fixes. * Refactor: Remove previour changes to unrelated files. * Add: rust native functions to target - Added rust native functionality to target such that a `py` would not be needed to use one. - Add Index trait to make `Target` subscriptable. - Other small tweaks and fixes. * Fix: Remove all unnecessary python method calls. - Remove uage of `inspect.isclass`. - Rename `Target` to `BaseTarget` in the rust side. - Rename `err.rs` to `errors.rs`. - Remove rust-native `add_inst` and `update_inst` as Target should not be modified from Rust. - Made `add_instruction` and `update_instruction_properties` private in `BaseTarget`. - Add missing `get` method in `Target`. - Other tweaks and fixes * Format: Fix lint * Fix: Wrong return value for `BaseTarget.qargs` * Add: Temporary Instruction representation in rust. - Add temporary instruction representation to avoid repeated extraction from python. * Add: Native representation of coupling graph * Fix: Wrong attribute extraction for `GateRep` * Remove: `CouplingGraph` rust native representation. - Move to different PR. * Format: Remove stray whitespace * Add: `get_non_global_op_names` as a rust native function * Fix: Use Ahash for Hashing - Use ahash for hashing when possible. - Rename `BaseTarget` to `Target` in rust only. - Rename `BaseInstructionProperties` to `InstructionProperties` in rust only. - Remove optional logic from `generate_non_global_op_names`. - Use dict for `__setstate__` and `__getstate__` in `Target`. - Reduced the docstring for `Target` and `InstructionProperties`. - Other small tweaks and fixes. * Format: new changes to `lib.rs` * Format: Adapt to new lint rules * Fix: Use new gates infrastructure (Qiskit#12459) - Create custom enum to collect either a `NormalOperation` or a `VariableOperation` depending on what is needed. - Add a rust native `is_instruction_supported` method to check whether a Target supports a certain instruction. - Make conversion methods from `circuit_instruction.rs` public. - Add comparison methods for `Param` in `operations.rs` - Remove need for isclass method in rustwise `add_instruction` - Other tweaks and fixes. * Format: Fix rust formatting * Add: rust-native method to obtain Operstion objects. * Add: Comparison methods for `Param` * FIx: Add display methods for `Params` * Format: Fix lint test * Format: Wrong merge conflict solve * Fix: Improve rust methods to use iterators. - Adapt the Python methods to leverage the rust native improvements. - Use python native structures for the Python methods. * Format: Remove extra blankspace * Fix: Remove `text_signature`, use `signature` instead. * Fix: Rectify the behavior of `qargs` - Keep insertion order by inserting all qargs into a `PySet`. - Perform conversion to `PyTuple` at insertion time leveraging the iterator architecture. - Remove python side counterpart to avoid double iteration. - Make rust-native `qargs` return an iterator. * Fix: Corrections from Matthew's review - Use `format!` for repr method in `InstructionProperties` - Rename `Variable` variant of `TargetInstruction` to `Variadic`. - Remove `internal_name` attribute from `TargetOperation`. - Remove `VariableOperation` class. - Use `u32` for `granularity`, `pulse_alignment`, and `acquire_alignment`. - Use `Option` to store nullable `concurrent_measurements. - Use `&str` instead of `String` for most function arguments. - Use `swap_remove` to deallocate items from the provided `properties` map in `add_instruction`. - Avoid cloning instructions, use `to_object()` instead. - Avoid using `.to_owned()`, use `.clone()` instead. - Remove mention of `RandomState`, use `ahash::HashSet` instead. - Move parameter check to python in `instruction_supported`. - Avoid exposing private attributes, use the available ones instead. - Filter out `Varidadic` Instructions as they're not supported in rust. - Use peekable iterator to peak at the next qargs in `generate_non_global_op_names`. - Rename `qarg_set` to `deduplicated_qargs` in `generate_non_global_op_names`. - Return iterator instances instead of allocated `Vec`. - Add `python_compare` and `python_is_instance` to perform object comparison with objects that satisfy the `ToPyObject` trait. - Other small tweaks and fixes. * Implement a nullable dict-like structure for IndexMap (Qiskit#2) * Initial: Implement a nullable dict-like structure for IndexMap * FIx: Erroneous item extraction from Python - Fix error that caused `None` values to be ignored from `None` keys. - Removed mutability from rust function argument in `add_instruction`. - Object is mutably referenced after option unwrapping. - Add missing header in `nullable_index_map.rs`. - Add Clone as a `K` and/or `V` constraint in some of the iterators. - Remove `IntoPy` constraint from `NullableIndexMap<K, V>`. - Add `ToPyObject` trait to `NullableIndexMap<K, V>`. * Fix: inplace modification of Python dict. - Perform `None` extraction from rust. - Revert changes to `Target.py` * Fix: Avoid double iteration by using filter_map. * Docs: Add inline comments. * Fix: More specific error message in `NullableIndexMap` * Fix: Use `Mapping` as the metaclass for `Target` - Minor corrections from Matthew's review. * Fix: Make `Target` crate-private. - Due to the private nature of `NullableIndexMap`, the `Target` has to be made crate private. - Add temporary`allow(dead_code)` flag for the unused `Target` and `NullableIndexMap` methods. - Fix docstring of `Target` struct. - Fix docstring of `add_instruction`. - Make several python-only operations public so they can be used with other `PyClass` instances as long as they own the gil. - Modify `py_instruction_supported` to accept bound objects. - Use rust-native functions for some of the instance properties. - Rewrite `instruction` to return parameters as slice. - `operation_names` returns an `ExactSizeIterator`. - All rust-native methods that return an `OperationType` object, will return a `NormalOperation` instance which includes the `OperationType` and the parameters. * Fix: Comments from Matthew's review - Mention duplication in docstring for rust Target. - Use f"{*:g}" to avoid printing the floating point for 0 in `Target`'s repr method. - Add note mentioning future unit-tests in rust. * Fix: Adapt to Qiskit#12730

Fixes Port `BasisTranslator` to Rust Qiskit#12246 This is the final act of the efforts to move the `BasisTranslator` transpiler pass into Rust. With many of the parts of this pass already living in Rust, the following commits attempt to bring in the final changes to allow complete operation of this pass in the Rust space (with of course some interaction with Python.) Methodology: The way this works is by keeping the original `BasisTranslator` python class, and have it store the rust-space counterpart (now called `CoreBasisTranslator`) which will perform all of the operations leveraging the existent Rust API's available for the `Target` (Qiskit#12292), `EquivalenceLibrary`(Qiskit#12585) and the `BasisTranslator` methods `basis_search` (Qiskit#12811) and `compose_transforms`(Qiskit#13137). All of the inner methods will have private visibility and will not be accessible to `Python` as they're intended to be internal by design. By removing the extra layers of conversion we should be seeing a considerable speed-up, alongside all of the other incremental improvements we have made. Changes: - Add the pyo3 class/struct `BasisTranslator` that will contain allof the main data used by the transpiler pass to perform its operation. - Convert the `target_basis` into a set manually from python before sending it into the Rust space. - Remove the exposure of `basis_search` and `compose_transforms` to python. - Change `basis_search` so that it accepts references to `HashSet` instances instead of accepting a `HashSet<&str>` instance. - Change inner method's visibility for `basis_search` and `compose_transform` modules in rust. - Expose the exception imports from `Target` to the `accelerate` crate. - Expose `DAGCircuit::copy_empty_like` to the rest of the crates. - Remove all of the unused imports in the Python-side `BasisTranslator`. Blockers: - [ ] Qiskit#12811

…Rust. (#13237) * Initial: Move the rest of the `BasisTranslator` to Rust. Fixes Port `BasisTranslator` to Rust #12246 This is the final act of the efforts to move the `BasisTranslator` transpiler pass into Rust. With many of the parts of this pass already living in Rust, the following commits attempt to bring in the final changes to allow complete operation of this pass in the Rust space (with of course some interaction with Python.) Methodology: The way this works is by keeping the original `BasisTranslator` python class, and have it store the rust-space counterpart (now called `CoreBasisTranslator`) which will perform all of the operations leveraging the existent Rust API's available for the `Target` (#12292), `EquivalenceLibrary`(#12585) and the `BasisTranslator` methods `basis_search` (#12811) and `compose_transforms`(#13137). All of the inner methods will have private visibility and will not be accessible to `Python` as they're intended to be internal by design. By removing the extra layers of conversion we should be seeing a considerable speed-up, alongside all of the other incremental improvements we have made. Changes: - Add the pyo3 class/struct `BasisTranslator` that will contain allof the main data used by the transpiler pass to perform its operation. - Convert the `target_basis` into a set manually from python before sending it into the Rust space. - Remove the exposure of `basis_search` and `compose_transforms` to python. - Change `basis_search` so that it accepts references to `HashSet` instances instead of accepting a `HashSet<&str>` instance. - Change inner method's visibility for `basis_search` and `compose_transform` modules in rust. - Expose the exception imports from `Target` to the `accelerate` crate. - Expose `DAGCircuit::copy_empty_like` to the rest of the crates. - Remove all of the unused imports in the Python-side `BasisTranslator`. Blockers: - [ ] #12811 * Fix: Redundancies with serialization methods - Remove extra copies of `target`, `target_basis`, `equiv_lib`, and `min_qubits`. - Remove unnecessary mutability in `apply_transforms` and `replace_node`. * Refactor: Remove `BasisTranslator` struct, use pymethod. - Using this method avoids the creation of a datastructure in rust and the overhead of deserializing rust structures which can be overly slow due to multiple cloning. With this update, since the `BasisTranslator` never mutates, it is better to not store anything in Rust. * Lint: Ignore too_many_args flag from clippy on `basis_translator::run()` * Fix: Remove redundant clone * Fix: Leverage using `unwrap_operation` when taking op_nodes from the dag. - Add function signature in `base_run`. * Update crates/accelerate/src/basis/basis_translator/mod.rs Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com> * Adapt to #13164 - Use `QuantumCircuit._has_calibration_for()` when trying to obtain calibrations from a `QuantumCircuit` due to the deprecation of the `Pulse` package. --------- Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>

raynelfss added 30 commits April 4, 2024 12:47

Initial: Add Target class to _accelerate

5c86791

- Add `Target` class to test mobility between Rust and Python. - Add `add_instruction` method to test compatibility with instructions.

Fix: Remove empty property check

1d50653

- Property check caused most cases to panic. - Will be commented out and restored at a later time.

Merge branch 'Qiskit:main' into move-target

7135d72

Merge branch 'Qiskit:main' into move-target

7d0982e

Add: Instructions property

ecc4668

- Instructions property returns all added to the target. - Similar behavior to source.

Chore: comments and deprecated methods

506b1bd

- Add comments to instruction property. - Use new_bound for new PyDicts.

Chore: Remove redundant code

f8064bf

- Remove redundant transformation of PyObject to PyTuple. - Remove debugging print statement.

Add: InstructionProperties class and type checkers

e4e53ce

- Add `InstructionProperties` class to process properties in rust. - Add `is_instance` and `is_class` to identify certain Python objects. - Modify logic of `add_instruction` to use class check. - Other tweaks and fixes.

Merge branch 'Qiskit:main' into move-target

b15e762

Add: Setter and Getter for calibration in InstructionProperty

3d82845

Add: update_instruction_properties to Target.

fa2baab

Add: Update_from_instruction_schedule_map

90b7081

- Partial addition from Target.py\ - Introduction of hashable qarg data structure. - Other tweaks and fixes.

Add: Complete `update_from_instruction_schedule_map1

a65bf82

- Complete missing procedures in function. - Rename `Qargs` to `HashableVec`. - Make `HashableVec` generic. - Separate `import_from_module_call` into call0 and call1. - Other tweaks and fixes.

Add: instruction_schedule_map property.

c00ab10

- Remove stray print statements. - Other tweaks and fixes.

Fix: Key issue in update_from_instruction_schedule_map

10cec4f

- Remove all unsafe unwraps

Fix: Use PyResult Value for void functon

12cccca

- Update `update_from_instruction_schedule_map to use PyResult and '?' operator. - Use Bound Python objects whenever possible. - Other tweaks and fixes.

Add: Python wrapping for Target

c3d79ff

- Add temporary _target module for testing. - Remove update_from_instruction_schedule_map function back to python. - Add python properties for all public attributes in rust - Other tweaks and fixes.

Merge branch 'Qiskit:main' into move-target

5e2d100

Add: qargs property

be6bf15

- Add identical method `qargs` to obtain the qargs of a target. - Other tweaks and fixes.

Add: qargs_for_operation_name function.

28e2e01

- Add function with identical behavior to the original in Target. - Other tweaks and fixes.

Add: durations method for Target

1f9bdbf

- Add target module to qiskit init file. - Remove is_instance method. - Modify set_calibration method in InstructionProperty to leave typechecking to Python. - Change rust Target alias to Target2. - Other tweaks and fixes,

Add: InstructionProperties wrapper in python

dd7e115

Fix: InstructionProperties could not receive calibrations

f58fb09

- Fix wrong setters/getters for calibration in InstructionProperty object in rust.

Fix: missing return value in operations_for_args

bc089cc

- Fix wrong name for function operation_for_qargs. - Fix missing return value in the python side. - Other tweaks and fixes.

Fix: Bad compatibility with InstructionProperties

6fc8679

- Make `InstructionProperties` "_calibration" attribute visible. - Removed attribute "calibration", treat as class property. - Other tweaks and fixes

Add: operation_names_for_qargs to Target

3fec978

- Port class method to rust and connect to Python wrapper. - Other tweaks and fixes.

Merge branch 'Qiskit:main' into move-target

01d146a

Add: instruction_supported method to rust and python:

020493d

- Other tweaks and fixes.

Add: changes to add_instruction function to increase functionality.

abe88cf

- These changes break current functionality of other functions, butemulate intended behavior better. - Fixes coming soon.

Merge branch 'Qiskit:main' into move-target

adc0231

mtreinish reviewed Jul 15, 2024

View reviewed changes

raynelfss and others added 4 commits July 15, 2024 16:16

Fix: Use Mapping as the metaclass for Target

69cbc4e

- Minor corrections from Matthew's review.

Merge branch 'main' into move-target

743c55a

Merge branch 'main' into move-target

d5392ea

raynelfss requested a review from mtreinish July 16, 2024 19:54

mtreinish self-assigned this Jul 18, 2024

mtreinish reviewed Jul 23, 2024

View reviewed changes

raynelfss and others added 5 commits July 23, 2024 15:16

Fix: Comments from Matthew's review

367d711

- Mention duplication in docstring for rust Target. - Use f"{*:g}" to avoid printing the floating point for 0 in `Target`'s repr method. - Add note mentioning future unit-tests in rust.

Merge branch 'main' into move-target

e7296ba

Merge branch 'main' into move-target

f6e8189

Merge branch 'main' into move-target

96bd389

Fix: Adapt to Qiskit#12730

70eafce

mtreinish approved these changes Jul 24, 2024

View reviewed changes

mtreinish modified the milestones: 1.2.0, 1.3 beta Jul 24, 2024

mtreinish added this pull request to the merge queue Jul 25, 2024

Merged via the queue into Qiskit:main with commit 898fdec Jul 25, 2024
15 checks passed

raynelfss mentioned this pull request Jul 26, 2024

[WIP] Try to speed up Target python-side mapping access. #12829

Closed

mtreinish mentioned this pull request Aug 7, 2024

Introduce a gate name interner to avoid duplicating String in the Target #12917

Closed

jakelishman mentioned this pull request Aug 14, 2024

Transpile fails when multiple circuits with UnitaryGate or DiagonalGate #12953

Closed

raynelfss mentioned this pull request Sep 27, 2024

[Oxidize BasisTranslator]: Move the rest of the BasisTranslator to Rust. #13237

Merged

1 task

raynelfss added the Changelog: None Do not include in changelog label Nov 5, 2024

mtreinish mentioned this pull request Nov 6, 2024

Solve strange behavior with qubit_properties in Target. #13400

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding `Target` and `InstructionProperties` representations to rust #12292

Adding `Target` and `InstructionProperties` representations to rust #12292

raynelfss commented Apr 23, 2024 •

edited

Loading

mtreinish left a comment

mtreinish Jul 12, 2024

mtreinish Jul 15, 2024

mtreinish Jul 15, 2024

raynelfss Jul 15, 2024

mtreinish left a comment

mtreinish Jul 23, 2024

raynelfss Jul 23, 2024

mtreinish Jul 23, 2024

mtreinish Jul 23, 2024

mtreinish Jul 23, 2024

raynelfss Jul 23, 2024

mtreinish Jul 23, 2024

raynelfss Jul 23, 2024 •

edited

Loading

mtreinish Jul 23, 2024

mtreinish left a comment

		#[pyo3(signature = (instruction, name, properties=None))]
		fn add_instruction(

Adding Target and InstructionProperties representations to rust #12292

Adding Target and InstructionProperties representations to rust #12292

Conversation

raynelfss commented Apr 23, 2024 • edited Loading

Summary

Details and comments

Rust

Python:

Possible breaking changes:

Broken tests

Blocks

Open to review!

mtreinish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtreinish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raynelfss Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtreinish left a comment

Choose a reason for hiding this comment

Adding `Target` and `InstructionProperties` representations to rust #12292

Adding `Target` and `InstructionProperties` representations to rust #12292

raynelfss commented Apr 23, 2024 •

edited

Loading

raynelfss Jul 23, 2024 •

edited

Loading