Skip to content
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

Slow down due to translatecell #78

Open
LHerviou opened this issue Jul 6, 2023 · 3 comments
Open

Slow down due to translatecell #78

LHerviou opened this issue Jul 6, 2023 · 3 comments

Comments

@LHerviou
Copy link
Contributor

LHerviou commented Jul 6, 2023

During the creation of MPOMatrices, I experienced some significant slowdown, in particular when not using the base translate function (though not only) because accessing a tensor in fact often creates a copy of it.

Would it not be better to add a small modification in all our translatecell such that if we ask a translation of 0, it directly return the original tensor. Something akin to (in the celledvectors.jl file):

#Transfer the functional properties
#translatecell(translator, T::ITensor, n::Integer) = translator(T, n)
function translatecell(translator::Function, T::ITensor, n::Integer)
  if n == 0
      return T
  end
  return ITensors.setinds(T, translatecell(translator, inds(T), n))
end
translatecell(translator::Function, T::MPO, n::Integer) = n==0 ? T : translatecell.(translator, T, n)
function translatecell(translator::Function, T::Matrix{ITensor}, n::Integer)
  if n == 0
      return T
  end
  return translatecell.(translator, T, n)
end  ###etc

This should drastically reduce the memory footprint and speed up some parts of the code.

@mtfishman
Copy link
Member

mtfishman commented Jul 6, 2023

So does it seem to currently actually copy the tensor data, or just do a "shallow" copy of the tensor? A "shallow" copy would just copy the tensor wrapper structure and indices but not the data itself. That could of course still have some overhead, especially if it is done a lot, but at least wouldn't scale with the size of the tensor data.

Operations like setinds should just be doing a shallow copy of the ITensor.

@LHerviou
Copy link
Contributor Author

LHerviou commented Jul 6, 2023

It should do a shallow copy of the tensor if I am not mistaken. I will try to investigate a bit. It is really a marginal cost most of the time (it just turned out I was doing it a few thousand times when filling up InfiniteMPOMatrix of long range models).

@mtfishman
Copy link
Member

Definitely makes sense to avoid setting indices/changing tags when it isn't necessary, so your proposal makes sense to me to do anyway. Maybe we have to think about the copying behavior and do a shallow copy in the case of iszero(n).

Another possibility is that if you know you are only working with a unit cell, you could slice out that unit cell temporarily and turn it into a finite MPO, then do operations on that finite MPO and then convert back to an infinite MPO after.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants