Make ancilla for BiQubitsMixer sided too. #6313
Conversation
|
@fdmalone I'm still interested in getting an answer to my question in #6312 (comment), maybe here or in the linked issue quantumlib/Qualtran#389 What's the expected behavior of the ancilla register? Do we need to expose it as a LEFT / RIGHT register at all ? From a quick glance, it looks like the answer is yes and that the gate is essentially using leaving dirty ancillas in the output, which get cleaned up by its adjoint, in order to reduce the T-counts. If that is the case, it'd be nice to clarify this as part of the docstring. |
|
I'll defer to @NoureldinYosri |
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6313 +/- ##
==========================================
- Coverage 97.89% 97.89% -0.01%
==========================================
Files 1106 1106
Lines 96156 96157 +1
==========================================
- Hits 94134 94132 -2
- Misses 2022 2025 +3
☔ View full report in Codecov by Sentry. |
|
"ancilla" here is a misnomer that follows from the original paper. it should be more like target registers that are assumed to be clean. @tanujkhattar the gate is simillar to so yea they should be sided (and probably renamed) |
| [ | ||
| infra.Register('x', 2), | ||
| infra.Register('y', 2), | ||
| infra.Register('ancilla', 3, side=one_side), |
There was a problem hiding this comment.
lets rename it target. this will result in some changes to the decomposition and docstring
There was a problem hiding this comment.
Let's also improve the docstring to more clearly specify what each of the 3 target bits contain. It's not clear to me right now by just reading the docstring.
There was a problem hiding this comment.
the short answer is .. enough information to do the adjoint. coming to think of it, this is why I kept the register name from the paper as ancilla. because they have no use otherthan as information holders for that will be used by the adjoint.
the actually useful information is stored inplace.
so maybe skip renaming it for now and I will come back for it later.
There was a problem hiding this comment.
hmm, we've renamed such registers to junk in Qualtran.
@fdmalone As part of this PR, I think we should at least capture this discussion in the docstring and let the reader know that the ancilla is used to reduce the T-counts and assumes that it should be cleaned up later by the adjoint.
There was a problem hiding this comment.
Ok. I'll try distill something later today. We should open an issue to track either here or in qualtran.
Similar fix as #6312 and fixes quantumlib/Qualtran#389