Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ For inference changes, real-checkpoint validation is required — synthetic-only

- [ ] `cargo fmt --all -- --check` (enforced by CI — violations block merge)
- [ ] `cargo clippy --all-targets -- -D warnings`
- [ ] `cargo test --release`
- [ ] `cargo test --profile test-fast`
- [ ] `cargo deny check`
- [ ] Validated with a real checkpoint (specify which, e.g. `mlx-community/Qwen3.5-0.8B-OptiQ-4bit`): ...

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
# for why it is nightly rather than per-PR.
#
# Quality gate still lives locally at PR time. Pre-push checklist:
# make verify # fmt + clippy(metal,accelerate, -D warnings) + test(release)
# make verify # fmt + clippy(metal,accelerate, -D warnings) + test(test-fast)
# make verify-clean # same, after `cargo clean` — use when clippy's
# # per-crate cache may be hiding a regression
#
Expand Down
300 changes: 264 additions & 36 deletions .github/workflows/nightly-verify.yml

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ jobs:
# machine in a fraction of the time.
#
# Quality gate lives locally now. Pre-push checklist:
# make verify # fmt + clippy(metal,accelerate, -D warnings) + test(release)
# make verify # fmt + clippy(metal,accelerate, -D warnings) + test(test-fast)
# make verify-clean # same, after `cargo clean` — use when clippy's
# # per-crate cache may be hiding a regression
#
Expand Down
10 changes: 5 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,22 @@ Thank you for your interest in contributing to mlxcel! This document covers the
```bash
# macOS (Apple Silicon)
cargo build --release --features metal,accelerate
cargo test --release
cargo test --profile test-fast --features metal,accelerate

# Linux / CUDA
cargo build --release --features cuda
cargo test --release
cargo test --profile test-fast --features cuda
```
4. Run the local quality gates:
```bash
cargo fmt --all -- --check # gated at PR time; fmt violations block merge
cargo clippy --all-targets --features metal,accelerate -- -D warnings # NOT gated at PR time; yours to run
cargo test --release --features metal,accelerate # NOT gated at PR time; yours to run
cargo test --profile test-fast --features metal,accelerate # NOT gated at PR time; yours to run
cargo deny check # gated at PR time (advisories + licenses + sources)
```
PR-time CI runs only the cheap gates: `cargo fmt` and `cargo deny` in [`ci.yml`](.github/workflows/ci.yml), plus a path-filtered clippy and a `distributed::`-scoped `cargo test` in [`pipeline-parallel-ci.yml`](.github/workflows/pipeline-parallel-ci.yml) when you touch pipeline-parallel code. Clippy and the general unit suite are **not** enforced on your PR. They were moved in #21 and removed in #23 because ~30 min per run on the shared self-hosted Apple Silicon runner blocked PRs and releases for failures that `make verify` catches locally in a fraction of the time. [`nightly-verify.yml`](.github/workflows/nightly-verify.yml) runs the full `make verify` once a day on `self-hosted-macos-26-arm64` and files an issue when `main` goes red, so a broken suite surfaces within a day rather than on the next contributor's `make verify`. Treat that as a backstop, not a substitute: run the two commands above yourself before you push. CUDA verification is not gated at PR time either; that stays exclusive to `release.yml`.
PR-time CI runs only the cheap gates: `cargo fmt` and `cargo deny` in [`ci.yml`](.github/workflows/ci.yml), plus a path-filtered clippy and a `distributed::`-scoped `cargo test` in [`pipeline-parallel-ci.yml`](.github/workflows/pipeline-parallel-ci.yml) when you touch pipeline-parallel code. Clippy and the general unit suite are **not** enforced on your PR. They were moved in #21 and removed in #23 because ~30 min per run on the shared self-hosted Apple Silicon runner blocked PRs and releases for failures that `make verify` catches locally in a fraction of the time. [`nightly-verify.yml`](.github/workflows/nightly-verify.yml) runs the full `make verify` once a day on `self-hosted-macos-26-arm64` and files an issue when `main` goes red or when the run does not finish, so a broken suite surfaces within a day rather than on the next contributor's `make verify`. Treat that as a backstop, not a substitute: run the two commands above yourself before you push. CUDA verification is not gated at PR time either; that stays exclusive to `release.yml`.

