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 new projects ParU and LAGraph to CI #450

Merged
merged 4 commits into from
Oct 20, 2023

Conversation

mmuetzel
Copy link
Contributor

@mmuetzel mmuetzel commented Oct 19, 2023

For the runners that are building on emulated hardware, GraphBLAS isn't built. (It would take too long.) So, LAGraph can't be built on them either. This PR adds it only for the "native" runners.
ParU is added in both the "native" runners and the "emulated" runners.

This also revealed an oversight in GraphBLAS. (The include directory was set incorrectly. But that only affects the import target before it is installed. So, probably not a huge problem downstream.)

Some of the runners failed to build for different reasons in my fork.
Could this PR be merged anyway? That would simplify checking follow-up changes that could fix these build issues. (Imho, a temporarily failing CI is completely ok. In the end, that's what CI is for.)

It is an import target now. But it defines all variables of the module
for backward compatibility.
Prefer GraphBLAS version built in common source tree.
Update version of GraphBLAS.
Do not override GraphBLAS target with own module.
@mmuetzel
Copy link
Contributor Author

Looking at the local FindGraphBLAS.cmake from the LAGraph project: Should the CMake target name that is exported by GraphBLAS be GraphBLAS::GraphBLAS? It is currently SuiteSparse::GraphBLAS for the GraphBLAS project that is built as part of SuiteSparse.

@DrTimothyAldenDavis
Copy link
Owner

Regarding the LAGraph FindGraphBLAS.cmake: that script can find SuiteSparse/GraphBLAS, but it can also find any other GraphBLAS-compliant library, which might not have a modern cmake build script, or might not have a cmake at all.

When LAGraph is inside SuiteSparse, however, it should just find SuiteSparse/GraphBLAS, by default.

It's possible that the stand-alone cmake build system at /~https://github.com/GraphBLAS/LAGraph, and the SuiteSparse cmake build system for LAGraph as library inside SuiteSparse, can't be made identical. If that's the case, we can just keep them different, and sync the source code itself every so often, between the /~https://github.com/GraphBLAS/LAGraph repo and mine here.

We can leave LAGraph out of the github workflow. The tests are run in the main repo, at /~https://github.com/GraphBLAS/LAGraph . For this copy, I renamed the LAGraph/.github/workflow folder to LAGraph/github_workflow, so it would run automatically.

@DrTimothyAldenDavis DrTimothyAldenDavis merged commit 0900858 into DrTimothyAldenDavis:dev2 Oct 20, 2023
@mmuetzel
Copy link
Contributor Author

Regarding the LAGraph FindGraphBLAS.cmake: that script can find SuiteSparse/GraphBLAS, but it can also find any other GraphBLAS-compliant library, which might not have a modern cmake build script, or might not have a cmake at all.

When LAGraph is inside SuiteSparse, however, it should just find SuiteSparse/GraphBLAS, by default.

Thanks for clarifying.
Trying to stay compatible with older (or different) versions of GraphBLAS might be possible. I'll try to think about how that could be done.

For ParU, that would likely be more complicated because it depends on more libraries with additional transient dependencies. Those additional dependencies haven't been reflected very well before the recent changes to the build system.

We can leave LAGraph out of the github workflow.

It's only left out for the arch-emu runners (the ones that use qemu to emulate hardware). It's part of the runners from the build group.
I didn't check if arch-emu could build GraphBLAS in an acceptable amount of time when configuring with -DCOMPACT=ON. Trying to test that is on my To-Do for a while now already...

@DrTimothyAldenDavis
Copy link
Owner

ParU doesn't need to be synced with the @Aznaveh's repo in a way that differs from SuiteSparse. It's designed to reside fully inside SuiteSparse as a SuiteSparse-specific library.

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.

2 participants