Functionality to list operations in QuantumCircuit with associated Qubits #13055

tonmoy-b · 2024-08-29T09:42:50Z

Summary

Added function to tally operations in circuit with associated qubits
fixes #12972

Details and comments

-Added function count_ops_with_qubits() in quantumcircuit.py, this returns the operations in the QuantumCircuit along with the qubits associated with those operations.
-The abovementioned function uses the Rust function fn operations_with_qubits in circuit_data.rs
-A test (test_count_ops_with_qubits()) has been added to file test/python/circuit/test_circuit_data.py
-tox has been run -- no test failures reported
-tox -eblack run to reformat files

[Yes ] I have added the tests to cover my changes.
[Yes ] I have updated the documentation accordingly.
[Yes ] I have read the CONTRIBUTING document.

…bits

qiskit-bot · 2024-08-29T09:42:56Z

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the following people are relevant to this code:

@Qiskit/terra-core
@kevinhartman
@mtreinish

tonmoy-b · 2024-08-29T09:52:39Z

Hi @mtreinish ,

 I have made this PR to fix issue 12972: Add a method to compute gate counts per qubit 
 
 Kindly take a look and let me know if any changes should be made.

 For a quick look, the new code works as below:
 a = QuantumCircuit(3, 3)
    a.h(0)
    a.x(1)
    a.h(1)
    a.cx(1, 0)
    b = a.count_ops_with_qubits()
    assert b == [
        ("h", "Qubit(QuantumRegister(3, 'q'), 0)"),
        ("x", "Qubit(QuantumRegister(3, 'q'), 1)"),
        ("h", "Qubit(QuantumRegister(3, 'q'), 1)"),
        ("cx", "Qubit(QuantumRegister(3, 'q'), 1)"),
        ("cx", "Qubit(QuantumRegister(3, 'q'), 0)"),
    ]

mtreinish

Thanks for taking the issue and getting an implementation started. I left a few inline comments. But I think my biggest concern right now is about the return type. Write now you're returning a Vec<Option<String>, Option<String>>, but instead I think it should be a Vec<HashMap<&str, usize>> where the vec index is the qubit index and the hashmap is gate name mapped to count. I had some other inline comments, but refactoring the code to adjust the return type will take those into account.

mtreinish · 2024-08-29T11:19:35Z

crates/circuit/src/circuit_data.rs

