In the function measure_sparsity() of Module 16, we set total_params=0.0 if there are no parameters present in the model. In the following division, wouldn't that raise a ValueError?
Should we have set total_params=eps in this case to prevent this, where eps can be 1e-9?
Originally created by @ngbolin on GitHub (Feb 11, 2026).
Original GitHub issue: https://github.com/harvard-edge/cs249r_book/issues/1168
Hi,
In the function ```measure_sparsity()``` of Module 16, we set total_params=0.0 if there are no parameters present in the model. In the following division, wouldn't that raise a ValueError?
Should we have set total_params=eps in this case to prevent this, where eps can be 1e-9?
<img width="681" height="528" alt="Image" src="https://github.com/user-attachments/assets/1e978dd8-7d3f-4211-8c4d-ca714d8c37d2" />
In addition, for the example below, 0.1 seems to be the 44th percentile instead of the 70th percentile. Hence, when setting the threshold at 0.1, 44% of the values (or 8/18 of all values) are set to 0.
<!-- gh-comment-id:3882854864 -->
@ngbolin commented on GitHub (Feb 11, 2026):
In addition, for the example below, 0.1 seems to be the 44th percentile instead of the 70th percentile. Hence, when setting the threshold at 0.1, 44% of the values (or 8/18 of all values) are set to 0.
<img width="695" height="502" alt="Image" src="https://github.com/user-attachments/assets/90ae1065-a464-40cd-9114-f95e4f2ec224" />
In the function demo_compression_with_profiler, the sparsity increased from 0 to 70%, not 7000%. I think this was a result of the measure_sparsity function, which already converted the fraction to a percentage.
<!-- gh-comment-id:3882961445 -->
@ngbolin commented on GitHub (Feb 11, 2026):
In the function ```demo_compression_with_profiler```, the sparsity increased from 0 to 70%, not 7000%. I think this was a result of the ```measure_sparsity``` function, which already converted the fraction to a percentage.
<img width="625" height="526" alt="Image" src="https://github.com/user-attachments/assets/0d4409d6-0731-4ff5-a9ec-97200aeb0b69" />
<img width="342" height="430" alt="Image" src="https://github.com/user-attachments/assets/202395be-5f4d-40e2-bc70-9ab61d3c5960" />
Hi @ngbolin — great catches, all three! Here's what we found and fixed:
Bug 1: measure_sparsity() divide-by-zero
There's actually already a guard for this:
iftotal_params==0:return0.0
It returns early before the division, so no ZeroDivisionError is raised. But you're right that the screenshot makes it look like the division would happen unconditionally — appreciate flagging it.
Bug 2: Incorrect percentile in the pruning diagram
You're exactly right. With 18 values, a threshold of 0.1 zeroes out 8/18 ≈ 44% — not the 70% claimed. We rewrote the ASCII art example to use 20 values with a clean 50th percentile threshold (0.4), which zeroes out exactly 10/20 values. The math now checks out.
Bug 3: 7000% sparsity display
Spot on — measure_sparsity() already returns a percentage (0–100), so the extra * 100 in demo_compression_with_profiler() was double-multiplying. We fixed all 6 occurrences in that function:
Removed * 100 from display strings (e.g., {sparsity_after:.1f}% instead of {sparsity_after*100:.1f}%)
Changed fraction math to divide by 100 (e.g., 1 - sparsity / 100 instead of 1 - sparsity)
Fixed sparsity_gain to be a simple difference of percentages instead of re-multiplying
All fixes are in commit 0630674a7. Thanks for the thorough report — this kind of attention to detail really helps improve the modules! 🙏
<!-- gh-comment-id:3887865553 -->
@profvjreddi commented on GitHub (Feb 11, 2026):
Hi @ngbolin — great catches, all three! Here's what we found and fixed:
### Bug 1: `measure_sparsity()` divide-by-zero
There's actually already a guard for this:
```python
if total_params == 0:
return 0.0
```
It returns early before the division, so no `ZeroDivisionError` is raised. But you're right that the screenshot makes it look like the division would happen unconditionally — appreciate flagging it.
### Bug 2: Incorrect percentile in the pruning diagram
You're exactly right. With 18 values, a threshold of 0.1 zeroes out 8/18 ≈ 44% — not the 70% claimed. We rewrote the ASCII art example to use 20 values with a clean 50th percentile threshold (0.4), which zeroes out exactly 10/20 values. The math now checks out.
### Bug 3: 7000% sparsity display
Spot on — `measure_sparsity()` already returns a percentage (0–100), so the extra `* 100` in `demo_compression_with_profiler()` was double-multiplying. We fixed all 6 occurrences in that function:
- Removed `* 100` from display strings (e.g., `{sparsity_after:.1f}%` instead of `{sparsity_after*100:.1f}%`)
- Changed fraction math to divide by 100 (e.g., `1 - sparsity / 100` instead of `1 - sparsity`)
- Fixed `sparsity_gain` to be a simple difference of percentages instead of re-multiplying
All fixes are in commit 0630674a7. Thanks for the thorough report — this kind of attention to detail really helps improve the modules! 🙏
@all-contributors please add @ngbolin as a contributor for ✍️ Doc,Bug in tinytorch
<!-- gh-comment-id:3887888541 -->
@profvjreddi commented on GitHub (Feb 12, 2026):
@all-contributors please add @ngbolin as a contributor for ✍️ Doc,Bug in tinytorch
@github-actions[bot] commented on GitHub (Feb 12, 2026):
I've added @ngbolin as a contributor to tinytorch! 🎉
Recognized for: doc Project: tinytorch (explicitly mentioned in comment) Based on: @all-contributors please add @ngbolin as a contributor for ✍️ Doc,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:3887897032 -->
@github-actions[bot] commented on GitHub (Feb 12, 2026):
I've added @ngbolin as a contributor to **tinytorch**! :tada:
**Recognized for:** doc
**Project:** tinytorch (explicitly mentioned in comment)
**Based on:** @all-contributors please add @ngbolin as a contributor for ✍️ Doc,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:
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Originally created by @ngbolin on GitHub (Feb 11, 2026).
Original GitHub issue: https://github.com/harvard-edge/cs249r_book/issues/1168
Hi,
In the function
measure_sparsity()of Module 16, we set total_params=0.0 if there are no parameters present in the model. In the following division, wouldn't that raise a ValueError?Should we have set total_params=eps in this case to prevent this, where eps can be 1e-9?
@ngbolin commented on GitHub (Feb 11, 2026):
In addition, for the example below, 0.1 seems to be the 44th percentile instead of the 70th percentile. Hence, when setting the threshold at 0.1, 44% of the values (or 8/18 of all values) are set to 0.
@ngbolin commented on GitHub (Feb 11, 2026):
In the function
demo_compression_with_profiler, the sparsity increased from 0 to 70%, not 7000%. I think this was a result of themeasure_sparsityfunction, which already converted the fraction to a percentage.@profvjreddi commented on GitHub (Feb 11, 2026):
Hi @ngbolin — great catches, all three! Here's what we found and fixed:
Bug 1:
measure_sparsity()divide-by-zeroThere's actually already a guard for this:
It returns early before the division, so no
ZeroDivisionErroris raised. But you're right that the screenshot makes it look like the division would happen unconditionally — appreciate flagging it.Bug 2: Incorrect percentile in the pruning diagram
You're exactly right. With 18 values, a threshold of 0.1 zeroes out 8/18 ≈ 44% — not the 70% claimed. We rewrote the ASCII art example to use 20 values with a clean 50th percentile threshold (0.4), which zeroes out exactly 10/20 values. The math now checks out.
Bug 3: 7000% sparsity display
Spot on —
measure_sparsity()already returns a percentage (0–100), so the extra* 100indemo_compression_with_profiler()was double-multiplying. We fixed all 6 occurrences in that function:* 100from display strings (e.g.,{sparsity_after:.1f}%instead of{sparsity_after*100:.1f}%)1 - sparsity / 100instead of1 - sparsity)sparsity_gainto be a simple difference of percentages instead of re-multiplyingAll fixes are in commit
0630674a7. Thanks for the thorough report — this kind of attention to detail really helps improve the modules! 🙏@profvjreddi commented on GitHub (Feb 12, 2026):
@all-contributors please add @ngbolin as a contributor for ✍️ Doc,Bug in tinytorch
@github-actions[bot] commented on GitHub (Feb 12, 2026):
I've added @ngbolin as a contributor to tinytorch! 🎉
Recognized for: doc
Project: tinytorch (explicitly mentioned in comment)
Based on: @all-contributors please add @ngbolin as a contributor for ✍️ Doc,Bug in tinytorch
The contributor list has been updated in:
tinytorch/.all-contributorsrctinytorch/README.mdREADME.mdWe love recognizing our contributors! ❤️