While iterating, prefer `[profile.test-fast]` over `--release`: `make test-fast` / `make test-fast-cuda` (or `cargo test --profile test-fast --features <...>`) rebuild in seconds instead of minutes. It is for local/agent edit-test loops only; run the `--release` commands above (or `make verify`) before opening or updating a PR. See [`docs/installation.md`](docs/installation.md#fast-iteration-builds) for the measured comparison.
`make verify` runs exactly the three commands above, and the nightly invokes those same Makefile targets, so the local gate and CI cannot drift apart. Tests build under `[profile.test-fast]` rather than `--release`: `opt-level = 3` is kept so MLX numerics stay representative, while the fat LTO and single codegen unit that make a full `--release` test link take hours are dropped. `make test-fast` / `make test-fast-cuda` are the same profile with `--test-threads=1` and a `FILTER` hook for narrowing the run while you iterate. Reach for `cargo test --release --features metal,accelerate` by hand only when you suspect a defect specific to release codegen. See [`docs/installation.md`](docs/installation.md#fast-iteration-builds) for the measured comparison.
5. For inference changes, validate against a real checkpoint. Synthetic or build-only validation is not enough: a shape-compatible change can compile, pass unit tests, and still produce wrong logits on an actual quantized checkpoint. Fetch one with `mlxcel download mlx-community/<model-id>`, and see [`docs/supported-models.md`](docs/supported-models.md) for the families each code path covers. A change to a shared component should be smoke-tested against at least two families.
6. Commit with a conventional prefix (see below) and a clear message.
7. Push to your fork and open a Pull Request. The PR template will prompt for a summary, test plan, and linked issues.
Expand Down
10 changes: 9 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,15 @@ panic = "unwind"
# DO NOT use this profile for anything that ships or gets measured:
# performance benchmarks, release binaries, or numbers quoted in a PR as
# representative throughput/latency. Use `[profile.release]` (via
# `make release*` / `make verify-test`) for those.
# `make release*`) for those.
#
# As of #1000 this profile also backs `make verify-test`, so it is what the
# CI-faithful gate and the nightly workflow run tests under. That is a
# deliberate trade: `opt-level = 3` is what makes the optimised-codegen
# argument for testing hold, and it is kept, while the fat LTO and single
# codegen unit that were consuming the nightly's entire budget in linking are
# not. It does not change what ships; release.yml still builds the shipped
# binaries under `[profile.release]`.
[profile.test-fast]
inherits = "release"
lto = "thin" # cross-crate inlining kept; drops the whole-program fat-LTO link
Expand Down
40 changes: 26 additions & 14 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -220,15 +220,19 @@ test-doc: ## Run documentation tests
# ----------------------------------------------------------------------------
# Fast dev-iteration test targets (issue #809)
#
# `test` / `verify-test` above build under [profile.release] (fat LTO,
# codegen-units = 1), which is the right choice for CI-faithful and shipping
# validation but measured at 4 to 6 minutes per incremental rebuild on the
# ~390k-line main crate, which is expensive for the edit-test-edit loop of local and
# agent development. These targets build under [profile.test-fast] instead
# (thin LTO, parallel codegen, incremental compilation); see the profile's
# Cargo.toml comment and docs/installation.md ("Fast iteration builds") for
# the measured speedup. Set FILTER to narrow the run, e.g.
# `release*` and a hand-run `cargo test --release` build under
# [profile.release] (fat LTO, codegen-units = 1), which is the right choice for
# shipping validation but measured at 4 to 6 minutes per incremental rebuild on
# the ~390k-line main crate, which is expensive for the edit-test-edit loop of
# local and agent development. These targets build under [profile.test-fast]
# instead (thin LTO, parallel codegen, incremental compilation); see the
# profile's Cargo.toml comment and docs/installation.md ("Fast iteration
# builds") for the measured speedup. Set FILTER to narrow the run, e.g.
# `make test-fast-cuda FILTER=server::chat_request`.
#
# `verify-test` below also builds under [profile.test-fast] as of #1000, so
# these targets and the CI-faithful gate now differ only in feature set and
# `--test-threads=1`, not in codegen.
# ----------------------------------------------------------------------------

.PHONY: test-fast
Expand Down Expand Up @@ -450,9 +454,17 @@ pre-commit: fmt clippy test ## Pre-commit checks
# 2. `-D warnings` — promotes every clippy warning to an error, matching
# `-- -D warnings` in CI. The default `clippy` target uses `-W warnings`
# and silently hides regressions.
# 3. `cargo test --release` — CI runs tests in release mode for realistic
# MLX/Metal codegen. Debug-mode tests can pass while release-mode tests
# hit different optimisation paths.
# 3. `--profile test-fast`: CI runs tests optimised, not in debug, for
# realistic MLX/Metal codegen. Debug-mode tests can pass while optimised
# tests hit different paths. `test-fast` keeps `opt-level = 3`, which is
# what that argument actually rests on, and drops the fat LTO and
# `codegen-units = 1` of `[profile.release]`, which exist to tune a
# shipped binary and were costing the nightly its whole budget in
# codegen and linking (#1000). release.yml still builds and links what
# ships under `[profile.release]`. The residual gap is a defect that
# reproduces only under fat LTO or single-unit codegen; use
# `cargo test --release --features metal,accelerate` by hand when you
# are chasing one.
#
# Run `make verify` before opening or updating a PR. Run `make verify-clean`
# (which prepends `cargo clean`) when you suspect clippy's per-crate result
Expand All @@ -471,9 +483,9 @@ verify-clippy: ## CI-faithful: clippy --all-targets --features metal,accelerate
$(CARGO) clippy --all-targets --features metal,accelerate -- -D warnings

.PHONY: verify-test
verify-test: ## CI-faithful: cargo test --release --features metal,accelerate
@echo "$(CYAN)[verify] test (release, features=metal,accelerate)...$(RESET)"
$(CARGO) test --release --features metal,accelerate
verify-test: ## CI-faithful: cargo test --profile test-fast --features metal,accelerate
@echo "$(CYAN)[verify] test (test-fast profile, features=metal,accelerate)...$(RESET)"
$(CARGO) test --profile test-fast --features metal,accelerate

.PHONY: verify
verify: verify-fmt verify-clippy verify-test ## Run the full CI-faithful gate locally (recommended before push)
Expand Down
32 changes: 21 additions & 11 deletions docs/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -277,17 +277,17 @@ racing another thread's stream capture) or, with graphs disabled, with
binaries are unaffected; this is a test-parallelism artifact.

```bash
cargo test --release --features cuda -- --test-threads=1
cargo test --profile test-fast --features cuda -- --test-threads=1
```

## Fast iteration builds

`cargo test --release` (and `cargo build --release`) use `[profile.release]`:
fat LTO across all ~439 locked crates plus `codegen-units = 1` for the
~390k-line main crate. That is the right tradeoff for CI-faithful validation
and for anything you ship, but it is expensive for the day-to-day edit-test
loop: measured at 4 to 6 minutes per incremental rebuild, so a typical issue
cycle of several edit-test iterations pays 20+ minutes of pure compile time.
`cargo build --release` (and a hand-run `cargo test --release`) use
`[profile.release]`: fat LTO across all ~439 locked crates plus
`codegen-units = 1` for the ~390k-line main crate. That is the right tradeoff
for anything you ship, but it is expensive for the day-to-day edit-test loop:
measured at 4 to 6 minutes per incremental rebuild, so a typical issue cycle of
several edit-test iterations pays 20+ minutes of pure compile time.

For local and agent development, use `[profile.test-fast]` instead (thin LTO,
`codegen-units = 16`, incremental compilation, `strip = false`; still
Expand Down Expand Up @@ -317,10 +317,20 @@ versus 4 to 6 minutes under `[profile.release]`, roughly a 13x to 19x
iteration speedup. A representative narrow test set (139 tests across model,
server, sampling, and cache modules) passes identically under both profiles.

`test-fast` is for iteration only. Use `[profile.release]` (`make release*`,
`make verify-test`, or plain `cargo test --release`) for anything you ship,
benchmark, or quote as representative performance: `test-fast` trades link
time and binary size for rebuild speed and is not tuned for either.
Use `[profile.release]` (`make release*`, or plain `cargo build --release`) for
anything you ship, benchmark, or quote as representative performance:
`test-fast` trades link time and binary size for rebuild speed and is not tuned
for either.

Running *tests* is the exception. `make verify-test`, and therefore the
[nightly workflow](https://github.com/lablup/mlxcel/blob/main/.github/workflows/nightly-verify.yml)
that invokes it, builds the test binaries under `test-fast` as well. Linking
roughly 77 test binaries under fat LTO was costing that job its entire
180-minute budget before a single test ran. `opt-level = 3` is unchanged, so
the optimised MLX numerics the suite depends on are the same; what is no longer
covered is a defect that reproduces only under fat LTO or `codegen-units = 1`.
Reach for `cargo test --release --features metal,accelerate` by hand when you
are chasing one of those.

## Troubleshooting

Expand Down