lablup / lablup/mlxcel

chore(scripts): honor MLXCEL_BIN and MODEL overrides in measure_sparse_v_skip_rate.sh

Open Beginner friendly
#1,661 0 comments 0 reactions 0 assignees View on GitHub
priority:low status:ready type:chore
Dominant language
Rust
Stars
467
Forks
54
Avg merge
4h 25m
Merged PRs (30d)
310

Description

## Summary

`scripts/measure_sparse_v_skip_rate.sh` hardcodes the binary and model paths, so it cannot run from a non-default build directory. Its sibling bench scripts already accept an `MLXCEL_BIN` environment override. This makes the sparse-V script consistent with them.

## Background

The benchmark scripts under `scripts/` are meant to run against different build outputs and model stores. Most of them read the binary path from `MLXCEL_BIN` with a fallback to the release path, so a caller can point them at a custom build. `measure_sparse_v_skip_rate.sh` is the exception: it assigns the paths unconditionally, giving the caller no way to redirect the binary.

## Proposed Solution

Introduce environment fallbacks for both the binary and the model path, mirroring the pattern already used in `scripts/bench_block_width.sh`. Keep the default behavior identical when the variables are unset.

## Implementation Notes

- `scripts/measure_sparse_v_skip_rate.sh:22-23` currently reads:

```bash
MLXCEL="./target/release/mlxcel"
MODEL="models/qwen3-4b-4bit"
```

Change to:

```bash
MLXCEL=${MLXCEL_BIN:-./target/release/mlxcel}
MODEL=${MODEL:-models/qwen3-4b-4bit}
```

- Reference implementation in the same directory: `scripts/bench_block_width.sh:43` uses `BIN=${MLXCEL_BIN:-target/release/mlxcel}`.
- The model is already overridable positionally through the `*)` case in the argument loop, so only the environment fallbacks are missing. Confirm the positional override still wins after this change (a later positional assignment should not be clobbered by the new default).
- The script uses `set -uo pipefail`, so an unset `MLXCEL_BIN` or `MODEL` must be referenced through the `${VAR:-default}` form (as above) rather than a bare `$VAR`.

## Acceptance Criteria

- [ ] `MLXCEL_BIN=/some/path scripts/measure_sparse_v_skip_rate.sh` uses the given binary
- [ ] `MODEL=/some/model scripts/measure_sparse_v_skip_rate.sh` uses the given model, and a positional model argument still takes precedence
- [ ] Default behavior without either variable is unchanged

---

## Original Suggestion

### Title: chore(scripts): honor MLXCEL_BIN/MODEL overrides in measure_sparse_v_skip_rate.sh

`scripts/measure_sparse_v_skip_rate.sh` hardcodes the release-binary path, unlike its sibling bench scripts which honor an `MLXCEL_BIN` environment override. The script fails from any non-default build directory with no way to redirect it.

## Evidence

`scripts/measure_sparse_v_skip_rate.sh:22-23`:

```bash
MLXCEL="./target/release/mlxcel"
MODEL="models/qwen3-4b-4bit"
```

Compare `scripts/bench_block_width.sh:43`:

```bash
BIN=${MLXCEL_BIN:-target/release/mlxcel}
```

The model is already overridable positionally (the `*)` case in the argument loop), so only the env fallbacks are missing.

## Suggested fix

Two-line change with an in-repo reference implementation: `MLXCEL=${MLXCEL_BIN:-./target/release/mlxcel}` and `MODEL=${MODEL:-models/qwen3-4b-4bit}`.

## Acceptance criteria

- [ ] `MLXCEL_BIN=/some/path scripts/measure_sparse_v_skip_rate.sh` uses the given binary
- [ ] Default behavior without the variables is unchanged

Contributor guide

Open the contributing guide

Research direction

Open scripts/measure_sparse_v_skip_rate.sh around lines 22-23 and compare its path setup with scripts/bench_block_width.sh:43. Verify the argument loop, especially the *) case, to ensure a positional model still takes precedence. Done means the script honors MLXCEL_BIN and MODEL when set while preserving both default behavior and positional overrides.

Written by the indexing model from the issue text.

Assessment

Tech stack
shell
Domain
tooling
Issue type
Feature
Difficulty
1/5
Estimated time
Under an hour
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
92/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.