-
Notifications
You must be signed in to change notification settings - Fork 126
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
oneITensor
to rule them all
#618
Comments
This is definitely interesting. So to make sure I understand, is the key to the different behavior here mainly that these tensors would have a distinct storage type? I.e. that you can overload on the type |
Sort of, but I think it is better to think about it in terms of the properties that we want to satisfy. I think ultimately the properties we want for any ITensor
So from the properties above it follows that Contrast this with the case where we chose that In principle we could do all of this without dispatch and without literally introducing a new type. For example, we could have some runtime value that flagged what kind of |
A common code pattern is:
which is useful when initializing a loop, for example. However, it has bothered me that it makes an unnecessary copy of
B
. We decided, I think correctly, thatITensor(1) * B
should make a copy ofB
, since it could lead to very subtle bugs ifITensor(1) * B
didn't copy butITensor(1.000001) * B
did copy (say you were doing floating point operations to compute a constanta
, and then didITensor(a) * B
, and then one timea
happened to be equal to1
but you were relying on the copying behavior).My proposal for this would be to introduce yet another storage type as well as a special number type called
One
. The proposal would be the following:The
Base.RefValue
storage allows the value to be modified in-place, i.e. would allowT[] = 2.0
. That's not totally necessary but I think is helpful for generic code, since a lot of code inNDTensors
assumes the tensors are mutable.Then, we can define contraction such that
(oneITensor() * B) = B
(without copying).Scalar(One())
storage could be interpreted as a universal multiplicative identity. Note that if someone did1.0 * oneITensor()
, it would create a tensor with storageScalar(1.0)
, which would then act asITensor(1.0)
with copying behavior with multiplication. This allows the nice property that(1.0 * oneITensor()) * B
andoneITensor() * (1.0 * B)
both return copies ofB
.Another note that the
Scalar
storage isn't strictly necessary here, and we could use a length 1Dense
storage with element typeOne
, but I've been meaning to add aScalar
type anyway since I think it will help clean up some code logic with scalar ITensor contractions.A bonus from this is getting a behavior that I've wanted for
delta
tensors, which is thatA * delta(i, j)
would not copyA
if the contraction is equivalent to an index replacement. This faced a similar issue to trying to makeITensor(1) * A
non-copying, which is thatA * (1 * delta(i, j))
would not copy but(A * 1) * delta(i, j)
would copy (and similarly, if1
was replaced by a general floating point constanta
, the behavior would subtly change ofa
happened to be equal to1
).With the new
One
type, adelta
tensor could have storage with a value ofOne
instead of1
, which would be clearly documented. Then, if you did1 * delta(i, j)
, it would return a tensor with storage of1.0
and have the regular copying behavior, which would have the property thatA * (1 * delta(i, j))
and(A * 1) * delta(i, j)
always return a copy ofA
.The text was updated successfully, but these errors were encountered: