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

Add square matrix scaling graph #43

Merged
merged 1 commit into from
May 20, 2024

Conversation

llewelld
Copy link
Contributor

Adds a graph to the benchmarking README.md to show the time taken to multiply square matrices of increasing size.

Results CSV file, graph PNG and graph SVG file are included.

@llewelld llewelld requested a review from mhauru May 19, 2024 10:02
Copy link
Contributor

@mhauru mhauru left a comment

Choose a reason for hiding this comment

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

LGTM

@mhauru
Copy link
Contributor

mhauru commented May 20, 2024

The performance of C naive with 10 threads is really interesting. Why does it lose some of its edge against single threaded when matrices get bigger?

Nothing to do with whether to merge this though.

@llewelld
Copy link
Contributor Author

The performance of C naive with 10 threads is really interesting. Why does it lose some of its edge against single threaded when matrices get bigger?

Thanks for the review. I'll merge in, but I'd also be interested to know the answer to your question.

In practice, it's losing its edge only in comparison with the transposed version, no? At the top end it's consistently faster than the non-transposed versions. Or am I misreading?

In this case, maybe it's a sign that the different memory cache levels are being exhausted?

Having reviewed these two, would you be up for taking a look at #40 as well? No obligation, but it would be great to get your feedback on it.

Adds a graph to the benchmarking README.md to show the time taken to
multiply square matrices of increasing size.

Results CSV file, graph PNG and graph SVG file are included.
@llewelld
Copy link
Contributor Author

Rebased.

@llewelld llewelld merged commit 72251c2 into alan-turing-institute:main May 20, 2024
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.

None yet

2 participants