bkochuna / ners570f23-SpMV

1 stars 0 forks source link

Add tests for the CSR matvec method #31

Open KyleVaughn opened 11 months ago

KyleVaughn commented 11 months ago

Description:

This task adds a program to test the CSR matvec method. This would be considered a Unit Test. This task does not directly satisfy any of the functional requirements, but it is critical to the requirement "The library shall provide an interface to perform sparse matrix vector multiplication". Tests are an important in programming because they ensure the program performs as expected in a variety of use cases.

Tasks:

Lazy-Beee commented 11 months ago

I would suggest using the function interface virtual fp_type* matvec(fp_type* vecin) and I'll add this to the SparseMatrix class.


Edit: change the api to avoid memory leak void matvec(fp_type* vecin, fp_type* vecout)

KyleVaughn commented 11 months ago

Review by @marchier

marchier commented 11 months ago

Description section for the CSR matvec operation looks well defined

For the Tasks section, include the label that the inputs you are looking for are in the CSR format (ex. mention that a "valid input" is in the CSR format)

For the Definition of done section, it should be specified that the tests are for the CSR format and matvec operation

jacob-umich commented 11 months ago

@marchier

For the Tasks section, include the label that the inputs you are looking for are in the CSR format (ex. mention that a "valid input" is in the CSR format)

Wouldn't valid input just be a vector? Those don't have any specific format, right?


it should be specified that the tests are for the CSR format and matvec operation.

It has been clarified!

marchier commented 11 months ago

@marchier

For the Tasks section, include the label that the inputs you are looking for are in the CSR format (ex. mention that a "valid input" is in the CSR format)

Wouldn't valid input just be a vector? Those don't have any specific format, right?

For the tasks section suggestion, I meant that you should clarify that the tasks should all mention that they are for the CSR format (similar to the Definition of Done suggestion, I apologize for the unclear review comment)

jacob-umich commented 11 months ago

Roger that. it has been updated for clarity. Thank you for the feedback.