- Notifications
You must be signed in to change notification settings - Fork15
Read Matrix Market withfast_matrix_market#391
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
coveralls commentedFeb 18, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Changes unknown |
eriknw commentedFeb 19, 2023
CC ,@alugowski
@alugowski, when writing Matrix Market files, do you recommend passing scipy CSR or CSC matrices instead of COO? I'm curious whether |
| if_output_type(A)is_Vector: | ||
| indices,values=A.to_coo(sort=False) | ||
| s=COO(indices,values,shape=A.shape) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
btw@SultanOrazbayev I just special-casedVector into_pydata_sparse, b/c previously it was creating a(1, n)-shaped sparse array from a Vector instead of a(n,)-shaped array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Oops, sorry about that!
But run `autoflake`, `isort`, `pyupgrade`, and `black` first (for now).
SultanOrazbayev left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks great,@eriknw !
Uh oh!
There was an error while loading.Please reload this page.
| try: | ||
| fromfast_matrix_marketimportmmwrite# noqa: F811 | ||
| exceptImportError:# pragma: no cover (import) | ||
| ifengine!="auto": | ||
| raiseImportError( | ||
| "fast_matrix_market is required to write Matrix Market files " | ||
| f'using the "{engine}" engine' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Not sure to what extent DRY principle is useful here, but perhaps there is scope for generalising this check for suitable import as some utility function (as a separate PR).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm open to such a PR :)
You may notice that my style sometimes doesn't adhere to DRY for small, straightforward (to me) things, but I suspect there are cases where things could be improved.
eriknw commentedMar 21, 2023
Thanks for reviewing@SultanOrazbayev! I just finished the final TODO for this PR--I added documentation to |
Uh oh!
There was an error while loading.Please reload this page.
Closes#384.
This uses
fast_matrix_market(which lookspretty fast) by default if available, but will fall back toscipyif necessary.scipyis still necessary to be installed even if using "fast_matrix_market" engine.There were three small issues I encountered that I plan to raise upstream, but I was able to work around them.
TODO:
fast-matrix-marketto get better performance:https://python-graphblas.readthedocs.io/en/latest/user_guide/io.htmlfrom_dense/to_dense