Skip to content

Update lapack wrapper functions to use LAPACK_INT - #369

Merged
emstoudenmire merged 1 commit into
ITensor:v3from
kyungminlee:ilp64
Oct 17, 2020
Merged

Update lapack wrapper functions to use LAPACK_INT#369
emstoudenmire merged 1 commit into
ITensor:v3from
kyungminlee:ilp64

Conversation

@kyungminlee

Copy link
Copy Markdown
Contributor

int is used here and there in functions inside lapack_wrap.cc and algs.cc.
Replace them with LAPACK_INT so that ITensor can be linked to BLAS/LAPACK with ILP64.

`int` is used here and there in functions inside `lapack_wrap.cc` and `algs.cc`. Replace them with `LAPACK_INT` such that itensor can be linked to BLAS/LAPACK with ILP64.
@emstoudenmire
emstoudenmire merged commit 7cc8fe6 into ITensor:v3 Oct 17, 2020
@emstoudenmire

Copy link
Copy Markdown
Contributor

Thanks for the PR. Apparently before this change users were also experiencing crashes when the memory usage became large:
http://itensor.org/support/2604/issue-with-size-of-the-vector-in-large-bond-dimension
which this PR may fix.

@kyungminlee

Copy link
Copy Markdown
Contributor Author

The alloc-related crash is related to another PR: #368.
This PR addresses a different issue when linking with ILP64 interface of LAPACK, and should not have any effect if you are using 32bit/LP64.

On that note, I notice that long is used here and there in the code where large integers are expected. This is fine in Linux and macOS which uses LP64, but MSVC on Windows uses LLP64 ( why? 😞 ) so long is a 32bit integer. It would be nice to replace the long types with something like size_t, intmax_t, or intptr_t. What do you think?

@emstoudenmire

Copy link
Copy Markdown
Contributor

That sounds like a good idea and not too hard to start doing. Just to be clear, do you think this is primarily an issue just in these lower-level wrappers or even in higher level code? (We use long for the dimension of an Index object for example.) Also what do you think about using int64_t which is always guaranteed to be a 64 bit signed integer? (And similarly uint64_t if an unsigned integer is wanted.)

@kyungminlee

kyungminlee commented Oct 18, 2020

Copy link
Copy Markdown
Contributor Author

The integer size issue can cause problems at all levels, and it's safe to use larger integers unless there's a guarantee that it will be small. Right now I haven't found any problem with Linux and macOS. On Windows it puts a restriction on the size of tensors since long is 32 bits.

All in all, I think replacing long with int64_t is a good idea. It's safe since they define the same type on LP64 convention, which is the platform on which ITensor has been tested extensively. Of course using size_t when referring to a "size" of an object is ideal in principle, but potential integer overflows make it dangerous; it would take much more time and effort to do it.

The only issue with intN_t types is that it is an optional type in C++11, defined only on systems that support N bit integers. Though practically, who runs ITensor on a 32 bit machine?

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

Successfully merging this pull request may close these issues.

2 participants