Skip to content
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

Circuit comparison logic doesn't make sense #487

Open
coreyostrove opened this issue Sep 18, 2024 · 0 comments
Open

Circuit comparison logic doesn't make sense #487

coreyostrove opened this issue Sep 18, 2024 · 0 comments
Assignees
Labels
bug A bug or regression
Milestone

Comments

@coreyostrove
Copy link
Contributor

Describe the bug
The logic used for the __lt__ and __gt__ methods for comparisons between Circuit objects doesn't produce results in line with what common sense would dictate is correct.

To Reproduce
Here are two circuits

a = Circuit(rho0[][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][]@(0))
b= Circuit(rho0[][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][]@(0))

With the currently implemented logic evaluating a<b results in True, and likewise b>a results in True.

Expected behavior
Intuitively one would expect that b would be considered less than a. As for why it isn't, it is because the comparison currently performs a __lt__ comparison between the tup attributes for each circuit. In this case the reason a is less than b is because the __lt__ logic for tuples iterates through the two tuples pairwise and short circuits one any pairwise comparison evaluates to True, and it only uses the length information is every element is pairwise equal across the two tuples (before running out of elements from the shorter one). The tup attribute also includes line label information though at the end of the tuple (the @ and 0), and apparently the '@' character is considered greater than Label(()) by python (🤷).

All of this said, this is kind of an obvious example, a robust fix for this is kind of nebulous though. What does it mean in general for one circuit to be less than or greater than another? Right now we're using a lexicographic ordering that seems pretty poorly motivated. One (possibly better motivated option) would be to define this in terms of inclusion. I.e. a circuit x would be considered less than another circuit y if x was a subcircuit contained within y, and vice versa for the notion of greater than. (sounds pretty tricky to implement, and possibly computationally expensive). Or, maybe we choose door 3 and decide that defining this notion in general is too fraught and we nix these methods entirely, leaving it up to users to define the appropriate notion given their particular context.

I don't know how widely these operations are used in the codebase, but the context in which I ran into this was investigating some strange looking results coming from the construction of PrefixTable objects, which are used for identifying efficient cache-based evaluation strategies for the MapForwardSimulator. In this construction a list of input circuits is/was being sorted using the default __lt__ and __gt__ methods (which causes some problems and will be changed in a forthcoming branch).

Environment (please complete the following information):

  • pyGSTi version: develop (though this goes all the way back to whatever version of pyGSTi was around circa 2020 based on commit history).
@coreyostrove coreyostrove added the bug A bug or regression label Sep 18, 2024
@coreyostrove coreyostrove self-assigned this Sep 18, 2024
@sserita sserita added this to the 0.9.14 milestone Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug or regression
Projects
None yet
Development

No branches or pull requests

2 participants