search

BallisticLA / RandBLAS

A header-only C++ library for sketching in randomized linear algebra
https://randblas.readthedocs.io/en/latest/
Other
64 stars 4 forks source link

Dense sketching operators: avoid int overflow when returning RNGState #63

Closed kaiwenhe7 closed 1 year ago

rileyjmurray commented 1 year ago

Thanks for the PR, @kaiwenhe7! The tests are failing on macOs when OpenMP isn't installed. Please take a look when you get a chance: https://github.com/BallisticLA/RandBLAS/actions/runs/5452751148/jobs/9920682956?pr=63.

kaiwenhe7 commented 1 year ago

@rileyjmurray Is there a way of disabling openmp so that I can do testing in the case that a machine does not have openmp installed? I don't want to try and mess with the cake files too much.

rileyjmurray commented 1 year ago

@kaiwenhe7 use

#if defined(RandBLAS_HAS_OpenMP)
  ... code ...
#endif

Edit: I'll just make this change now.

rileyjmurray commented 1 year ago

Okay, the basic build passes with this change, but the address sanitizer build fails. Compiler warnings for the address sanitizer build show that some values (ctr_arr, thrd) aren't being initialized properly; see here. This isn't surprising given how I just slapped on the compiler directives to the problematic lines without looking for other places where the directives were needed.

@kaiwenhe7 I'll let you take it from here.

kaiwenhe7 commented 1 year ago

@rileyjmurray I think it should be fixed now. The preexisting tests pass, which means that its returning the correct counter, but I am writing additional tests to make sure.

rileyjmurray commented 1 year ago

Ah shoot, in that case I may have jumped the gun. Please make a new PR with any tests you'd like.