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

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

Open
jfowkes opened this issue Sep 18, 2023 · 3 comments
Assignees

Comments

@jfowkes
Copy link
Contributor

jfowkes commented Sep 18, 2023

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
Copy link
Contributor Author

jfowkes commented Sep 21, 2023

@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
Copy link
Collaborator

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
Copy link
Contributor Author

jfowkes commented Sep 21, 2023

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.

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

No branches or pull requests

2 participants