You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I noticed from #43 that the eprstate assumes vacuum variance is 1/2. I haven't had time to follow recent developments so I'm not sure what other places make assumptions of vacuum variance here, but I wanted to make an issue so that it can be tracked and discussed. This is by no means urgent, but I think it's worth considering ways make this convention possibly user configurable. At least in my research it's definitely been a source of bugs as there is unfortunately no universally agreed upon convention. I think weedbrook et all uses the convention of vacuum variance being 1. At the very least it is necessary to be consistent about what convention is being used and that should be documented.
This is relevant to thinking about #43 since having (1/2) as a julia expression in constructors for gaussian objects means one will get floating point numerical constants inside symbolic expressions and it's typically more helpful for this to be SymbolicUtils.Div(1,2) so that further simplifications can happen (often one ends up with factors that cancel the 2).
The text was updated successfully, but these errors were encountered:
Thanks for bringing this up, I've been thinking about making this convention user-configurable for the past few weeks. It's quite annoying how much the convention varies, it's been a source of bugs for me in the past as well. Probably the most straightforward way of dealing with this is to add the kwarg ħ = 1 to every relevant method so that users can change it to fit their own convention. If we want to be really careful, we can define the GaussianState type with the @kwdef macro and set ħ = 1 as a default there. Error or warning arguments can be thrown when, say, one takes a tensor product of Gaussian states with different conventions. And yes the default convention should be made clear in the documentation in the beginning, not just in the docstrings as it currently is.
This is relevant to thinking about #43 since having (1/2) as a julia expression in constructors for gaussian objects means one will get floating point numerical constants inside symbolic expressions and it's typically more helpful for this to be SymbolicUtils.Div(1,2) so that further simplifications can happen (often one ends up with factors that cancel the 2).
That's fair. Besides making the default convention of vacuum variance to be one (which would avoid a lot of these floating point problems), it would probably take a fair bit of internal work for each predefined method to support symbolic coefficients, which I'm certainly in favor of. For me, that specifically is a lower priority in the near term, but I'd definitely support this and give quick code reviews if you want to take this on.
I noticed from #43 that the
eprstate
assumes vacuum variance is1/2
. I haven't had time to follow recent developments so I'm not sure what other places make assumptions of vacuum variance here, but I wanted to make an issue so that it can be tracked and discussed. This is by no means urgent, but I think it's worth considering ways make this convention possibly user configurable. At least in my research it's definitely been a source of bugs as there is unfortunately no universally agreed upon convention. I think weedbrook et all uses the convention of vacuum variance being 1. At the very least it is necessary to be consistent about what convention is being used and that should be documented.This is relevant to thinking about #43 since having
(1/2)
as a julia expression in constructors for gaussian objects means one will get floating point numerical constants inside symbolic expressions and it's typically more helpful for this to beSymbolicUtils.Div(1,2)
so that further simplifications can happen (often one ends up with factors that cancel the 2).The text was updated successfully, but these errors were encountered: