DrTimothyAldenDavis / GraphBLAS

SuiteSparse:GraphBLAS: graph algorithms in the language of linear algebra. For production: (default) STABLE branch. Code development: ask me for the right branch before submitting a PR. video intro: https://youtu.be/Tj5y6d7FegI .
http://faculty.cse.tamu.edu/davis/GraphBLAS.html
Other
364 stars 63 forks source link

Bug with `GrB_{Matrix,Vector}_assign`? 3.3.0 and 3.2.2 #9

Closed eriknw closed 4 years ago

eriknw commented 4 years ago

Best just to show. It's possible I'm not doing things quite right:

int main (void)
{
    GrB_Vector w, u ;
    GrB_Info info ;
    OK (GrB_init (GrB_NONBLOCKING)) ;
    OK (GrB_Vector_new (&w, GrB_INT64, 1)) ;
    OK (GrB_Vector_setElement_INT64 (w, 1, 0)) ;
    OK (GxB_print (w, 3)) ;

    OK (GrB_Vector_new (&u, GrB_FP64, 1)) ;
    OK (GrB_Vector_assign (u, NULL, NULL, w, GrB_ALL, 1, NULL)) ;
    OK (GxB_print (u, 3)) ;

    OK (GrB_Vector_setElement_FP64(u, 98.5, 0)) ;
    OK (GxB_print (u, 3)) ;

    OK (GrB_finalize ( )) ;
}

w Looks good so far:

  1x1 GraphBLAS int64_t vector, sparse by col:
  w, 1 entry

    (0,0)   1

u after assign. Wait, why is u an int64_t now?!

  1x1 GraphBLAS int64_t vector, sparse by col:
  u, 1 entry

    (0,0)   1

u after set element. Indeed, I can't assign a float to u:

  1x1 GraphBLAS int64_t vector, sparse by col:
  u, 1 entry

    (0,0)   98

Now for another example:

int main (void)
{
    GrB_Vector w, u ;
    GrB_Info info ;
    OK (GrB_init (GrB_NONBLOCKING)) ;
    OK (GrB_Vector_new (&w, GrB_INT64, 1)) ;
    OK (GrB_Vector_setElement_INT64 (w, 1, 0)) ;
    OK (GxB_print (w, 3)) ;

    OK (GrB_Vector_new (&u, GrB_FP64, 1)) ;
    OK (GrB_Vector_setElement_FP64(u, 42.5, 0)) ;   // NEW
    OK (GxB_print (u, 3)) ;                         // NEW

    OK (GrB_Vector_assign (u, NULL, NULL, w, GrB_ALL, 1, NULL)) ;
    OK (GxB_print (u, 3)) ;

    OK (GrB_Vector_setElement_FP64(u, 98.5, 0)) ;
    OK (GxB_print (u, 3)) ;

    OK (GrB_finalize ( )) ;
}

This outputs:

  1x1 GraphBLAS int64_t vector, sparse by col:
  w, 1 entry

    (0,0)   1

  1x1 GraphBLAS double vector, sparse by col:
  u, 1 entry

    (0,0)    42.5

  1x1 GraphBLAS double vector, sparse by col:
  u, 1 entry

    (0,0)    4.94066e-324

  1x1 GraphBLAS double vector, sparse by col:
  u, 1 entry

    (0,0)    98.5

So, u stays a double, but now the assignment into it from w gives bizarre results.

I tested this on my personal build on OS X with versions 3.2.2 and 3.3.0.

xref: https://github.com/jim22k/grblas/issues/25

DrTimothyAldenDavis commented 4 years ago

Thanks for the bug report. I recently updated the setElement methods and must have introduced a bug; somehow my tests missed this case. I'll track it down shortly.

On Sun, Jun 28, 2020 at 6:20 PM Erik Welch notifications@github.com wrote:

Best just to show. It's possible I'm not doing things quite right:

