_calculate_improvements() guards latency/memory/energy division against
opt_metrics[metric] <= 0 with a documented "fallback to 1.0", but
accuracy_retention = opt_metrics['accuracy'] / base_metrics['accuracy']
had no such guard on the denominator. A baseline model with 0.0 accuracy
(a broken/failing baseline -- exactly the kind of input a benchmarking
suite should handle gracefully) raises ZeroDivisionError and crashes the
whole comparison, while the sibling metrics computed two lines above
degrade gracefully.
Added the same guard pattern already used for the other three metrics.
The function's own docstring already promised "Handle division by zero
with fallback to 1.0" as a general rule -- this fix makes the accuracy
branch actually honor it.
Verified: the old code raises ZeroDivisionError for
base_metrics={'accuracy': 0.0}; the fixed code returns
accuracy_retention=1.0 instead, and the normal (non-zero) case is
unaffected (0.6/0.5 = 1.2, unchanged).
benchmark_model()'s throughput_samples_per_sec (float(1000 / avg_latency))
and generate_submission()'s speedup (float(baseline_latency /
optimized_latency)) both divide by a latency measured with time.time()
around a single tiny forward pass, with no zero-guard. time.time() has
coarse resolution on Windows (commonly ~15.6ms); for a small model, a
fast forward pass can legitimately measure 0.0 elapsed time across all
runs, making avg_latency (and therefore optimized_latency downstream)
exactly 0.0 and raising ZeroDivisionError.
An existing test (line ~1347) asserts throughput_samples_per_sec > 0, and
another (line ~1564) asserts speedup > 0, so the fix floors each
denominator at a small epsilon (1e-6) rather than returning 0.0 or None --
both of which would fail those assertions and would also misrepresent an
immeasurably-fast result as zero throughput.
Verified: reproduced ZeroDivisionError for avg_latency=0.0 and
optimized_latency=0.0 with the old expressions; confirmed the floored
versions return large-but-finite positive numbers instead, and that
normal (non-zero) latency values are unaffected.
_cached_generation_step() writes the new token's K/V into the cache via
update(), then calls cache_obj.get() and uses its result directly as
K_all/V_all for the attention computation. But KVCache.get() is designed
(and tested in test_unit_kvcache) to return only tokens already made
visible by advance() -- exactly NOT the token just written by update(),
since advance() is meant to run once per token after all layers finish.
So the new token's own K/V was never included in its own attention
computation, and on the very first generation step (cache empty, nothing
advanced yet), cache.get() returns a zero-length sequence, crashing the
matmul with "zero-size array to reduction operation maximum which has no
identity" -- cached generation was broken from the first token onward.
Fixed by concatenating the current step's own K_heads/V_heads (already
computed, no extra work needed) onto cache.get()'s history, instead of
using the cache's history alone.
Verified with a stub attention module (identity projections) against the
real KVCache: reproduced the crash with the old logic on the first
token (zero-length K/V from cache.get()), and confirmed the fixed
function produces the correct self-attention output ([1,0,0,0], the
token attending fully to itself) instead of crashing.
In the HTTP-error handler, error_body was only assigned inside the try
block (e.read().decode('utf-8')). If that read/decode itself raised
(network hiccup mid-read, or a non-UTF-8 error body from the server), the
broad except (json.JSONDecodeError, Exception) still ran and referenced
error_body unconditionally, raising UnboundLocalError -- a confusing crash
in the middle of tito module complete's auto-sync step, instead of the
clean "Upload failed" message this code is trying to produce.
Initialize error_body = "" before the try, and only print it in the
except block when it's non-empty, so a read/decode failure degrades to
"nothing to print" instead of crashing.
Verified with two scenarios: (1) the read/decode call raising before
error_body is ever set -- old code crashed with UnboundLocalError, new
code handles it silently; (2) read succeeding but json.loads failing
(the normal case this except block exists for) -- both old and new code
correctly print the error body.
_ensure_module_notebook() regenerates modules/<name>/<name>.ipynb from
src/<name>.py whenever the source file's mtime is newer -- but that exact
path is the same notebook tito module start/resume open for students to
edit. A routine `git pull` that bumps a source file's mtime, followed by
an instructor running `tito nbgrader generate --all` on a shared checkout,
silently overwrote any student notebook whose file mtime was older, with
no backup and no warning beyond a benign "Source file is newer; converting"
message.
Fixed by never writing to the student-facing notebook path once it
exists: if it needs regenerating from a newer source, stage the fresh
conversion into a private .nbgrader_staging/ cache instead, and read from
there for nbgrader's own assignment staging. First-time creation (no
notebook yet) and the already-up-to-date case are unaffected.
Verified with three scenarios against the real _ensure_module_notebook
(jupytext call mocked, real file I/O): (1) notebook missing -> created
directly at the notebook path as before; (2) notebook up to date -> no
conversion happens at all, matching prior behavior; (3) notebook exists
but source is newer -> notebook content is left untouched, fresh content
is staged separately instead.
Module 06's test_unit_broadcast_gradients ran Tensor(data, requires_grad=True)
before enable_autograd() upgrades the constructor to accept it, so the exported
notebook cell raised TypeError. Match the sibling test_unit_function_classes
convention and set requires_grad as an attribute instead; the constructor stays
minimal by design. Fixes#1893.
The PR converted the SGD markdown cell to a raw string (r""") but kept \\
in the redrawn ASCII art. In a raw string \\ is two literal backslashes, so
the loss-surface, oscillation, and momentum diagrams rendered with doubled
backslashes in the generated notebook (correct-looking in the .py source).
Normalize the art to single backslashes, matching the foundations cell.
AvgPool2d returned a tensor with no _grad_fn, so average pooling had no
gradient path (MaxPool2d and Conv2d both wire theirs). Add AvgPool2dBackward,
which distributes each output gradient equally (1/kernel_area) across its
window and accumulates overlaps, padding the gradient buffer with zeros to
match the forward pass. Wire it into AvgPool2d.forward.
From #1885, taking the AvgPool backward feature only: singular name to match
the *Backward convention, zero (not -inf) padding, and verified by numeric
gradient check. The PR diagram edits are not included.
Adds unit tests for the community submission flow: payload schema
(assemble_payload), the 200 success path, and the 401 token-refresh path,
plus an opt-in live end-to-end test that self-skips without credentials.
Corrects two assertions to match the current handler: sync_progress returns
a SyncResult (check .ok, not identity against True), and the success message
is "Sync successful!" (lowercase). The three unit tests pass locally.
From #1886, test files only; the bundled guard.js/auth.js auth change is left
for a separate, reviewable PR.
Two latent bugs in tito module complete:
- Unit tests run against the instructor src/ file, so a SyntaxError in the
student notebook slipped through to a silent broken export. Adds an additive
_check_notebook_syntax pass before export (unit tests still run) that compiles
each code cell, skipping IPython magics and shell escapes.
- Integration-test discovery looked for tests/<module>/test_progressive_integration.py,
which exists in no module; the real files are test_<module>_progressive.py, so
integration tests never ran. Resolves the correct file with a glob fallback.
Combines #1877 (syntax check, kept additive) and #1878 (integration discovery);
the unit-test phase is preserved rather than replaced.
The naive 1/(1+exp(-x)) overflows exp for large negative x and the unit
test hid the resulting RuntimeWarning by suppressing the whole category.
Use the piecewise-stable form (each branch keeps its exponent <= 0) under
np.errstate, since np.where evaluates both branches and the discarded one
can still overflow harmlessly. Drop the test-wide warning suppressor.
Based on #1870; uses errstate so the piecewise form is actually silent.
quantize_int8 set scale=1.0 for a constant tensor and clamped zero_point
to the INT8 range, so for any constant |c| > 127 the zero_point saturated
and dequantization silently returned a wrong value (c=500 recovered as 128).
For |c| > 127 use zero_point = +-1 and scale = |c| so
(0 - zero_point) * scale = c holds exactly. Adds a test exercising that branch.
Reapplies the fix from #1874 on dev with a corrected scale (the PR used
scale = -c/(-zero_point), which flips the sign and recovers -c).
The 3-D mask reshape used seq_len for both the query and key dimensions,
raising ValueError for cross-attention (seq_q != seq_k) and only working
when seq_q == seq_k. Read both dimensions and reshape to
(batch, 1, seq_q, seq_k) so the mask broadcasts across heads correctly.
Extracted from #1875 (branch was based on main; reapplied on dev).
The module declares rng = np.random.default_rng(7) at the top so that all
stochastic operations are reproducible for a given seed. Dropout ignored this
and called np.random.random(shape), which draws from the global unseeded state.
Masks were different on every run even when the module seed was set, making
Dropout unit tests non-reproducible and causing hard-to-debug numerical
differences between training runs.
Changed to rng.random(shape) so Dropout participates in the same reproducible
stream as every other random operation in the module.
All three classes are declared with no base class, so super().__init__()
resolves to object.__init__(), a silent no-op. The calls made these look like
they inherited from Layer (as Linear does in 03_layers) when they do not.
Removing them makes the code honest: these are standalone modules that
implement parameters() directly. The right long-term fix is to export Layer
from tinytorch.core and wire these classes into that hierarchy, but that
requires a separate refactor across multiple modules.
The old test called transpose(0, 2) on a (2, 2, 2) tensor and only checked
that the output shape was (2, 2, 2). Because all three dims are equal, any axis
swap produces the same shape, so the test would have passed even if transpose
was completely broken and returned the input unchanged.
Switched to a (2, 3, 4) tensor where all three dimension sizes are distinct.
Now transpose(0, 2) must produce (4, 3, 2), a shape only achievable by swapping
exactly axis 0 and axis 2. Also added a data-value check verifying that element
[i,j,k] in the original appears at [k,j,i] in the transposed result, catching
implementations that swap the wrong pair of axes.
When the shape contains -1, the inferred size was computed with integer
division (self.size // known_size). If the total size is not evenly divisible
by the known dimensions, floor division silently produces a truncated value
and the element-count check catches it only after the wrong number has already
been placed in new_shape, producing a confusing error.
For example, Tensor([1..7]).reshape(-1, 3) should fail immediately because 7
is not divisible by 3, but the old code inferred 2 (7//3), then raised a
generic element-count mismatch that did not explain the actual problem.
Added a divisibility check before the division so the error fires at the right
place with a clear message, matching PyTorch behavior.
tracked_mul() wraps a scalar other into other_tensor = Tensor(other) and
calls _ensure_grad_attrs() on it correctly, but then passed the original raw
scalar to MulBackward instead of other_tensor. MulBackward.apply() pulls both
operands out of saved_tensors and calls .data on them, so any scalar multiply
in a tracked computation graph crashed on the backward pass with AttributeError
because int and float have no .data attribute.
Fixed by passing other_tensor to MulBackward so both operands in saved_tensors
are always Tensor instances. The forward computation still uses the original
scalar unchanged.
When bias=None, only x and weight are saved in saved_tensors (2 entries),
but apply() always returned a 3-tuple of grad_input, grad_weight, grad_bias.
The backward walk zips saved_tensors with the return tuple so the extra
grad_bias (None) was silently dropped by zip truncation. This made bias-free
Conv2d appear to train correctly while the tuple length mismatch left the
code one refactor away from corrupting gradients silently.
Fixed by returning only (grad_input, grad_weight) when bias is None so
the return tuple always has the same length as saved_tensors.
nbdev walks up from cwd looking for a pyproject.toml that contains a
[tool.nbdev] section. If it does not find one it falls through to a legacy
check: if settings.ini exists anywhere in the parent chain it raises
"Found old settings.ini. Migrate to pyproject.toml" as a hard error.
pyproject.toml existed but had no [tool.nbdev] section, so every call to
nb_export() hit that fallback and failed. This broke Stage 1 (Inline Build)
on every CI run and cascaded to block all downstream stages.
The fix is the proper migration: add [tool.nbdev] with the canonical nbdev
fields matching what is already in settings.ini. nbdev now finds the section
on its first walk and never reaches the settings.ini check. settings.ini is
kept intact because the release workflow still writes the version number into
it on every release.
Also updates two stale settings.ini references in export.py (directory
detection heuristic and error message) and removes settings.ini from
UpdateCommand.UPDATE_FILES since pyproject.toml is now the authoritative
nbdev config file.
The dashboard did not reflect CLI progress for users across Windows and
Ubuntu. Three independent client defects, now fixed:
1. Automatic sync was silently skipped on any non-TTY shell. _trigger_submission
gated on `not sys.stdin.isatty()`, which is False on Windows Git Bash/MinTTY
and many IDE terminals even when interactive, so `tito module complete`
updated local progress.json but never uploaded, with no message. Decouple
"should we sync" (logged-in and not CI) from "should we prompt" (needs a
TTY): non-interactive real users now sync without a prompt instead of being
skipped.
2. A 2xx response with synced_modules null/0 was reported as success using the
local count, hiding backend failures. sync_progress now returns a SyncResult
and reports an honest accepted-but-unconfirmed warning when the server does
not confirm persistence.
3. No standalone resync path and login did not sync, so modules completed before
logging in never reached the dashboard. Add `tito community sync` and an
offer to sync existing progress after login.
The sync trigger decision now lives in one shared helper
(auto_sync_after_completion) used by the module, milestone, and login paths,
plus a small core/runtime.py that owns is_ci()/is_interactive(). Adds
tests/cli/test_progress_sync.py (15 tests) covering the non-TTY regression, the
CI/not-logged-in branches, honest 2xx interpretation, and the new command.
Add What You Need to Start, For Instructors, and Make It Better sections; broaden The Bigger Picture with the build-deploy-test arc and the breadboard/FPGA/TinyOS tradition; route resource links through mlsysbook.ai and the repo via mlsysbook.ai/git; drop em-dashes and contractions for house style.
The subtitle script injected the 'Machine Learning Systems' link as a
sibling of the logo <img>, which sits inside Quarto's
<a class="sidebar-logo-link"> wrapper. That nested our anchor inside the
logo anchor (invalid HTML), so clicking it fired the outer logo link
(./index.html) instead of navigating to the book. Insert the subtitle
after the logo anchor as a child of .sidebar-header so the link works.
The version chip keys off subtitle.parentNode and is fixed by the same move.
