Simplify Relabel logic
#2085
Simplify Relabel logic
#2085
Conversation
This simplifies the relabeling logic to collapse the four match cases down into one for easier readibility. It avoids trying to optimize special cases for qubits that may not have been mapped and treats all labels as having an intial mapping to themselves.
| // This tells us which label to use in the swap, which we will use in the update of the mappings too. | ||
| let label_r = *mappings | ||
| .keys() | ||
| .find(|k| mappings[*k] == r) |
There was a problem hiding this comment.
This step is linear, which makes the whole thing quadratic. Granted, our qubit count won't be large, but let's figure out linear algorithm (in average case). Should be doable with mutable array and mutable hash map.
There was a problem hiding this comment.
The previous implementation avoided doing this reverse lookup every time by only performing it when necessary. You are right, I wouldn't expect Relabel to be called with very large lists of qubits, and even if it were I don't think this would take lots of time unless relabels were happening very frequently.
If we do want to optimize this further, we could maintain two maps, one from label to actual and one from actual to label. Or, if we are comfortable with some higher memory costs, we could use an IndexMap for fast lookup (but then any gaps in qubit ids would be empty entries in the underlying vector). Overall, I'm don't really think it's worth optimizing this further until we have reason to believe that large lists of qubits will be relabeled frequently, and then we can perf test to find the right optimizations.
There was a problem hiding this comment.
I think we should eventually move this to simulator. And we could improve it at that time. For now, I suggest we add a comment that this step may take O(n) time, but we are only expecting small number of qubits.
This simplifies the relabeling logic to collapse the four match cases down into one for easier readibility. It avoids trying to optimize special cases for qubits that may not have been mapped and treats all labels as having an intial mapping to themselves.
Based on feedback from @DmitryVasilevsky in #2082 (comment)