-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Surround filter #257
base: main
Are you sure you want to change the base?
Surround filter #257
Conversation
|
||
point = self.normalize_point(point) | ||
def default_scoring_fn(region: CircuitRegion) -> float: | ||
return float(sum(op.num_qudits for op in self[region])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this isn't super important as it's just a default, but this means single- and multi- qudit operations are very similarly important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is an important point. I thought going by the number of qudits in a gate would be enough, but your comment made me realize this needs more. For example, a region with two single-qubit gates is equivalent to one with a two-qubit gate. How do you think we should solve this? Multiply by a factor, e.g., x100?
region = circuit.surround((1, 3), 4) | ||
assert region.location == CircuitLocation([2, 3, 4, 5]) | ||
|
||
def test_surround_filter_hard(self) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are some other use cases for filtering? It seems like maybe a MachineModel/topology or gateset check would be a nice extra test to have.
break | ||
# Absorb Single-qudit gates | ||
index = point[0] | ||
while index < self.num_cycles - 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the parts that do "Absorb Single-qudit gates" and "Check for gates in the middle not in region" should be sub-functions on there own.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, using the flag "valid_region" gave me that same thought, but I can't see a way that makes it any simpler. How are you thinking to do this?
New features:
Circuit.surround
now can take a scoring function and a filterCircuit.next
andCircuit.prev
can now also take aCircuitRegion
and return the next or prev points of the entire regionBug Fixes:
exclude=True
Circuit.check_region
missing bad regions where a gate would be half-in-half-out