feature: Added counts based on operator name and qubits specified#1275
feature: Added counts based on operator name and qubits specified#1275ACE07-Sev wants to merge 3 commits into
Conversation
|
I added the qubit specifier you requested, but not quite sure what you meant by density of operators. If you can kindly guide me on that, I can add that as well today. |
|
@sesmart Can I please get a review? |
|
Hi @ACE07-Sev , @aniksd-braket will be reviewing this issue. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1275 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 169 169
Lines 10963 10975 +12
Branches 1412 1416 +4
=========================================
+ Hits 10963 10975 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
This looks like a useful direction. I think the “operator density” stretch goal from the issue could be implemented as a small complementary layer on top of raw instruction counts. My interpretation would be a deterministic circuit summary that includes both counts and normalized densities, for example:
This would make the feature useful not only as a convenience method, but also for benchmarking, sanity checks, and comparing generated or transformed circuits. A possible output shape could be: { If this interpretation aligns with the maintainers’ expectations, I can open a focused follow-up PR for the density/summary part, without duplicating the existing count implementation. |
|
@jaimeajl may I ask if you're part of the maintainer team? |
|
@aniksd-braket Can I please get a review? |
aniksd-braket
left a comment
There was a problem hiding this comment.
Would be great if you could also add the case of getting counts for of operations on all qubits specified, instead of just any of the qubits specified
|
|
||
| for instruction in self.instructions: | ||
| if qubits and not set(instruction.target).intersection(qubits): | ||
| continue |
There was a problem hiding this comment.
Would the output be an empty dictionary if a qubit exists in the circuit but has no operation? Could you add a test for this case?
There was a problem hiding this comment.
You mean if I have this
from braket.circuits import Circuit
circuit = Circuit().h(0).cnot(0, 2)
print(circuit.count(qubits={1}))and wondering what will return for this?
| f"Invalid qubits: {qubits - self.qubits}" | ||
| ) | ||
|
|
||
| for instruction in self.instructions: |
There was a problem hiding this comment.
Here do you consider all operators, including noise operations?
There was a problem hiding this comment.
Any instruction applied to the circuit (if we go with just ones applied to qubits then gphase would need to be skipped I suppose). Are there exclusions you want me to skip?
|
Greetings @aniksd-braket Thank you very much for the review. Sure thing, I'll get to it first thing in the morning. |
Issue #, if available: Closes #1235
Description of changes:
Added instruction counting to
Circuitwith additional filtering based on specific gate name and/or qubits applied to.Testing done:
Added a unit tester to
test_circuit.pywhich asserts the desired behavior, and ensures aValueErroris raised for out of bounds qubits specified.Merge Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.