+    /// Returns:
+    ///     list.
+    #[pyo3(signature = ())]
+    pub fn operations_with_qubits(&self, py: Python<'_>) -> PyResult<Py<PyAny>> {


Instead of returning a Py<PyAny> here typically I'd prefer to return the Vec directly because then this function would be callable from rust too. The pyo3 macros build a parallel c ffi function for use that is what python is actually calling, so this is still usable rust function otherwise. If we return a Vec here the macro will do the pyobject conversion for us when called from python.

mtreinish · 2024-08-29T13:39:01Z

crates/circuit/src/circuit_data.rs

+    ///     list.
+    #[pyo3(signature = ())]
+    pub fn operations_with_qubits(&self, py: Python<'_>) -> PyResult<Py<PyAny>> {
+        let mut ret: Vec<(Option<String>, Option<String>)> = vec![];


I think I think the return type here is a bit off. What I think would be useful here is to return either a vec or hashmap of qubit indices that map to the instruction counts on them. For example something like:

[ {"h": 20, "cx" 2, "s": 10}, {"cx": 2}, ]

for a circuit that has 2 qubits with 20 hadamards, 10 s gates, and 2 cx gates on qubit 0 and qubit 1 has 2 cx gates.

hi @mtreinish , I'm a bit stuck here, how could I get this information (the qubit numbers) without having to get substrings on the string gotten from the Qubit objects from 'Some(self.qubits.get(*b).unwrap().to_string()'? Or should I go ahead and get the substrings to obtain the qubits and populate the HasMaps from that?

Oh, that's not too difficult for any given PackedInstruction object instr you get while iterating over the circuit data you can call CircuitData::get_qargs(instr.qubits) which will return a &[Qubit] object. The Qubit object is a newtype u32. So you can do Qubit.0 as usize to get the index into a vec for any Qubit.

Ok, will follow that procedure. Thanks

mtreinish · 2024-08-29T13:41:16Z

qiskit/circuit/quantumcircuit.py

+    # The stringified return type is because OrderedDict can't be subscripted before Python 3.9, and
+    # typing.OrderedDict wasn't added until 3.7.2.  It can be turned into a proper type once 3.6
+    # support is dropped.
+    def count_ops_with_qubits(self) -> list:


The next qiskit release will only support python >= 3.9 so this isn't relevent. Also this isn't currently returning an ordered dict. Although I would argue we should not be using an ordered dict at all. The other methods do because they have been around since Qiskit supported much older versions of python (before dicts were sorted by default) and have retained it for backwards compatibility. But for new functionality we should just return a dict or a list.

Suggested change

# The stringified return type is because OrderedDict can't be subscripted before Python 3.9, and

# typing.OrderedDict wasn't added until 3.7.2. It can be turned into a proper type once 3.6

# support is dropped.

def count_ops_with_qubits(self) -> list:

mtreinish · 2024-08-29T13:45:08Z

crates/circuit/src/circuit_data.rs

+    pub fn operations_with_qubits(&self, py: Python<'_>) -> PyResult<Py<PyAny>> {
+        let mut ret: Vec<(Option<String>, Option<String>)> = vec![];
+        for (_index, inst) in self.data.iter().enumerate() {
+            let op_str = inst.op.view().name().to_string();


We should probably try to avoid to_string() here as this will make a copy of every string, and we can keep it all by reference in Rust space. When we return to Python that will do the copying for us.

mtreinish · 2024-08-29T13:48:50Z

crates/circuit/src/circuit_data.rs

+            for b in self.qargs_interner.get(inst.qubits) {
+                ret.push((
+                    Some(op_str.to_string()),
+                    Some(self.qubits.get(*b).unwrap().to_string()),


If we were going to use qubit objects like this, I'd have suggested returning them directly instead of stringify them. Then the python user could interact with the bit object directly. However, I don't think we need to work with actual bit objects and can just use indices. If a user wants to map the index to a bit, in python that's just circuit.qubits[index].

coveralls · 2024-08-29T16:36:14Z

Pull Request Test Coverage Report for Build 10612643610

Details

14 of 14 (100.0%) changed or added relevant lines in 2 files are covered.
18 unchanged lines in 3 files lost coverage.
Overall coverage increased (+0.008%) to 89.204%

Files with Coverage Reduction	New Missed Lines	%
crates/accelerate/src/two_qubit_decompose.rs	1	90.82%
crates/qasm2/src/lex.rs	5	92.23%
crates/qasm2/src/parse.rs	12	97.15%

Totals
Change from base Build 10598013395:	0.008%
Covered Lines:	71661
Relevant Lines:	80334

💛 - Coveralls

Functionality to list operations in QuantumCircuit with associated Qu…

56582c3

…bits

tonmoy-b requested a review from a team as a code owner August 29, 2024 09:42

qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Aug 29, 2024

mtreinish reviewed Aug 29, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Functionality to list operations in QuantumCircuit with associated Qubits #13055

Functionality to list operations in QuantumCircuit with associated Qubits #13055

tonmoy-b commented Aug 29, 2024

qiskit-bot commented Aug 29, 2024

tonmoy-b commented Aug 29, 2024

mtreinish left a comment

mtreinish Aug 29, 2024

mtreinish Aug 29, 2024

tonmoy-b Sep 8, 2024

mtreinish Sep 13, 2024

tonmoy-b Sep 13, 2024

mtreinish Aug 29, 2024

mtreinish Aug 29, 2024

mtreinish Aug 29, 2024

coveralls commented Aug 29, 2024

Functionality to list operations in QuantumCircuit with associated Qubits #13055

Are you sure you want to change the base?

Functionality to list operations in QuantumCircuit with associated Qubits #13055

Conversation

tonmoy-b commented Aug 29, 2024

Summary

Details and comments

qiskit-bot commented Aug 29, 2024

tonmoy-b commented Aug 29, 2024

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

Choose a reason for hiding this comment

coveralls commented Aug 29, 2024

Pull Request Test Coverage Report for Build 10612643610

Details

💛 - Coveralls