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

MPFR locating #903

Open
VictorEijkhout opened this issue Feb 20, 2025 · 19 comments
Open

MPFR locating #903

VictorEijkhout opened this issue Feb 20, 2025 · 19 comments

Comments

@VictorEijkhout
Copy link

Previous ticket 320 is un-re-openable.

[staff suitesparse:663] echo $CMAKE_PREFIX_PATH
/work2/00434/eijkhout/mpfr/installation-mpfr-4.2.1-clx-intel23.1:more......

and

[staff suitesparse:665] ls /work2/00434/eijkhout/mpfr/installation-mpfr-4.2.1-clx-intel23.1
include/  lib/  share/

but suitesparse still reports

-- gmp static:  /usr/lib64/libgmp.so
-- from mpfr.h file: #define MPFR_VERSION_STRING "3.1.1"
CMake Error at /opt/apps/cmake/3.24.2/share/cmake-3.24/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find MPFR: Found unsuitable version "3.1.1", but required is at
  least "4.0.2" (found /usr/lib64/libmpfr.so.4.1.1)
Call Stack (most recent call first):
  /opt/apps/cmake/3.24.2/share/cmake-3.24/Modules/FindPackageHandleStandardArgs.cmake:592 (_FPHSA_FAILURE_MESSAGE)
  cmake_modules/FindMPFR.cmake:91 (find_package_handle_standard_args)
  CMakeLists.txt:97 (find_package)

This is the latest 7.8.3 release

@mmuetzel
Copy link
Contributor

Do you have multiple versions of mpfr.h on your build host? Where are they located?

@mmuetzel
Copy link
Contributor

mmuetzel commented Feb 20, 2025

As a potential fix, you could try to configure with -DMPFR_INCLUDE_DIR="/work2/00434/eijkhout/mpfr/installation-mpfr-4.2.1-clx-intel23.1/include".
That would maybe "fix" the results of the CMake module.
However even if that works, it would still depend on the order of preprocessor -I flags which header will actually be used when building the library. They might already be in the correct order though (depending on where your other mpfr.h files are installed).

@VictorEijkhout
Copy link
Author

Ok, that worked. Now I get the same problem with GMP: the .pc file is nicely on the PKG_CONFIG_PATH but you ignore that so I added an explicit flag.

@VictorEijkhout
Copy link
Author

Say, what happened to #321 ?

Is that not yet in the latest release?

@mmuetzel
Copy link
Contributor

Good point.
That PR got merged. But the changes from it were reverted:
32caf51

@DrTimothyAldenDavis: Was that intentional?

@VictorEijkhout
Copy link
Author

You know I just noticed that I was using the ?legacy? build where make invokes cmake. I've now switched to a full cmake-based install, and that actually seems to work without further commandline defines. So I was actually concluding that that PR was successful.

You and Tim will have to fight it out.....

@DrTimothyAldenDavis
Copy link
Owner

I can't recall. It was likely a mistake ... the primary SPEX repo is elsewhere and owned by someone else (I'm a collaborator). We do all the SPEX development on the other repo ( https://github.com/clouren/SPEX ) and then I copy major releases into the SuiteSparse repo and integrate them there. It's possible I accidentally overwrote this part.

@mmuetzel
Copy link
Contributor

Ah ok. That makes sense.

It would probably make sense to try and merge the changes from this repository (before that "merge" commit) and potential changes from the upstream repository in the two CMake modules in SPEX.
Would it be better to do that here? Or in the upstream repository? (In case it is the latter: Would that be for the SPEX-3.3.0 branch?)

@DrTimothyAldenDavis
Copy link
Owner

Yes, I'll take a look at that. I will merge the two CMakeLists.txt files and post an update to it in the SPEX-3.3.0 branch ( https://github.com/clouren/SPEX/tree/SPEX-3.3.0 ). That's where our current development is going on.

Alternatively, I could perhaps merge these changes into the SPEX-3.2.2 branch, which is a minor update from the stable v3.2.1 release of SPEX.

It will take some juggling so I'll take care of it, one way or the other.

@mmuetzel
Copy link
Contributor

Thanks for taking that on. Please, let me know if I can be of assistance.

The relevant changes are probably mostly in the files FindGMP.cmake and FindMPFR.cmake.
You could probably just take the versions from immediately before that SPEX "merge" commit in this repository and apply the relevant parts of b09df58#diff-543ba362312a405a300bc5ed1597555c7da123d3f11ba2650b4f85e48edc90f6 on top of them.

As far as I can see, there haven't been any relevant changes to these files in the upstream repository.

@mmuetzel
Copy link
Contributor

Looking at other parts of b09df58:
There should be no reason to explicitly look for the GMP or MPFR packages. The SPEX module should do that automatically.
If looking for these packages is indeed necessary, it is likely a bug that should be fixed.

@DrTimothyAldenDavis
Copy link
Owner

it looks like I can just copy the updated FindGMP.cmake and FindMPFR.cmake files from that PR, into the SPEX/cmake_modules folder. It has a PREPEND instead of APPEND, though. Not sure which would be needed.

@mmuetzel
Copy link
Contributor

It has a PREPEND instead of APPEND, though. Not sure which would be needed.

Following the principle of "the user is always right", a PREPEND is probably the better choice.
If that breaks something, that is the user's fault (following that same principle).

@mmuetzel
Copy link
Contributor

it looks like I can just copy the updated FindGMP.cmake and FindMPFR.cmake files from that PR, into the SPEX/cmake_modules folder.

It looks like the changes from 4f0af11 and ee6b694 are still lost with the latest version of these files in the dev2 branch.

I don't know what motivated those changes though. As far as I can tell, they would lead to a failing detection if these libraries can only be found in their static version (i.e., with those changes only shared gmp and mpfr libraries are accepted).
The commit message or the comments don't point to a reason for that change. So, I'm not sure if that was intentional.

@DrTimothyAldenDavis: Do you recall which issue(s) triggered these modifications to the detection logic?

@DrTimothyAldenDavis
Copy link
Owner

I'm pretty sure it was unintentional. The Spex development is done in a separate repo, and from time to time I copy over the code from there. I just accidentally over-wrote the update to the cmake module folder in SuiteSparse.

I'll look at those commits and bring back the latest version from them.

@DrTimothyAldenDavis
Copy link
Owner

Looking at other parts of b09df58:
There should be no reason to explicitly look for the GMP or MPFR packages. The SPEX module should do that automatically.
If looking for these packages is indeed necessary, it is likely a bug that should be fixed.

I can't recall which system/compiler/OS it was that I had to do that for.

@mmuetzel
Copy link
Contributor

I've opened #905 with some follow-up changes for these CMake Find modules.

@DrTimothyAldenDavis
Copy link
Owner

I've merged in #905. I hope to post a beta release of SuiteSparse v7.9.0 soon, with these updates to SPEX, LAGraph, GraphBLAS, etc.

@mmuetzel
Copy link
Contributor

@VictorEijkhout: Does using the pkg-config files of GMP and MPFR work for you again with SuiteSparse 7.9.0?

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

No branches or pull requests

3 participants