Integrate quant: production-qualified training, scalable diagnostics and inference tooling - #2
Conversation
Verify baseline and dataset provenance, calibrate on training samples, and evaluate reloaded integer artifacts on held-out synthetic, MNIST, and Blair data. Preserve per-level predictions and coverage for review.
Add a CUDA probe with INT8 weights, saved activations, and forward/backward GEMMs plus numerical and storage checks. Record workload-dependent speed and memory results, loader measurements, and the remaining optimizer, checkpoint, and convergence requirements.
…dates Add isolated SGD and AdamW parameter dispatch, stochastic checkpoint continuation checks, and ordinary Linear coverage. Form backward scales in float32 to avoid FP16 underflow. Reject zero-gradient compiled probes and record corrected eager learning plus negative performance results; production QT integration remains ongoing.
Include implementation source and tensor metadata in the experimental parameter cache key. Add compiled-gradient regression coverage and replace the stale-cache diagnosis with corrected learning and performance measurements.
Replace unbounded queues and the daemon writer with bounded ordered futures and direct validated writes. Decode each sample once, propagate failures with reader cleanup, and cap automatic cache threads at 16 with a synchronous override. Add CPU/CUDA regressions and a reproducible comparison showing similar PNG throughput.
Move integer kernels into the optional runtime backend, prepare eligible Linear weights before optimizer creation, and report floating-point coverage. Preserve selected weight ties and restore quantized parameter types from checkpoint recipes. Cover AMP, masked heads, inference aliases, regularization, diagnostics, MuonAuxAdamW and training resume; validate minimal and CUDA installed wheels.
Keep AMP update gating and Muon counters outside compiled child updates. Initialize lazy state on the first real call, use tensor learning rates for scheduler updates, and save scalar rates for eager resume. Preserve Muon's explicit BF16 casts. Expose --compile-optimizer independently of model compilation and record it in benchmark reports. Validate with static checks, the full suite (261 passed, 76 skipped, 1 expected EMA failure), and focused CUDA checks (81 passed). INT8 storage-kernel experiments remain separate.
Record dense MNIST quality, timing and whole-run versus later-phase allocation. Trace the 256 MiB Triton tuning buffer and document fake-tensor and compiler-cache failures that prevent shipping the experimental storage kernel.
Use explicit Triton row updates with a fake-tensor-safe custom operator, preserving integer storage, stochastic rounding, and tensor version invalidation. Tune TorchAO matrix kernels through a local CUDA-graph tuner without its 256 MiB cache-flush buffer. The paired dense MNIST run reduces whole-training peak allocation from 236.30 to 154.00 MiB. Record that runtime and quality parity remain unproven. Keep the many-group outer optimizer fallback as a strict expected-failure regression. Validation: static checks; full suite 261 passed, 85 skipped, 1 expected EMA failure; CUDA 44 passed and 1 expected compiler limitation; minimal installed-wheel smoke passed.
Handle the out-of-place primitive that Dynamo emits for tensor learning rates. This prevents optimizer-loop graph breaks and per-weight cache exhaustion without changing global compiler limits. Restore the many-group regression and extend it to AdamW. Verify one-rounding update bounds, optimizer state, and unbiased sub-code updates with advancing CUDA RNG. Document the paired MNIST result: memory remains lower, but speed and quality parity are not established. Validation: static checks; full suite 264 passed, 90 skipped, 1 expected EMA failure; CUDA regression set 108 passed plus focused arithmetic and RNG checks passed.
Tag repository batch indices so LazyDataset can defer redundant per-sample views. Preserve ordinary external batched fetches, default collation, dataset identity, spawned workers, shuffle RNG, and CUDA prefetch. The small-image cache probe improves from 0.52M to 2.22M samples/s. Record larger-batch MNIST runs showing QT memory and warm-runtime gains, while retaining cold-start and accuracy-variation limits. Validation: static checks; full suite 274 passed, 90 skipped, 1 expected EMA failure; CUDA loader and QT suite 108 passed.
Include matrix and update kernel sources in the training tensor's compiler fingerprint, preventing old opaque graphs from hiding operator changes during validation. Retain the opaque matrix path after the graph-visible experiment showed excessive startup cost and unstable real-data accuracy despite passing numerical checks. Validation: static checks; full suite 274 passed, 90 skipped, 1 expected EMA failure; retained CUDA backend 52 passed.
Record explicit modes in benchmark reports and preserve existing defaults. Keep normalized weights beside their Linear consumer so embedding publication does not split the INT8 autograd boundary. Validate portable checkpoints and eager resume across normalized heads and optimizer modes; compare embedding-loss gradients in FP32. Full CPU suite and focused CUDA regressions pass.
Add a separate three-seed MNIST profile with matched float and INT8 compilation modes, preserving ordinary compilation baselines. Retain failures, arguments, summaries and artifacts. Record 31% lower INT8 peak allocation and shorter whole training calls, while explicitly retaining the mixed later-epoch speed result. Validate harness failure paths and both synthetic oracles.
There was a problem hiding this comment.
This PR should bump version
There was a problem hiding this comment.
Bumped the package to 0.2.0 in pyproject.toml and the lockfile (3cb478f). The isolated installed-wheel smoke passed.
There was a problem hiding this comment.
Adding many specific cases is not necessarily wrong, but it is definitely an anti-pattern
There was a problem hiding this comment.
Consolidated the individual UCloud JSON exceptions into one directory-scoped pattern and removed a redundant temporary-directory pattern (3cb478f). Generated artifacts remain ignored.
There was a problem hiding this comment.
Overall, I am quite suspicious about rolling our own test collections, though it does just wrap pytest anyways.
There was a problem hiding this comment.
There is no custom pytest collector: the shell wrapper selects the existing environment, sets CPU/headless defaults and forwards arguments to python -m pytest. Clarified this and documented direct pytest usage in tests/README.md (3cb478f).
There was a problem hiding this comment.
My impression is also that this PR suggest many new tests which are a bit overly pedantic, wasting both compute and time for minimal gain.
There was a problem hiding this comment.
Added --durations=15 to CI so expensive cases are visible, and documented the preference for observable-contract/regression coverage over implementation-detail assertions. I have not pruned tests indiscriminately: the affected 66-case suite took 41.50s locally (65 passed, one expected EMA failure), while unrelated benchmark-model tests made the broad local run substantially slower. Further consolidation remains a review target, rather than claiming this concern is fully resolved.
| best_epoch = epoch | ||
| if output_dir is not None: | ||
| raw_model = model.module if hasattr(model, "module") else model | ||
| raw_model = model |
There was a problem hiding this comment.
Bit of a weird choice to use a while statement for this purpose, I do see that this won't cause an infinite loop, but there isn't any guarantee that this won't change in the future.
There was a problem hiding this comment.
Kept traversal because callers can supply nested compilation/distribution wrappers, but added identity-based cycle detection with a clear ValueError (3cb478f). A cyclic wrapper chain can no longer hang checkpoint saving. Compiled checkpoint/resume and CPU DDP tests passed.
| log.info(f"Training restarted from checkpoint(s): {checkpoint}") | ||
|
|
||
| if compile_optimizer: | ||
| from mini_trainer.training.compilation import compile_optimizer as prepare_compiled_optimizer |
There was a problem hiding this comment.
Why are we doing lazy imports?
There was a problem hiding this comment.
This lazy import was unnecessary: the compilation module is already imported above. Moved the optimizer compiler alias into the normal module-level imports (3cb478f).
| ) | ||
| validate_type(nn_model, torch.nn.Module) | ||
| if quantized_training or getattr(nn_model, "_quantized_training_recipe", None): | ||
| from mini_trainer.modeling.quantized_training import prepare_quantized_training |
There was a problem hiding this comment.
Why are we doing lazy imports?
There was a problem hiding this comment.
Moved this import to module scope too (3cb478f). The public quantized-training facade is safe to import; only its optional native backend remains lazy. Minimal installed-wheel imports and training passed without quantization extras.
| report.update(source=os.path.abspath(class_list), sha256=hashlib.sha256(contents).hexdigest()) | ||
| with open(os.path.join(output_dir, "class_filter.json"), "w", encoding="utf-8") as handle: | ||
| json.dump(report, handle, indent=2) | ||
| print( |
There was a problem hiding this comment.
We should NOT use bare print statements like this.
There was a problem hiding this comment.
Replaced the bare print with the repository logger (3cb478f). Class-filter reporting remains in class_filter.json as well. The class-list inference integration tests passed.
| return cached_result | ||
|
|
||
| with urlopen(req) as resp: | ||
| with urlopen(req, timeout=10) as resp: |
There was a problem hiding this comment.
Why are we adding a fixed timeout here? Seems like the responsibility for the issue this is supposed to solve is somewhere else, where?
There was a problem hiding this comment.
Removed the hardcoded 10-second policy (3cb478f). The helper now accepts an optional keyword-only socket timeout and otherwise uses urllib's process-wide default. A socket timeout cannot enforce a whole taxonomy lookup or job deadline; that policy belongs to the calling orchestration layer. The distinction is documented, and default/explicit forwarding was checked without network requests.
asgersvenning
left a comment
There was a problem hiding this comment.
This is generally a PR with many useful features, but it does require a bit of cleanup before it can be pulled.
|
Review follow-up pushed through focused branches and explicit merges (head 400377d). Replied to each inline comment; threads remain open for your review.
Current statistics, generated locally from the PR diff using the same categorization: 176 included files · +18,959 / −1,172 lines
Root README.md is the initial explicit Markdown exception. Other feature documentation can be added to the workflow's featureMarkdown set. Counts represent the diff, not commit totals or a measure of effort. |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
Purpose
Integrate the
quantbranch intomasterafter qualification and a completed 30-epoch production run on four B200 GPUs. The production configuration used floating-point FP16 AMP with model compilation, figures and W&B enabled. Fully quantized training with a demonstrated throughput and memory advantage remains an unfinished goal.Changes
--class-listcandidate filtering without dropping images or ground truth.This is a long-lived integration branch: 156 commits and 212 changed files at ad1fd10. Preserve the existing commits with a merge commit, rather than squashing or rewriting the development history.
Validation evidence
User-reported production run:
Local validation:
reduce_range=Falseretains the older full-range recipe for validated VNNI targets.Remaining limits
Please review and let required CI finish before merging. Auto-merge is not enabled.