int main (void) { GrB_Vector w, u ; GrB_Info info ; OK (GrB_init (GrB_NONBLOCKING)) ; OK (GrB_Vector_new (&w, GrB_INT64, 1)) ; OK (GrB_Vector_setElement_INT64 (w, 1, 0)) ; OK (GxB_print (w, 3)) ;

OK (GrB_Vector_new (&u, GrB_FP64, 1)) ;
OK (GrB_Vector_assign (u, NULL, NULL, w, GrB_ALL, 1, NULL)) ;
OK (GxB_print (u, 3)) ;

OK (GrB_Vector_setElement_FP64(u, 98.5, 0)) ;
OK (GxB_print (u, 3)) ;

OK (GrB_finalize ( )) ;

}

w Looks good so far:

1x1 GraphBLAS int64_t vector, sparse by col: w, 1 entry

(0,0)   1

u after assign. Wait, why is u an int64_t now?!

1x1 GraphBLAS int64_t vector, sparse by col: u, 1 entry

(0,0)   1

u after set element. Indeed, I can't assign a float to u:

1x1 GraphBLAS int64_t vector, sparse by col: u, 1 entry

(0,0)   98

Now for another example:

int main (void) { GrB_Vector w, u ; GrB_Info info ; OK (GrB_init (GrB_NONBLOCKING)) ; OK (GrB_Vector_new (&w, GrB_INT64, 1)) ; OK (GrB_Vector_setElement_INT64 (w, 1, 0)) ; OK (GxB_print (w, 3)) ;

OK (GrB_Vector_new (&u, GrB_FP64, 1)) ;
OK (GrB_Vector_setElement_FP64(u, 42.5, 0)) ;   // NEW
OK (GxB_print (u, 3)) ;                         // NEW

OK (GrB_Vector_assign (u, NULL, NULL, w, GrB_ALL, 1, NULL)) ;
OK (GxB_print (u, 3)) ;

OK (GrB_Vector_setElement_FP64(u, 98.5, 0)) ;
OK (GxB_print (u, 3)) ;

OK (GrB_finalize ( )) ;

}

This outputs:

1x1 GraphBLAS int64_t vector, sparse by col: w, 1 entry

(0,0)   1

1x1 GraphBLAS double vector, sparse by col: u, 1 entry

(0,0)    42.5

1x1 GraphBLAS double vector, sparse by col: u, 1 entry

(0,0)    4.94066e-324

1x1 GraphBLAS double vector, sparse by col: u, 1 entry

(0,0)    98.5

So, u stays a double, but now the assignment into it from w gives bizarre results.

I tested this on my personal build on OS X with versions 3.2.2 and 3.3.0.

xref: jim22k/grblas#25 https://github.com/jim22k/grblas/issues/25

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/DrTimothyAldenDavis/GraphBLAS/issues/9, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYIIOKPWILKRFHGK5D5LHTRY7F4VANCNFSM4OKYASUQ .

DrTimothyAldenDavis commented 4 years ago

Found it. The bug is in Source/GB_dense_subassign_24, which should have handled typecasting but didn't. The bug appears first in version 3.2.0, through 3.3.0. Fixed in 3.3.1 which I'll post shortly.

DrTimothyAldenDavis commented 4 years ago

Give it a try with the draft v3.3.1 I just posted to github on the master branch. It fixes this bug, but I need to go through all of my extensive test suite before posting it as the public v3.3.1.

eriknw commented 4 years ago

Hooray! I'll give this a try shortly and will report back if it didn't fix it.

eriknw commented 4 years ago

Works for me!

DrTimothyAldenDavis commented 4 years ago

Sounds good. The update passes all my tests with 100% coverage but doesn’t actually exercise the typecast. So I will add that to my tests just to be sure, before I call this 3.3.1.

On Mon, Jun 29, 2020 at 5:27 PM Erik Welch notifications@github.com wrote:

Works for me!

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/DrTimothyAldenDavis/GraphBLAS/issues/9#issuecomment-651402654, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYIIOMBV4O37HHP2D6SLL3RZEINRANCNFSM4OKYASUQ .

-- Sent from Gmail Mobile