-
Notifications
You must be signed in to change notification settings - Fork 193
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
new passive circuit compiler #600
Conversation
Codecov Report
@@ Coverage Diff @@
## master #600 +/- ##
==========================================
+ Coverage 98.45% 98.50% +0.04%
==========================================
Files 76 77 +1
Lines 8758 8834 +76
==========================================
+ Hits 8623 8702 +79
+ Misses 135 132 -3
Continue to review full report at Codecov.
|
I'm not sure why the coverage is so bad, maybe it's not aware of my new test files? Also "test_all_passive_gates" is failing but it passes on my laptop, so I'm not sure what's going on here. |
@jakeffbulmer the coverage is low because the following CI test is failing: This check is the one that contains frontend test and Gaussian backend tests :) Since it is not completing, it is not reporting its coverage information to the coverage check. |
@josh146 : when I run the test that the bots are failing locally, it passes. Not to sure what to do. Maybe I am missing one of the fancy options when the test are run. I am simply typing |
class PassiveChannel(Channel): | ||
r"""Perform an arbitrary multimode passive operation | ||
|
||
Args: | ||
T (array): an NxN matrix acting on a N mode state | ||
|
||
.. details:: | ||
|
||
Acts the following transformation on the state: | ||
|
||
.. math:: | ||
a^{\dagger}_i \to \sum_j T_{ij} a^{\dagger}j | ||
""" | ||
|
||
def __init__(self, T): | ||
super().__init__([T]) | ||
self.ns = T.shape[0] | ||
|
||
def _apply(self, reg, backend, **kwargs): | ||
p = par_evaluate(self.p) | ||
backend.passive(p[0], *reg) | ||
|
||
|
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.
since the Fock backends do not know how to deal with passive
operations, we should add a decompositions method by using the SVD of the passive operation. Similarly, it should be fairly straightforward to add support to the bosonic backend. I get that this can be added in a future PR... If that is the case, are we raising the correct Not Implemented error
when this function is called?
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.
💯
def __init__(self, T): | ||
super().__init__([T]) | ||
self.ns = T.shape[0] | ||
|
||
def _apply(self, reg, backend, **kwargs): | ||
p = par_evaluate(self.p) | ||
backend.passive(p[0], *reg) |
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.
I guess this takes care of raising an error if the backend does not have the passive
method implemented?
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.
@jakeffbulmer I'm curious as well. Could we define a decomposition for PassiveChannel
so that it would work on any backend? Similar to how Interferometer
works
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.
Very nice addition @jakeffbulmer . Assuming that the corrects error messages are raised for the fock
, tf
and bosonic
backend this look almost ready merge.
This reverts commit b4d9153.
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.
@jakeffbulmer this is a great addition! Aside from the comments I left here, three main requests that should be addressed in a new PR asap:
-
Don't forget to update the changelog with an example!
-
The new
PassiveChannel
is not showing up in thesf.ops
module docstring. I believe it needs to be added to the correct list at the bottom of theops.py
file (thechannel
variable might make the most sense? or alternativelydecompositions
?) -
Finally, the
PassiveChannel
needs to be added to the operations quickstart in the filedoc/introduction/ops.rst
.
Not as urgent, but I am also curious re: supporting decompositions for the PassiveChannel
. Could you open a GitHub issue briefly explaining the requested feature and (vaguely) what needs to be done to support it? Thanks!
def passive(self, T, *modes): | ||
""" | ||
linear optical passive transformations | ||
""" | ||
T_expand = identity(self.circuit.nlen, dtype=T.dtype) | ||
T_expand[ix_(modes, modes)] = T | ||
self.circuit.passive(T_expand) |
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.
Since this has been added to the GaussianBackend
class, it should probably be added to BaseGaussian
, or even BaseBackend
(@nquesada, thoughts?)
All other methods in this class are inherited from and overwritten from either BaseBackend
(should be supported on all backends) or BaseGaussian
(should be supported on all Gaussian backends), hence why they do not have docstrings defined 🙂
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.
I've added to BaseGaussian!
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.
Perfect!
self.mean = U @ self.mean | ||
self.nmat = U.conj() @ self.nmat @ U.T | ||
self.mmat = U @ self.mmat @ U.T |
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.
how come the changes to conjugation here?
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.
the old version was incorrect and it wasn't being used by any existing operations, so after checking with @nquesada I changed it
T = np.dot(other.p[0], self.p[0]) | ||
# if one, replace with the identity | ||
if T == 1: | ||
T_arr = np.atleast_2d(T) | ||
if np.allclose(T_arr, np.eye(T_arr.shape[0])): |
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.
Not 100% sure myself, but this doesn't have any effect for users using Channels with the TF backend?
I assume not, since the tests still passed, although it could be something the test suite is missing :)
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.
When p[0] is a scalar (as is assumed in the previous version) I think the only operational difference after my change is that something within tolerance of 1 will be assumed to be identity, whereas previously it had to be exact. Is this okay? Is there an appropriate tol variable I can pass into the np.allclose
?
def __init__(self, T): | ||
super().__init__([T]) | ||
self.ns = T.shape[0] | ||
|
||
def _apply(self, reg, backend, **kwargs): | ||
p = par_evaluate(self.p) | ||
backend.passive(p[0], *reg) |
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.
@jakeffbulmer I'm curious as well. Could we define a decomposition for PassiveChannel
so that it would work on any backend? Similar to how Interferometer
works
class PassiveChannel(Channel): | ||
r"""Perform an arbitrary multimode passive operation | ||
|
||
Args: | ||
T (array): an NxN matrix acting on a N mode state | ||
|
||
.. details:: | ||
|
||
Acts the following transformation on the state: | ||
|
||
.. math:: | ||
a^{\dagger}_i \to \sum_j T_{ij} a^{\dagger}j | ||
""" | ||
|
||
def __init__(self, T): | ||
super().__init__([T]) | ||
self.ns = T.shape[0] | ||
|
||
def _apply(self, reg, backend, **kwargs): | ||
p = par_evaluate(self.p) | ||
backend.passive(p[0], *reg) | ||
|
||
|
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.
💯
@josh146 one fairly easy way to implement passive channels in other backends would be to perform a singlular value decomposition on T, then implement it as Interferometer -> Loss -> Interferometer. What would be the best way to implement a direct implementation in gaussian backened and a decomposition with the other backends for the PassiveChannel operation? |
@jakeffbulmer if you add a Since all backends support interferometers and loss |
Context:
If you want to run a circuit on a large number of modes, with a large depth of passive optical operations, this can be quite slow. In this PR, we introduce a new compiler which works for fully passive circuits. This is able to compile any passive, linear operation into a single matrix. This can then be applied as a single operation.
Description of the Change:
The "passive" compiler can compile passive linear circuits. The PassiveChannel operation can apply arbitrary passive multimode operations to a state.
Benefits:
Should allow for faster computation of e.g. large multimode covariance matrices which are formed by a large depth of operations.
Possible Drawbacks:
Only implemented for the Gaussian backened.
Not compatible with active operations, such as squeezing, displacement etc. So circuits can only be compiled if they produce a vacuum state, which is a bit confusing.
Related GitHub Issues: