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

[docs] Update pytorch article figure and caption #1246

Merged
merged 2 commits into from
Jul 20, 2024

Conversation

ryan-williams
Copy link
Contributor

Correct "default" marker annotation at https://chanzuckerberg.github.io/cellxgene-census/articles/2024/20240709-pytorch.html#increased-training-speed:

gif: before / after

20240709-pytorch-fig-benchmark

Also made a few tweaks to the caption, to reflect what I think are the most important take-aways:

  • ≈2.5x speed improvement vs. shuffling with scipy.csr method (red circles > red squares)
  • marginal speed improvement, and decreased memory usage, compared to {sparse, unshuffled} benchmarks (i.e. before #1188 and #1169/#1224; basically red-orange "default" circles vs. corresponding blue squares)

Summarizing another way:

  • #1188 introduced shuffling, but ≈halved speed (blue squares → red squares)
  • #1224 added 2.5x speedup (red squares → red circles)
  • Net result: we added shuffling (⇒ better model training), and marginally increased speed

If a user is willing to use 2x the memory, they can go for the larger red-orange circles (2048 chunks @ 128 rows/chunk), and another nearly ≈2x speedup, but that's not explicitly called out, which is probably fine?

@pablo-gar
Copy link
Contributor

Thanks @ryan-williams Would you mind adding a line right under

*Published:* *July 11th, 2024*

To make it this:

*Published:* *July 11th, 2024*

*Updated:* *July 19th, 2024*. Figure 3 has been improved for readability.

@pablo-gar pablo-gar self-requested a review July 19, 2024 16:01
@pablo-gar pablo-gar changed the title update 20240709-pytorch-fig-benchmark.png, caption [docs] Update pytorch article figure and caption Jul 20, 2024
@pablo-gar pablo-gar merged commit 313e201 into chanzuckerberg:main Jul 20, 2024
9 of 11 checks passed
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.

2 participants