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

Add support for 64-bit ints. #54

Closed
wants to merge 4 commits into from

Conversation

MonkeysAreEvil
Copy link

@MonkeysAreEvil MonkeysAreEvil commented Oct 5, 2021

See issue #50 .

Add support for 64-bit ints. In particular, the nnz (number of nonzeros)
field and rowind, colind (row and column indices) in storage
structures have been updated, as have all computational functions
(e.g. zgstrf), and utility functions (e.g. sp_coletree). Also updated
are memory functions, guaranteeing that the correct size is allocated.

In practice, this requires updating a huge number of functions to
guarantee correctness and minimise typecasts. Some typecasts have been
added, where integer overflow is unlikely (e.g. colamd, and also to
BLAS functions[1]).

This is achieved by the typedef:

/* Define my integer type int_t */
typedef long long int int_t;
typedef int int_t; /* default */

int_t can then be selected at compile-time via the cmake option
-Denable_longints.

When compiled with this flag on this library is API breaking and any
library which depends on SuperLU (e.g. armadillo, scipy) will need to
be updated to use long long int where relevant. In practice this
should be straightforward [2].

Notes:

  • I know there's a lot in this patch, but it is atomic, i.e. if I try and break it up something will break.
  • Hopefully there's no formatting issues; in general I don't believe I've changed anything too drastic.
  • I haven't updated any comments or inline docs. I'm not sure if it's worth updating argument docs to say int_t rather than int, or if there should be any comments anywhere discussing int_t
  • I've updated the tests to use int_t where relevant, but haven't added any tests which actually require int64. I'm using my own test cases, which are a bit complicated and involve armadillo. It might be worth adding some extra tests; all current ones pass for both int32 and int64.
  • I've taken notes from superlu_mt but I don't believe I've straight up copied anything.
  • I think I've updated most of the relevant info/debug printfs too, but may have missed a couple.

[1] Note: there are some BLAS calls wrapped in CRAY ifdefs that I
haven't added typecasts to because I can't test and also don't expect
CRAY to ever need int64.
[2] A patch for armadillo is under preparation and currently stands at
8 files changed, 109 insertions(+), 90 deletions(-).

…eros)

field and `rowind`, `colind` (row and column indicies) in storage
structures have been updated, as have all computational functions
(e.g. zgstrf), and utility functions (e.g. sp_coletree). Also updated
are memory functions, guaranteeing that the correct size is allocated.

In practice, this requires updating a huge number of functions to
guarantee correctness and minimise typecasts. Some typecasts have been
added, where integer overflow is unlikely (e.g. `colamd`, and also to
BLAS functions[1]).

This is achieved by the typedef:

```
/* Define my integer type int_t */
typedef long long int int_t;
typedef int int_t; /* default */
```

`int_t` can then be selected at compile-time via the cmake option
`-Denable_longints`.

When compiled with this flag on this library is API breaking and any
library which depends on SuperLU (e.g. armadillo, scipy) will need to
be updated to use `long long int` where relevant. In practice this
should be straightforward [2].

[1] Note: there are some BLAS calls wrapped in e.g. CRAY ifdefs that I
*haven't* added typecasts to because I can't test and also don't expect
CRAY to ever need int64.
[2] A patch for armadillo is under preparation and currently stands at
`8 files changed, 109 insertions(+), 90 deletions(-)`.
SRC/dutil.c Outdated
@@ -293,15 +293,15 @@ dPrint_Dense_Matrix(char *what, SuperMatrix *A)
/*! \brief Diagnostic print of column "jcol" in the U/L factor.
*/
void
dprint_lu_col(char *msg, int jcol, int pivrow, int *xprune, GlobalLU_t *Glu)
dprint_t_lu_col(char *msg, int_t jcol, int_t pivrow, int_t *xprune, GlobalLU_t *Glu)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect replacement of int in function name.
Name of function must be "dprint_lu_col".

@MonkeysAreEvil
Copy link
Author

I've fixed the typo mentioned above, and also merged in the latest commit. At least on my end there seems to be no issues; all my tests pass.

@gruenich
Copy link
Contributor

gruenich commented Apr 7, 2023

In recent days, @xiaoyeli introduces support for 64 bit indexing (44f2c5e, 91276c9, 8fea5b0, 32cba61). Do these changes obsolete this merge request or is anything left, that is worth to extract and merge?

@MonkeysAreEvil
Copy link
Author

No, the recent patches don't seem to fix the issue. Building the latest head (0060a50) with -DXSDK_INDEX_SIZE=64 fails my test case with

SUPERLU_MALLOC fails for buf in intMalloc() at line 161 in file ...../SRC/memory.c

Setting -DPRNTlevel=1 -DDEBUGlevel=1 gives a little bit of extra info

.. parameters in sp_ienv():
         1: panel size     20 
         2: relax          10 
         3: max. super    200 
         4: row-dim 2D    200 
         5: col-dim 2D    100 
         6: fill ratio     30 
superlu_malloc fails: malloc_total 1775 MB, size 1663238955135632
superlu_malloc: out of memory at line 42 in file /var/tmp/portage/sci-libs/superlu-9999/work/superlu-9999/SRC/memory.c

Needless to say the out of memory error is spurious; I can monitor memory usage and it doesn't exceed a few GB. Also, the size being printed out is incorrect because the cast from size_t -> long long can overflow.

I don't have time right now to debug this in detail right now, and diffing the latest head and my fork shows quite some differences so I couldn't guess if there's an easy fix or not.

@xiaoyeli
Copy link
Owner

xiaoyeli commented Aug 5, 2023

64-bit is supported now.

@xiaoyeli xiaoyeli closed this Aug 5, 2023
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.

None yet

4 participants