Skip to content

tinytorch module 09 diagrams and spacing - #1917

Merged
profvjreddi merged 3 commits into
harvard-edge:devfrom
vedant-a-joshi:tinytorch-module-09-fixes
Aug 10, 2026
Merged

tinytorch module 09 diagrams and spacing#1917
profvjreddi merged 3 commits into
harvard-edge:devfrom
vedant-a-joshi:tinytorch-module-09-fixes

Conversation

@vedant-a-joshi

@vedant-a-joshi vedant-a-joshi commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

TinyTorch Module 09

Fixed misaligned tables. Fixed calculations for some of the comparisons. Fixed formatting of cells with extra new lines at the top as well as HTML tags. Other minor fixes to make sure everything looks neat.

Tested by running pytest tests/, and the tests relevant to module 09 passed. Ran tito module test 09 and appropriate tests were passed. Note, there were some warnings about assert vs. return in the tests. Manually verified that formatting looks good by running it as a jupyter notebook and checking manually.

Revision of PR #1885


By submitting this PR, you agree to release your contribution under the project's license.

@github-actions github-actions Bot added area: tinytorch TinyTorch framework core type: bug bug in rendering labels Jun 30, 2026
@profvjreddi

Copy link
Copy Markdown
Contributor

Thanks for the focused diagram pass, @vedant-a-joshi — the alignment and neatness are genuinely welcome. I'm holding it for one more round rather than merging, because a few things would regress module 09 as-is:

  • Functional regression: it drops the result._grad_fn = AvgPool2dBackward(...) wiring in AvgPool2d.forward that already landed from tinytorch module 09 fixes and feature #1885, which re-breaks average-pool autograd and orphans AvgPool2dBackward as dead code. Looks like an accidental self-revert from a stale branch — a re-sync on current dev should clear it.
  • Number still off (the tinytorch module 09 fixes and feature #1885 one): the worked-example Final Output shows 7 at row-2 col-0 where the position just above computes 5 (should read 5 7 9).
  • Param count: the SimpleCNN Linear count should stay 20,490 — TinyTorch's Linear defaults to bias=True (its own docstring examples include bias), so 20,480 is the no-bias value.

Two that are genuinely your call, not bugs — just confirm they were intentional: switching the intro example to a depthwise per-color conv (28×28×3 / 75 params), and deleting the CNN-vs-Dense '62× fewer params' comparison block.

Sorry this took so long to get back to you — thanks for your patience, and happy to re-review as soon as it's re-synced.

@vedant-a-joshi
vedant-a-joshi force-pushed the tinytorch-module-09-fixes branch 2 times, most recently from 2760e0c to 93296c1 Compare July 19, 2026 00:18
@vedant-a-joshi
vedant-a-joshi force-pushed the tinytorch-module-09-fixes branch from 93296c1 to 04731f2 Compare July 19, 2026 00:24
@vedant-a-joshi

vedant-a-joshi commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Hello @profvjreddi,

I have made the updates per your comments.

You were correct about the AvgPool2d.forward error (it was stale). I also fixed the typos in the parameter count due to the bias as well as the worked-out example.

The intro example that you mentioned was a typo, I meant to change it 30x30x16. I deleted the 62x fewer params comparison block since a similar comparison is being made above it with the same model structure when showcasing parameter efficiency (showed 120x).

Please let me know what you think. I appreciate it.

@Shashank-Tripathi-07

Copy link
Copy Markdown
Collaborator

Hi @vedant-a-joshi, I see that your PR has been in waiting for some time and I've tagged prof. vijay to take a look into it and merge it or suggest changes to help you move forward with this. Happy Engineering !

@profvjreddi
profvjreddi merged commit 6f76f3e into harvard-edge:dev Aug 10, 2026
55 of 59 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Thanks @vedant-a-joshi! 🎉

I added @vedant-a-joshi to tinytorch for: code, test, doc.

The contributor tables are now handled directly by this workflow; no follow-up command is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: tinytorch TinyTorch framework core type: bug bug in rendering

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants