-
Notifications
You must be signed in to change notification settings - Fork 59
Description
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).