Skip to content

Final report#79

Merged
dstansby merged 37 commits into
mainfrom
final-report
Oct 14, 2025
Merged

Final report#79
dstansby merged 37 commits into
mainfrom
final-report

Conversation

@dstansby
Copy link
Copy Markdown
Contributor

@dstansby dstansby commented Jun 2, 2025

This is a PR for the final report, in the form of the website. I intend for this to be pretty much the last thing we merge, but opening as a draft so everyone knows I'm working on this, and so I can use it to iterate on what graphs we want to put in here to tell the story.

Fixes #109
Fixes #108

@dstansby dstansby marked this pull request as ready for review August 20, 2025 07:05
@dstansby dstansby requested review from K-Meech and ruaridhg August 20, 2025 07:05
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
@dstansby
Copy link
Copy Markdown
Contributor Author

@ruaridhg @K-Meech could you review what's here? I still need to add sections for chunk size and different data (image/sparse/dense), but would be good to review what's here, and I'll open issues to remind myself to finish off the other sections after this PR.

Copy link
Copy Markdown
Contributor

@K-Meech K-Meech left a comment

Choose a reason for hiding this comment

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

Thanks @dstansby - looks good! I put some comments below.

Some more general points:

  • We should give some details about the hardware the benchmarks were run on e.g. OS, RAM, number of cores, maybe type of storage (SSD type), as we are using the full time to read/write to disk?
  • It would be good to mention that the points on the graphs are the mean values from 5 runs
  • Exact numbers will have to be updated throughout once the benchmarks are run again with more iterations. I think a number of these plots aren't up to date with the latest example data?

Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Copy link
Copy Markdown
Contributor

@ruaridhg ruaridhg left a comment

Choose a reason for hiding this comment

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

Looks good, just made a few minor suggestions.

Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
dstansby and others added 4 commits September 2, 2025 17:01
Co-authored-by: ruaridhg <32329546+ruaridhg@users.noreply.github.com>
Co-authored-by: ruaridhg <32329546+ruaridhg@users.noreply.github.com>
@dstansby
Copy link
Copy Markdown
Contributor Author

dstansby commented Sep 2, 2025

RIGHT - I think I have included all the graphs 🎉 @K-Meech & @ruaridhg I'd appreciate if you could give this another review when you have time to check that I haven't missed anything. If I haven't, we can merge and the benchmarking will be done!

@dstansby dstansby requested review from K-Meech and ruaridhg September 2, 2025 16:06
@dstansby
Copy link
Copy Markdown
Contributor Author

dstansby commented Sep 2, 2025

(actually, not quite done, we should address #110 before considering this finsihed)

Copy link
Copy Markdown
Contributor

@ruaridhg ruaridhg left a comment

Choose a reason for hiding this comment

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

Just left a couple of comments for things that could/should be added.

Comment thread docs/index.md Outdated
Comment thread docs/index.md
Copy link
Copy Markdown
Contributor

@ruaridhg ruaridhg left a comment

Choose a reason for hiding this comment

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

I've added alt text for all the plots as I think this is what @K-Meech meant

Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
@K-Meech
Copy link
Copy Markdown
Contributor

K-Meech commented Sep 16, 2025

@dstansby I'm part way through reviewing the final report - should be able to finish this tomorrow. I wasn't sure if the plots / text had been updated to match the latest results?

E.g. running python src/zarr_benchmarks/create_plots.py --example_results on the main branch + looking at data/plots/heart/write/format_v2, seems to show a different plot to the first graph used in the report? The one in the report has multiple points below 1 second, while most in the main plot are above 1 second write time.

dstansby and others added 3 commits September 17, 2025 10:10
Co-authored-by: ruaridhg <32329546+ruaridhg@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@K-Meech K-Meech left a comment

Choose a reason for hiding this comment

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

Thanks @dstansby - looks good! I've put some comments below:

Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Copy link
Copy Markdown
Contributor

@ruaridhg ruaridhg left a comment

Choose a reason for hiding this comment

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

Added more detailed alt text descriptions if necessary

Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
Comment thread docs/index.md Outdated
dstansby and others added 8 commits October 9, 2025 14:41
Co-authored-by: Kimberly Meechan <24316371+K-Meech@users.noreply.github.com>
Co-authored-by: ruaridhg <32329546+ruaridhg@users.noreply.github.com>
Co-authored-by: Kimberly Meechan <24316371+K-Meech@users.noreply.github.com>
@dstansby
Copy link
Copy Markdown
Contributor Author

Thanks all for comments/contributions to this!

@dstansby dstansby merged commit 70f1fe3 into main Oct 14, 2025
2 checks passed
@dstansby dstansby deleted the final-report branch October 28, 2025 19:19
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.

Add different data types to writeup Add chunks section to writeup

3 participants