Skip to content

fix(colgrep): persist FP32 override so settings --fp32 forces full precision (#130)#140

Merged
raphaelsty merged 1 commit into
mainfrom
fix/settings-fp32-persist
Jun 17, 2026
Merged

fix(colgrep): persist FP32 override so settings --fp32 forces full precision (#130)#140
raphaelsty merged 1 commit into
mainfrom
fix/settings-fp32-persist

Conversation

@raphaelsty

Copy link
Copy Markdown
Collaborator

Summary

Fixes #130. colgrep settings --fp32 was a no-op on non-CUDA builds: it printed success but the index/search path kept selecting model_int8.onnx.

Root cause: the --fp32 branch called config.clear_fp32() (sets the field to None). Config::use_fp32() resolves a missing value per build:

#[cfg(feature = "_cuda")]    { self.fp32.unwrap_or(true) }   // FP32 default
#[cfg(not(feature = "_cuda"))] { self.fp32.unwrap_or(false) } // INT8 default

So on CoreML/CPU builds, clearing → Noneuse_fp32() == false → INT8. Clearing is not equivalent to forcing FP32.

Changes

  • --fp32 now persists Some(true) (set_fp32(true)) so use_fp32() returns true on every build → forces model.onnx.
  • New --default-precision flag explicitly resets to the build default (FP32 on CUDA, INT8 otherwise). This is what clear_fp32() now backs, mirroring the existing reset-to-default pattern (e.g. --pool-factor 0, --clear-ignore).
  • Status output shows the effective precision and whether it's an explicit override or the build default (previously hard-coded fp32 (default), which was itself misleading).
  • Help text + README updated.

Tests

  • test_set_fp32_forces_fp32_regression_130 — asserts set_fp32(true)use_fp32() == true, set_fp32(false)false, clear_fp32()None (the regression).
  • test_fp32_override_survives_serialization — JSON round-trip keeps Some(true) and use_fp32().
  • Manual end-to-end (isolated config dir):
    • settings --fp32precision: fp32, persists "fp32": true
    • settings --default-precisionprecision: int8 (build default) on a non-CUDA build
    • settings --fp32 --int8 → conflict error

make ci-quick (fmt + clippy -D warnings + tests) passes via the pre-commit hook.

Closes #130

…precision (#130)

`colgrep settings --fp32` called `clear_fp32()`, which drops the persisted
field. On non-CUDA builds a missing value resolves to INT8, so `--fp32` was a
no-op there: it printed success but the index/search path still selected
`model_int8.onnx`.

- `--fp32` now persists `Some(true)` via `set_fp32(true)` so `use_fp32()`
  resolves to true on every build.
- Add `--default-precision` to explicitly reset to the build default
  (FP32 on CUDA, INT8 otherwise) — this is what `clear_fp32()` now backs,
  matching the existing reset-to-default pattern (e.g. pool-factor 0).
- Status output shows the effective precision and whether it is an explicit
  override or the build default (no longer hard-codes "(default)").
- Add regression + serialization round-trip tests; update README and help.

Closes #130
@raphaelsty raphaelsty merged commit 46f7af9 into main Jun 17, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

colgrep settings --fp32 clears precision config on non-CUDA builds

1 participant