search

ralna / spral

Sparse Parallel Robust Algorithms Library
https://ralna.github.io/spral/
Other
104 stars 27 forks source link

METIS 5 with 64bit integers only supported if --with-metis-inc-dir passed to configure #136

Open jfowkes opened 11 months ago

jfowkes commented 11 months ago

It turns out that the current autotools build system only supports 64bit METIS 5 index types if --with-metis-inc-dir=/path/to/metis.h is passed to configure. This is not documented anywhere and clearly confusing for users.

As a result anyone who tries to build SPRAL with 64bit METIS without passing --with-metis-inc-dir=/path/to/metis.h to configure will almost certainly experience segfaults. We should update the documentation to support this use case.

jfowkes commented 11 months ago

@mjacobse do you have any thoughts on a better way of doing this? Not necessarily in autotools, the plan is to transition over to a Meson build system (see #141) but the SPRAL_HAVE_METIS_H and SIZEOF_IDX_T defines are a pain point (see #135).

For reference support for 64bit METIS 5 index types was added in commit ff78163

mjacobse commented 11 months ago

I think it would be reasonable to require a Metis header to be available and otherwise error out of the compilation. So basically get rid of SPRAL_HAVE_METIS_H and the default case if it is not defined. Then always check sizeof(idx_t) and define SIZEOF_IDX_T (or perhaps a better name like SPRAL_SIZEOF_METIS_IDX_T). That might require users to specify an extra path for the include (if not using default system paths), but that seems like better practice to me anyways. Just linking a C library and assuming declarations rather than getting them from a header is kind of asking for problems.

jfowkes commented 11 months ago

Thanks @mjacobse I like your suggestion, it's probably not worth the effort of changing it for the autotools build system but we should definitely be able to do this with Meson.