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

Using other LAGraph functions #44

Open
marci543 opened this issue Apr 14, 2020 · 5 comments
Open

Using other LAGraph functions #44

marci543 opened this issue Apr 14, 2020 · 5 comments

Comments

@marci543
Copy link
Contributor

marci543 commented Apr 14, 2020

I wanted to use LAGraph_cc_fastsv from pygraphblas as seen in the wrappers of other functions.
I realized that only a fraction of LAGraph header and source code is included, which I extended with LAGraph_cc_fastsv header and code (branch here) and rebuilt the Docker image.

Is there a more general solution which enables using other LAGraph functions (e.g. BFS, CC)? Is there a way to include the whole LAGraph in case anyone needs other algorithms?

Thank you for your help.

@michelp
Copy link
Collaborator

michelp commented Apr 22, 2020

Good question, previously pygraphblas required lagraph, but there was some pushback on that so I inlined what was useful. Unfortunately it's a compile time task to include the source of the specific function you want.

Maybe the internal lagraph stuff that is inlined could be bound to a different namespace, and restore the dynamically linked version for cases where people want to access the full library like you do.

@marci543
Copy link
Contributor Author

What was the reason behind the pushback? So what are the pitfalls to avoid?

@michelp
Copy link
Collaborator

michelp commented Apr 24, 2020

Some of the feedback I received was that a LAGraph dependency made it harder to install, made it more of a pylagraph than pygraphblas, and that the algorithms in lagraph were too early stage to be binding like that. My primary goal was to have the utility functions that are super useful, like reading data files and LAGraph_isequal(), so I inlined them.

Attending the LAGraph weekly call occasionally, I can see that there is still some uncertainty in where the lagraph interface is going to go, so actually I do feel fine inlining functions I intend to keep that may diverge in the future anyway.

That being said, I think it's possible to do both, I can restore the dynamic binding support and keep the inline functions, it will just be in some optional namespace like pygraphblas.lagraph that throws an error if you don't have lagraph installed, users can use the library but it will be optional for the core since the inlined functions are provided. Does that work for you?

@marci543
Copy link
Contributor Author

Yes. It is perfect for my purpose and brings the advantages of both option.

@marci543
Copy link
Contributor Author

marci543 commented Apr 24, 2020

I have an idea which can make the functions more user-friendly.

Is it possible to access the function signatures and wrap the function calls automatically?
I think the majority of LAGraph functions can be covered with the following conversions of the parameters:

  • pointers to GrB objects (GrB_Matrix*, GrB_Vector*, etc.) should be turned into allocation and returned as wrapper class instances (Matrix, Vector, etc.)
  • wrapped GrB objects should expose their pointers (e.g. A.matrix[0])

E.g. the following function can be converted like this:
lagraph_raw.bfs_pushpull(v_output: GrB_Vector*, pi_output: GrB_Vector*, A: GrB_Matrix, ...) -> GrB_Info
lagraph.bfs_pushpull(A: Matrix, ...) -> Tuple[Vector, Vector]

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

2 participants