Skip to content

Update the Julia interface of ColPack#30

Merged
amontoison merged 5 commits intomasterfrom
colpack
Mar 30, 2025
Merged

Update the Julia interface of ColPack#30
amontoison merged 5 commits intomasterfrom
colpack

Conversation

@amontoison
Copy link
Copy Markdown
Member

@amontoison amontoison commented Mar 26, 2025

  • Recompiled version of ColPack without OpenMP. It seems to fix the non-determinism.
  • Add more options for order and method in the C++ code like "NAIVE_STAR" or "DYNAMIC_LARGEST_FIRST".
  • Reimplement the C++ class Timer such that it works and that we have highly accurate timers directly from ColPack for the ordering and coloring.
  • Add a folder gen to automatically regenerate the Julia wrappers in libcolpack.jl from the header cinterface.h if the C interface of ColPack is updated.
  • Provide three new routines to get the ordering from ColPack as well as timers for coloring and ordering.

Comments:

  • Even in the serial version of ColPack, we still have some minor bugs. The ordering LARGEST_FIRST returns invalid ordering that is not a permutation is some rare cases. The method TRIANGULAR fails with segmentation fault.
  • The interface for CSC matrices that I implemented in ColPack seems to provide some errors sometimes, I have an idea of what is the issue but I don't think that it is relevant to spend too much time on it. We can do the conversion CSC -> CSR -> ADOLC format in Julia.
    The ADOLC reader was not modified by my commits.
    The CSR should work but it was buggy before and I fixed it, so for the current paper it is more relevant to use as much as possible things not added by me in the C++ code (ADOLC format / MTX files).

@amontoison amontoison requested a review from gdalle March 26, 2025 06:01
Copy link
Copy Markdown
Collaborator

@gdalle gdalle left a comment

Choose a reason for hiding this comment

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

Looks good to me, I presume the timer tests are deactivated because timers are not initialized?
Can we add some tests for the new ordering retrieval, then this should be good to go?

@amontoison amontoison requested a review from gdalle March 29, 2025 06:12
@gdalle
Copy link
Copy Markdown
Collaborator

gdalle commented Mar 30, 2025

Once this is merged we could do a new release of ColPack.jl? If I understand correctly, we now go through the ADOL-C format systematically, which might fix #21?

@amontoison
Copy link
Copy Markdown
Member Author

The format ADOL-C has been used for a long time in ColPack, and it's quite safe to use.
The other formats, CSR and CSC, are less tested, and I found a few bugs.
Even though I fixed them in my fork, we should play it safe.
If issue #21 is fixed, it's because ColPack was recompiled without OpenMP; it's not related to the graph's input format.

@amontoison amontoison merged commit 1c3e86f into master Mar 30, 2025
4 of 9 checks passed
@amontoison amontoison deleted the colpack branch March 30, 2025 18:20
@gdalle
Copy link
Copy Markdown
Collaborator

gdalle commented Mar 30, 2025

At the beginning of #21 we had also found a case where the binary was deterministic but the coloring through CSR/CSC was random, that's what I was referring to here

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