Skip to content

Commit

Permalink
Add ordering option: METIS_AT_PLUS_A
Browse files Browse the repository at this point in the history
  • Loading branch information
@xiaoyeli
xiaoyeli committed Jul 15, 2024
1 parent c511e1d commit 6f7ea89
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 4 deletions.
4 changes: 2 additions & 2 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ endif()
option(enable_internal_blaslib "Build the CBLAS library" ${enable_blaslib_xSDK})
option(enable_single "Enable single precision library" ON)
option(enable_double "Enable double precision library" ON)
option(enable_complex "Enable complex precision library" ON)
option(enable_complex16 "Enable complex16 precision library" ON)
option(enable_complex "Enable single precision complex library" ON)
option(enable_complex16 "Enable double precision complex (complex16) library" ON)
option(enable_matlabmex "Build the Matlab mex library" OFF)
option(enable_doc "Add target 'doc' to build Doxygen documentation" OFF)
option(enable_examples "Build examples" ON)
Expand Down
19 changes: 18 additions & 1 deletion SRC/get_perm_c.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ get_perm_c(int ispec, SuperMatrix *A, int *perm_c)
#endif
return;
#ifdef HAVE_METIS
case METIS_ATA: /* METIS ordering on A'*A */
case METIS_ATA: /* METIS ordering on A'*A */
getata(m, n, Astore->nnz, Astore->colptr, Astore->rowind,
&bnz, &b_colptr, &b_rowind);

Expand All @@ -487,6 +487,23 @@ get_perm_c(int ispec, SuperMatrix *A, int *perm_c)
printf(".. Use METIS ordering on A'*A\n");
#endif
return;
case METIS_AT_PLUS_A: /* METIS ordering on A'*A */
if ( m != n ) ABORT("Matrix is not square");
at_plus_a(n, Astore->nnz, Astore->colptr, Astore->rowind,
&bnz, &b_colptr, &b_rowind);

if ( bnz ) { /* non-empty adjacency structure */
get_metis(n, bnz, b_colptr, b_rowind, perm_c);
} else { /* e.g., diagonal matrix */
for (i = 0; i < n; ++i) perm_c[i] = i;
SUPERLU_FREE(b_colptr);
/* b_rowind is not allocated in this case */
}

#if ( PRNTlevel>=1 )
printf(".. Use METIS ordering on A'+A\n");
#endif
return;
#endif

default:
Expand Down
2 changes: 1 addition & 1 deletion SRC/superlu_config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#define SUPERLU_CONFIG_H

/* Enable metis */
/* #undef HAVE_METIS */
#define HAVE_METIS TRUE

This comment has been minimized.

Sign in to view

Copy link
@dschmitz89

dschmitz89 Aug 4, 2024

Over at SciPy PR scipy/scipy#21172, I get errors that METIS is not found even if this is set to FALSE. Must it be 0 or something else? Note that we do not use your CMake file but compile ourselves.

This comment has been minimized.

Sign in to view

Copy link
@dschmitz89

dschmitz89 Aug 4, 2024

Commenting it out works but this is still confusing.

This comment has been minimized.

Sign in to view

Copy link
@dschmitz89

dschmitz89 Aug 6, 2024

CC @xiaoyeli

This comment has been minimized.

Sign in to view

Copy link
@rgommers

rgommers Aug 6, 2024

That setting it to FALSE doesn't work isn't surprising. HAVE_xxx variables are usually checked with #ifdef, so all that matters is whether the variable is defined at all or not. That's how #cmakedefine will behave:

superlu/SRC/superlu_config.h.in

Line 6 in 67bdc36

#cmakedefine HAVE_METIS @HAVE_METIS@

The only issue here is that the generated superlu_config.h is checked into the repo and that in SciPy we then need to patch the default value since we don't accept an external METIS dependency.

/* Enable colamd */
/* #undef HAVE_COLAMD */
Expand Down

6 comments on commit 6f7ea89

@xiaoyeli
Copy link
Owner Author

Choose a reason for hiding this comment

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

Checking superlu_config.h in the repo is for the projects that do not use CMake, rather, they want to use their own make. So this file needs to be present.
I don't have a good solution to satisfy both types of users.

@rgommers
Copy link

Choose a reason for hiding this comment

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

SciPy is in that second category (we use Meson rather than CMake). The generic answer there is that those users need to trigger the header file generation themselves. Meson does this without problems. For users of make, this is normally done with configure. Hardcoding values is probably never the correct answer, because it's harder to patch hardcoded values than it is to regenerate the header - you can simply expect make users to do this I believe.

@xiaoyeli
Copy link
Owner Author

Choose a reason for hiding this comment

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

I removed superlu_dist_config.h from github repo.

@xiaoyeli
Copy link
Owner Author

Choose a reason for hiding this comment

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

also removed the serial version superlu_config.h

@rgommers
Copy link

Choose a reason for hiding this comment

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

Thank you @xiaoyeli!

@dschmitz89
Copy link

Choose a reason for hiding this comment

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

Thanks @xiaoyeli , I will update scipy's SuperLU accordingly in the coming weeks.

Please sign in to comment.