-
Notifications
You must be signed in to change notification settings - Fork 2
Vector ops + Matrix select #9
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
Conversation
.. currently only double and int values
.. also a small change to VectorHandler to produce vectors with a size of > 1 (before it was always one when debugging)
.. ! Thunk not mapped yet
| } | ||
| } | ||
|
|
||
| // behavior of "GrB_assign" |
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.
Are these supposed to be commented?
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 added those, as the testGrBMatrixAssign method was not used otherwise. However these are failing, so thats why I commented them out.
I will have a look, why they are failing
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.
Getting some GrB_DIMENSION_MISMATCH, but don't see the cause (my hunch was that the into matrix needs to have the same dimensions as mat but thats not resolving it).
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 added a FIXME and removed the test-cases for now.
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.
OK, I'll merge and try to understand what's happening.
Maybe it's a bug with assign
| GRBOPSMAT.select(out, null, null, selectOp, mat, null, null) shouldBe GRBCORE.GrB_SUCCESS | ||
|
|
||
| // check could be improved | ||
| mt.vals.filter(p => (p._1 == p._2)).size shouldBe GRBCORE.nvalsMatrix(out) |
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.
you can use extract from SparseMatrixHandler then simple equals against the filtered mt.vals you've got here
| GrB_Descriptor d = desc != NULL ? (GrB_Descriptor) (*env)->GetDirectBufferAddress(env, desc) : NULL; | ||
|
|
||
| long res = check_grb_error(GrB_assign(out, m, acc, first, I, grb_nj, J, grb_nj, d)); | ||
| long res = check_grb_error(GrB_assign(out, m, acc, first, I, grb_ni, J, grb_nj, d)); |
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.
good one, I don't know how this didn't show up in tests
| public static int GxB_MODE = 2; // mode passed to GrB_init (blocking or non-blocking) | ||
| public static int GxB_THREAD_SAFETY = 3; // thread library that allows GraphBLAS to be thread-safe for user threads. | ||
| public static int GxB_THREADING = 4; // thread library used for internal GraphBLAS threads | ||
| //public static int GxB_LIBRARY_NAME = 8; // name of the library (char *) |
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.
Can you remove the comments?
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.
Do you also mean the comments describing the fields or just the unsupported fields?
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.
just the unsupported fields
fabianmurariu
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.
LGTM
based on #8.
This PR also uses the redundant code-lines for handling of
GrB_ALL. So refactoring this part into a seperate function in C would be really useful.(This is my latest state)