Adding a fix and a simple test

[TeachRaccooon]

Adding a fix (and a simple test) for issue #57 that has been affecting RandLAPACK.

[TeachRaccooon]

@kaiwenhe7 Can you take a look at the failing test case?

[kaiwenhe7]

Yes I can do that

[kaiwenhe7]

I have a few things in my mind that may be causing the issue. I'm looking at the functions fill_dense and fill_dense_submat and for some reason I feel like there is some information being lost about the submatrix. I cloned the state_fix branch onto mine. I want to rewrite your test just using fill_dense_submat_impl, and I can get back to you.

[kaiwenhe7]

@TeachRaccooon In your second call for generating the bottom half of the random matrix, I think you should use the function fill_dense_submat with i_off = 0 and j_off = 5. When you call fill_dense, fill_dense calls full_dense_submat with i_off and j_off = 0, but the start of the new submatrix starts at index 2 of the array of random numbers associated with counter given by next_state. I can test it myself and get back.

[TeachRaccooon]

@TeachRaccooon In your second call for generating the bottom half of the random matrix, I think you should use the function fill_dense_submat with i_off = 0 and j_off = 5. When you call fill_dense, fill_dense calls full_dense_submat with i_off and j_off = 0, but the start of the new submatrix starts at index 2 of the array of random numbers associated with counter given by next_state. I can test it myself and get back.

One quick note is that in this test, i am assuming column-major storage, so the second call will be filling the right half of the matrix. About the need to use fill_dense_submat on the subportion of a given matrix, shouldn't the user have an option to just call the regular fill_dense (for example, when column-major storage is used and numrows(sumbat) == numrows(mat))? I am not seeing the reason why this should not work. @rileyjmurray we spoke about something along these lines briefly yesterday, what are your thoughts?

In any case, @kaiwenhe7, if you have a fix, please apply it.

[kaiwenhe7]

@TeachRaccooon I have the fix on my branch.

There shouldn't be an issue if the user wants to call fill_dense to generate a random matrix one and done. However, in the concatenation, we should be calling fill_dense_submat at least for the second half because the second half is part of a larger random matrix. There needs to be additional information about the starting pointer of the sub matrix within the larger matrix that fill_dense does not take in.

[TeachRaccooon]

@TeachRaccooon I have the fix on my branch.

There shouldn't be an issue if the user wants to call fill_dense to generate a random matrix one and done. However, in the concatenation, we should be calling fill_dense_submat at least for the second half because the second half is part of a larger random matrix. There needs to be additional information about the starting pointer of the sub matrix within the larger matrix that fill_dense does not take in.

Why not just pass an offset pointer into fill_dense? But anyway, we can discuss this later; for now i need this fix applied so that i can continue working on RandLAPACK.

You can either open a new PR or, presumably, just push your changes into this branch.

[rileyjmurray]

@kaiwenhe7

However, in the concatenation, we should be calling fill_dense_submat at least for the second half because the second half is part of a larger random matrix. There needs to be additional information about the starting pointer of the sub-matrix within the larger matrix that fill_dense does not take in.

I don't fully agree with this. I want us to assume the user has an initial sketching operator S1, and then they want a new sketching operator S2 with the same aspect ratio (wide vs tall) and same major axis (short vs long) as S1. If this happens, then the implicit matrix [S1; S2] (or [S1, S2], depending on dimensions) is the same as if we asked for a matrix S which is the same size as [S1; S2] (again, or [S1, S2]) from the beginning. Note that we're only asking that this be possible when (S1, S2) have the same aspect ratio and the same major axis.