Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding CSR Sparse Matrix Dense Vector Multiplication Benchmark #281

Merged
merged 3 commits into from
Sep 7, 2023

Conversation

yxd97
Copy link
Contributor

@yxd97 yxd97 commented Aug 31, 2023

This benchmark generates a CSR sparse matrix and a dense vector with a linear feedback shift register RNG, multiplies them, and prints the matrix, intput, and output. I have verified it against the scipy implementation of matrix-vector multiplication.

Copy link
Owner

@sampsyo sampsyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! This is really ambitious. Nice work getting the end-to-end thing to work, including the random matrix generator!!

@sampsyo sampsyo merged commit 41c556f into sampsyo:main Sep 7, 2023
5 checks passed
free res;

good: int = const 0;
ret good;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you consider removing the return value of main? While I guess this hasn't been specified, afaik there isn't any implementation that will make use of this value. Most will silently ignore it like brili does.

brilirs rejects this program(I thought because of #91 but git blame predates that) which I think is fair?

Let me know what you think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Indeed there is no specifiction saying main shoud return a value to be used.
Shall I make a new PR for this?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! Yes please, a new PR would be great. (I suppose we should document this, and probably add a check in brilck…)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just created #293

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants