Conversation
| ) | ||
| # left to middle | ||
| U = ones(scalartype(H), left_virtualspace(H, 1)) | ||
| U = ones(storagetype(H), left_virtualspace(H, 1)) |
There was a problem hiding this comment.
I think here we should actually call removeunit instead of writing this as a contraction. This is both more efficient and avoids having to deal with storagetype altogether.
Let me know if you need help with this though
| TensorKit.storagetype(PA::PeriodicArray{T, N}) where {T, N} = storagetype(T) | ||
|
|
There was a problem hiding this comment.
I'm a little hesitant to adopt storagetype in such a general manner here in MPSKit, since I don't think this is even exported by TensorKit. If this function should exist, I think we would have to decide to use storagetype(A::AbstractArray) = storagetype(eltype(A)), but I'm not sure if that really is in the scope of that (somewhat internal) function
There was a problem hiding this comment.
So, after thinking a bit about this: it is in fact an exported method, and I do agree that this is the one we want to use. This being said, I would like to avoid defining it on PeriodicArray directly, and reserve it for specific cases where actual tensors are involved. I am not sure if that makes sense to you or it just feels arbitrary, but I feel like PeriodicArray{Float64} is in principle a reasonable thing to have, and in that case the storagetype does not have to be storagetype(Float64).
There was a problem hiding this comment.
I think that's fair, I can try to remove this and work around any places it breaks things. Shouldn't be too hard to do, might just have to define a few more specializations (e.g. for InfiniteMPS)
|
Your PR no longer requires formatting changes. Thank you for your contribution! |
kshyatt
force-pushed
the
ksh/cu
branch
3 times, most recently
from
9890491 to
046f4d0
Compare
Co-authored-by: Lukas Devos <[email protected]>
Co-authored-by: Lukas Devos <[email protected]>
2 participants
Mostly allowing passing an array type instead of just an element type to allow GPU arrays to back MPS tensors