fix(hybrid): resolve ECG denoiser INT8 quantization tail risk#58
Merged
Conversation
Root-causes and fixes the tail-risk quantization error flagged in #55: direct-mode (non-gain) denoisers showed up to 21.3% PRD delta vs the float32 Keras reference on real held-out ECG frames, even after the calibration-normalization fix from #47. ## Fixes 1. Richer calibration data: fold in up to 100 real, already-normalized evaluation frames (sampled from the same pairs used for scoring) alongside the 10 synthetic sample_stimulus.npz frames, so INT8/INT16X8 calibration ranges reflect real held-out data, not just a handful of synthetic physiokit windows. 2. Per-mode quantization scheme: gain-mode denoisers (output in [0, 1], multiplies the full-precision input coefficient) stay INT8 -- measured safe (max ~0.002% PRD delta). Direct-mode denoisers (output replaces the coefficient outright, so quantization error becomes reconstruction error directly) now export as INT16X8 instead, using the finer resolution to stay within tolerance. 3. Generalized compressionkit.runtime._litert.quantize()/dequantize() to handle int16 tensors (previously only branched on int8, silently falling through to plain dtype-cast for int16 -- would have produced garbage output the moment an INT16X8 model was ever loaded). 4. model_card.py's hybrid C quickstart section now reports the actual per-model quantization scheme (INT8 vs INT16X8) read from hybrid_manifest.json's mode.gain_mode, instead of hardcoding "INT8". ## Measured results (real held-out ECG frames, Keras vs TFLite PRD delta) - Before (INT8, 10-frame calibration): median 3.76%, p99 18.6%, max 21.3% - After (INT16X8, 100+10-frame calibration): median ~0.4%, max ~0.7% PPG's gain-mode denoiser is unaffected (still INT8, still negligible delta) -- this only changes behavior for direct-mode models (ECG today). ## Validation - Regenerated all 11 local hybrid goldens; `golden validate-all --strict-release` passes 33/33. - 449 tests pass (2 new: INT16 quantize/dequantize round-trip, INT16X8 export accuracy on a direct-mode toy model). - ruff clean, zensical build clean. - Did not republish to HuggingFace this session. Closes #55
There was a problem hiding this comment.
Pull request overview
This PR reduces hybrid ECG denoiser quantization tail-risk by (1) expanding representative calibration data with real eval frames and (2) switching direct-mode denoisers to INT16X8, plus updating runtime quantize/dequantize helpers to correctly handle int16 tensors and adding regression tests.
Changes:
- Sample up to 100 already-normalized eval frames and fold them into denoiser TFLite representative datasets during hybrid golden export.
- Export gain-mode denoisers as INT8 but direct-mode denoisers as INT16X8 (with int16 I/O), and generalize LiteRT quantize/dequantize to apply scale/zero-point math for int16.
- Add tests covering int16 quantize/dequantize round-trip and INT16X8 denoiser export fidelity on a direct-mode toy model; update model card text to report INT8 vs INT16X8 (with a documentation correctness issue noted below).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/test_denoiser_tflite.py |
Adds regression tests for int16 quantize/dequantize and INT16X8 export accuracy on direct-mode denoisers. |
scripts/run_hybrid_golden.py |
Uses real eval frames for calibration and selects INT8 vs INT16X8 export based on denoiser mode. |
compressionkit/runtime/_litert.py |
Extends quantize/dequantize helpers to correctly handle int16 quantized tensors. |
compressionkit/export/model_card.py |
Updates hybrid C quickstart wording to reflect per-model quantization (INT8 vs INT16X8), but needs an additional guard for missing .tflite artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+553
to
+557
| denoiser_quantization = "INT8" | ||
| hybrid_manifest_path = deploy_dir / "hybrid_manifest.json" | ||
| if hybrid_manifest_path.exists(): | ||
| with hybrid_manifest_path.open() as f: | ||
| hybrid_manifest = json.load(f) |
- Fix misleading "Failed to export INT8 denoiser TFLite" log message to be quantization-scheme-agnostic (it's INT8 or INT16X8 depending on denoiser mode). (Copilot review) - generate_spiht_model_card()'s C quickstart section now only claims a quantized denoiser_gain_model.tflite when hybrid_manifest.json's denoise stage actually has both artifact_tflite and mode -- falls back to the original "Python/TFLite runtime path" wording for older packages or failed exports, instead of always assuming a .tflite ships. (Copilot review) - Added 3 regression tests covering: INT16X8 wording for direct-mode denoisers, INT8 wording for gain-mode denoisers, and the Keras-only fallback text when artifact_tflite/mode are absent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root-causes and fixes the tail-risk quantization error flagged in #55: the ECG hybrid lane's direct-mode (non-gain) denoiser showed up to 21.3% PRD delta vs the float32 Keras reference on real held-out frames, even after the calibration-normalization fix from #47.
Fixes
sample_stimulus.npzframes, so calibration ranges reflect real held-out data rather than a handful of synthetic physiokit windows.[0, 1], multiplies the full-precision input coefficient) stay INT8 — measured safe (max ~0.002% PRD delta). Direct-mode denoisers (output replaces the coefficient outright, so quantization error becomes reconstruction error directly) now export as INT16X8 instead.compressionkit.runtime._litert.quantize()/dequantize()to handle int16 tensors — previously only branched on int8, silently falling through to a plain dtype-cast for int16 (would have produced garbage output the moment an INT16X8 model was ever loaded).model_card.py's hybrid C quickstart section now reports the actual per-model quantization scheme (INT8 vs INT16X8) read fromhybrid_manifest.json, instead of hardcoding "INT8".Measured results (real held-out ECG frames, Keras vs TFLite PRD delta)
PPG's gain-mode denoiser is unaffected (still INT8, still negligible delta) — this only changes behavior for direct-mode models (ECG today).
Testing
golden validate-all --strict-release→ 33/33.uv run pytest -q— 449 passed (2 new: INT16 quantize/dequantize round-trip, INT16X8 export accuracy on a direct-mode toy model).uv run ruff check ./uv run ruff format --check .— clean.uv run zensical build— clean.Closes #55