github-starred/cs249r_book
2
0
Fork 0
mirror of https://github.com/harvard-edge/cs249r_book.git synced 2026-07-16 06:07:17 -05:00
Code Issues 276 Packages Projects Releases 26 Wiki Activity

[GH-ISSUE #1893] [Bug] Jupyternotebook: Tensor.__init__() Does Not Accept requires_grad Used by Autograd Tests #35283

New Issue
Closed
opened 2026-07-15 22:41:34 -05:00 by GiteaMirror · 2 comments
GiteaMirror commented 2026-07-15 22:41:34 -05:00
Owner
Copy Link

Originally created by @Chufeng-Jiang on GitHub (Jun 21, 2026).
Original GitHub issue: https://github.com/harvard-edge/cs249r_book/issues/1893

Area

TinyTorch

Location

Module 06 and forward in the jupyternotebook

Description

The autograd broadcasting tests construct tensors using the following syntax:

x = Tensor(data, requires_grad=True)

However, the currently exported Tensor class only defines:

Tensor(data)

As a result, the tests in the Jupyternotebook fail before reaching the broadcasting-gradient implementation.

TypeError Traceback (most recent call last) Cell In[43], line 111 108 print("✅ Broadcasting gradient tests pass!") 110 if __name__ == "__main__": --> 111 test_unit_broadcast_gradients() Cell In[43], line 7, in test_unit_broadcast_gradients() 3 print("🔬 Unit Test: Broadcasting in Gradients...") 5 # Scenario 1: Bias-like broadcasting (most common case) 6 # Shape: (batch, features) + (features,) → (batch, features) ----> 7 x = Tensor(rng.standard_normal((4, 3)), requires_grad=True) 8 bias = Tensor(np.ones(3), requires_grad=True) 10 add_func = AddBackward(x, bias) TypeError: Tensor.__init__() got an unexpected keyword argument 'requires_grad'
Image

However, it doesn't affect the unit test when call "tito module complete 06", everything passed successfully.

Image

Expected Behavior

I think all related - codes in the notebook cell needs to revised to, making the requires_grad separately :

x = Tensor(rng.standard_normal((4, 3)))
x.requires_grad = True
bias = Tensor(np.ones(3))
bias.requires_grad = True

The same change may need to be applied to the other occurrences of Tensor(..., requires_grad=True) in the cell.

I would be happy to submit a pull request with the modifications in the cell. However, before doing so, I wanted to confirm whether this is the intended fix or whether the Tensor constructor is instead supposed to be updated to support the requires_grad argument.

Environment (TinyTorch bugs only)

No response

Originally created by @Chufeng-Jiang on GitHub (Jun 21, 2026). Original GitHub issue: https://github.com/harvard-edge/cs249r_book/issues/1893 ### Area TinyTorch ### Location Module 06 and forward in the jupyternotebook ### Description The autograd broadcasting tests construct tensors using the following syntax: x = Tensor(data, requires_grad=True) However, the currently exported Tensor class only defines: Tensor(data) As a result, the tests in the Jupyternotebook fail before reaching the broadcasting-gradient implementation. ``` TypeError Traceback (most recent call last) Cell In[43], line 111 108 print("✅ Broadcasting gradient tests pass!") 110 if __name__ == "__main__": --> 111 test_unit_broadcast_gradients() Cell In[43], line 7, in test_unit_broadcast_gradients() 3 print("🔬 Unit Test: Broadcasting in Gradients...") 5 # Scenario 1: Bias-like broadcasting (most common case) 6 # Shape: (batch, features) + (features,) → (batch, features) ----> 7 x = Tensor(rng.standard_normal((4, 3)), requires_grad=True) 8 bias = Tensor(np.ones(3), requires_grad=True) 10 add_func = AddBackward(x, bias) TypeError: Tensor.__init__() got an unexpected keyword argument 'requires_grad' ``` <img width="682" height="329" alt="Image" src="https://github.com/user-attachments/assets/5c800466-12f1-491d-9d79-03cef819b0de" /> However, it doesn't affect the unit test when call "tito module complete 06", everything passed successfully. <img width="740" height="690" alt="Image" src="https://github.com/user-attachments/assets/181e29d9-2e99-4872-85b6-92011ca422eb" /> ### Expected Behavior I think all related - codes in the notebook cell needs to revised to, making the requires_grad separately : ``` x = Tensor(rng.standard_normal((4, 3))) x.requires_grad = True bias = Tensor(np.ones(3)) bias.requires_grad = True ``` The same change may need to be applied to the other occurrences of Tensor(..., requires_grad=True) in the cell. I would be happy to submit a pull request with the modifications in the cell. However, before doing so, I wanted to confirm whether this is the intended fix or whether the Tensor constructor is instead supposed to be updated to support the requires_grad argument. ### Environment (TinyTorch bugs only) _No response_
GiteaMirror added the area: tinytorchtype: bug labels 2026-07-15 22:41:34 -05:00
GiteaMirror commented 2026-07-15 22:41:37 -05:00
Author
Owner
Copy Link

@profvjreddi commented on GitHub (Jul 2, 2026):

Thanks @Chufeng-Jiang, and good catch. Your proposed fix is the right one. The Tensor constructor is intentionally minimal in module 01; enable_autograd() later monkey-patches __init__ to accept requires_grad. Since test_unit_broadcast_gradients() runs before enable_autograd() (like its sibling test_unit_function_classes(), which already sets the attribute manually with a note), the test should set requires_grad as an attribute rather than pass it to the constructor. We do not want to change the constructor. I've applied that fix on our end so it lands cleanly across the module. Thanks for the clear report and repro.

@all-contributors please add @Chufeng-Jiang for 🪲 Bug in TinyTorch

<!-- gh-comment-id:4867974808 --> @profvjreddi commented on GitHub (Jul 2, 2026): Thanks @Chufeng-Jiang, and good catch. Your proposed fix is the right one. The `Tensor` constructor is intentionally minimal in module 01; `enable_autograd()` later monkey-patches `__init__` to accept `requires_grad`. Since `test_unit_broadcast_gradients()` runs before `enable_autograd()` (like its sibling `test_unit_function_classes()`, which already sets the attribute manually with a note), the test should set `requires_grad` as an attribute rather than pass it to the constructor. We do not want to change the constructor. I've applied that fix on our end so it lands cleanly across the module. Thanks for the clear report and repro. @all-contributors please add @Chufeng-Jiang for 🪲 Bug in TinyTorch
GiteaMirror commented 2026-07-15 22:41:38 -05:00
Author
Owner
Copy Link

@github-actions[bot] commented on GitHub (Jul 2, 2026):

I've added @Chufeng-Jiang as a contributor to tinytorch! 🎉

Recognized for: bug, code
Project(s): tinytorch (explicitly mentioned in comment)
Based on: @all-contributors please add @Chufeng-Jiang for 🪲 Bug in TinyTorch

The contributor list has been updated in:

  • tinytorch/.all-contributorsrc, tinytorch/README.md
  • Main README.md

We love recognizing our contributors! ❤️

<!-- gh-comment-id:4868007630 --> @github-actions[bot] commented on GitHub (Jul 2, 2026): I've added @Chufeng-Jiang as a contributor to **tinytorch**! :tada: **Recognized for:** bug, code **Project(s):** tinytorch (explicitly mentioned in comment) **Based on:** @all-contributors please add @Chufeng-Jiang for 🪲 Bug in TinyTorch The contributor list has been updated in: - `tinytorch/.all-contributorsrc`, `tinytorch/README.md` - Main `README.md` We love recognizing our contributors! :heart:
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area: book
area: collabs
area: kits
area: labs
area: socratiq
area: staffml
area: tinytorch
area: tools
area: website
bug
dependencies
format: epub
format: pdf
javascript
link-health
link-rot
priority-high
pull-request
Mirrored from GitHub Pull Request
 staffml
type: bug
type: citation
type: code
type: errata
type: improvement
type: new
type: question
vault-sli
No Label area: tinytorch type: bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/cs249r_book#35283
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?