bkochuna / ners570f23-SpMV

1 stars 0 forks source link

Add CSR view tests #56

Open KyleVaughn opened 11 months ago

KyleVaughn commented 11 months ago

Description:

Add a test for the CSR view function, which is to check if the view function of the SparseMatrix_CSR class is correct.

Tasks:

Definition of done:

To successfully test the CSR view function that prints out the matrix.

KyleVaughn commented 11 months ago

Review by @tjayasa

tjayasa commented 11 months ago

For the description, it might be a good idea to explicitly define what you mean by "correct". Do you mean that the formatting is correct, or that the values that are printed out, or both?

For the second task you listed, I think it might be a good idea to use a few different matrices of different sizes and structures in the tests just for the sake of thoroughness. (Second Task) For the third task you listed, it would be good to specify how exactly you are going to see that the outputs match your expectations. Are you going to use assert statements and, if so, what kinds?

The definition of done looks good.

Overall, this looks great, but could benefit from a little more specificity in certain places.

lwh1106 commented 11 months ago

@tjayasa Thanks for your comments! By saying "correct", it primarily means the results (values) are correct -- we can also do the formatting, but different persons have different definitions of formats, so it may not be as important as the correct values. Here comes the issue of correctness. I think a good way is to check the infinite norm of the results, i.e., the maximum differences between our way of calculation and view function and the nominal results. Yes, you are correct. During this process, we can use different sizes with different values to test the robustness of our functions.

Again, thanks for your inspiring comments! I will pay attention to the issue mentioned above in my test.

lwh1106 commented 10 months ago

@tjayasa Sorry, I am a bit late (hopefully not the very last minute) to finish this test! Now the test is over there (in the directory of spmv/tests/CSR_View_Test), and you are free to review my work! It works in the version pushed by me at midnight, but now, as there is some update for the assemble stuff, the branch may not be built, but the view test is still OK and works! You can also see the discussion of adding the CSR view method (https://github.com/bkochuna/ners570f23-SpMV/issues/52#issuecomment-1781019082). Thanks for your patience!