-
Notifications
You must be signed in to change notification settings - Fork 8
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
[BUG] Can't measure complex operator on real MPS in expect
and correlation_matrix
#79
Comments
It looks like it is related to GPU support enhancements. Presumably the purpose of calling adapt is for adapting the precision of element type of the op to match precision of the MPS, but not to force Float<->Complex conversions. If this is correct, then we want this sort of behaviour: op{ComplexF64}=adapt(ComplexF64,op{ComplexF64}) |
@JanReimers's assessment is correct. One way to solve this would be to define a custom adaptor to handle this case, as described here: https://github.com/JuliaGPU/Adapt.jl/blob/4c07cccd182bd36a7cbb288d64c8281dd7be665d/src/Adapt.jl#L5-L39. It would mostly just do conversion of the element type, but also be less strict about adapting complex values to real values (i.e. in this case when the MPS is real but the adapt(RealOrComplex(Array{Float32}), ::Array{Float64})::Array{Float32}
adapt(RealOrComplex(Array{ComplexF32}), ::Array{Float64})::Array{ComplexF32}
adapt(RealOrComplex(Array{Float32}), ::Array{ComplexF64})::Array{ComplexF32} # Currently broken for `adapt(Array{Float32}, ::Array{ComplexF64})` The name An alternative would be to first construct the Ultimately, the |
I got it working, and ready to make a PR. However, there is already a
Any thoughts about what name to use for the Adapt one? It could be |
What about |
I'm good with that. One other question is about the type that goes into the adaptor. (This is something that's still a bit magical about Adapt overall to me.) For this bug, the argument on the right will be an ITensor, formed by calling So for that specific example (adapting an ITensor) would we want the call to look like:
|
I think we can support both, but that is up to us. For this case we want the version |
Sounds good. Would those have different behavior and/or serve a different purpose? Or would they be two ways of accomplishing the same result? |
They would have different behavior in general, because |
So for example |
Great. I think I get it now - very helpful. I was just a bit catching up on the intended interface and behavior of |
I think basically we can define our own "adaptors" to have any behavior, and you can think of adapt as a way to "reach down" into a nested data structure, where the adaptor works at the low level of the data structure. So for example our adaptors could work on an |
And specifically, you overload
|
Is there any update on this issue? Is there a possible workaround?
|
You should be able to use |
expect
and correlation_matrix
Description of bug
Using complex operator on real MPS in
expect
is throwing InexactError. I think the issue is coming from a functionadapt
which may be trying to convert the operator to a real-valued operator, which is possibly incorrect logic for that code since contraction of a complex operator with a real MPS is allowed.Minimal code demonstrating the bug or unexpected behavior
Minimal runnable code
Version information
versioninfo()
:using Pkg; Pkg.status("ITensors")
:The text was updated successfully, but these errors were encountered: