fix(comb): k=0 periodic-chirp comb builder replaces the warped 2^N LFM path #50

Merged

ykp merged 1 commit from fix-lfm-periodic-comb into master

2026-10-07 18:10:37 +02:00

ykp commented

2026-10-07 18:07:27 +02:00

Owner

Summary

Operator-confirmed redesign of the comb's CombOversampling == 0 path: the LFM branch now synthesizes a periodic chirp of exactly one tone-spacing period, P = round(Fe/spacing) samples, with continuous phase — tiling it yields a frequency comb with the exact spacing Fe/P (the requested ToneSpacing rounded to a whole-sample period), with no power-of-two bin warping.

What was wrong with `create_lfm_tones_old`

Built a power-of-two IFFT block and quantized every tone to the Fe/nfft grid → the realized comb spacing was warped and the block period was nfft, not one comb period P.
The quadratic bin phase implies a chirp sweeping the band in one spacing period, but the block was nfft long → phase discontinuities when tiled (no periodic chirp).
Additional defects: no ±Fe/2 tone filtering (out-of-range tones raised IndexError or silently wrapped via numpy negative indexing), duplicate bins overwrote instead of summing, max() on an empty array for all-zero scales, an inconsistent phase constant (−0.25 vs the −0.125 used in create_multi_tones' fallback), pseudo-docstring strings as comments.
The ticket-39 create_lfm_tones was dead code with the same root flaw (tones quantized to the nfft grid ⇒ the P-sample block was not P-periodic).

The fix

create_lfm_tones(tones, scales, spacing_hz, fe) rewritten as the direct periodic builder: ONE block of P = round(Fe/spacing) samples; full union comb (a line at every Fe/P multiple across the union span of all bands, edges nearest-snapped); uniform −π/4 quadratic (chirp) phase law anchored at the union's lowest line; whole chirp scaled to max(|scales|) (per-band Levels intentionally not per-line — operator decision). Strictly P-periodic ⇒ continuous phase when tiled. The tone sum is evaluated as a length-P inverse DFT — the transform length is the comb period, so it is an exact, efficiency-equivalent evaluation with no 2^N spectral quantization.
create_lfm_tones_old deleted; dead code cleaned; __all__ updated.
Unified dispatch multi_tones.synthesize_comb_block + geometry single source comb_block_samples / comb_period_samples: comb.py and comb_iq.py both route oversampling == 0 → periodic-chirp builder (deterministic, RNG-free, iteration-cap-irrelevant) and k > 0 → the seeded PSO IFFT builder. The iteration_cap == 0 IFFT fallback inside create_multi_tones is untouched (behavior unchanged; only its stale "linear-phase" docstring corrected).
Validation L13 for k = 0 now bounds one comb period (round(Fe/spacing)/Fe) — the old 2^floor bound could pass durations too short for a full period.
Multi-band gaps: carved by the existing BandIsolation post-chain stage (Q26) when enabled — no in-generator notch (operator decision, option C).
spec.md §3.1 / §5.2.2 / L13 updated: k = 0 documented as the periodic-chirp mode.

Tests

create_lfm_tones: exact period (67 = round(200e6/3e6), NOT 2^N); tiled spectrum has lines ONLY at fe/P multiples across the union span (gap lines included) with uniform magnitude; peak = max(|scales|); empty/zero-scale/out-of-band → zeros; input validation.
Dispatch: k=0 RNG-free and seed-independent; k>0 identical to a direct create_multi_tones call.
End-to-end: comb and comb-iq with CombOversampling = 0 produce exactly P-periodic output under hard tiling.
pytest 1306 passed / 64 skipped (headless UI tier) · ruff check gensig tests clean · mypy gensig tests clean.

## Summary Operator-confirmed redesign of the comb's `CombOversampling == 0` path: the LFM branch now synthesizes a **periodic chirp** of exactly one tone-spacing period, `P = round(Fe/spacing)` samples, with continuous phase — tiling it yields a frequency comb with the **exact** spacing `Fe/P` (the requested `ToneSpacing` rounded to a whole-sample period), with **no power-of-two bin warping**. ## What was wrong with `create_lfm_tones_old` - Built a power-of-two IFFT block and quantized every tone to the `Fe/nfft` grid → the realized comb spacing was warped and the block period was `nfft`, not one comb period `P`. - The quadratic bin phase implies a chirp sweeping the band in one *spacing* period, but the block was `nfft` long → phase discontinuities when tiled (no periodic chirp). - Additional defects: no ±Fe/2 tone filtering (out-of-range tones raised `IndexError` or silently wrapped via numpy negative indexing), duplicate bins overwrote instead of summing, `max()` on an empty array for all-zero scales, an inconsistent phase constant (`−0.25` vs the `−0.125` used in `create_multi_tones`' fallback), pseudo-docstring strings as comments. - The ticket-39 `create_lfm_tones` was dead code with the same root flaw (tones quantized to the `nfft` grid ⇒ the `P`-sample block was not `P`-periodic). ## The fix - **`create_lfm_tones(tones, scales, spacing_hz, fe)`** rewritten as the direct periodic builder: ONE block of `P = round(Fe/spacing)` samples; **full union comb** (a line at *every* `Fe/P` multiple across the union span of all bands, edges nearest-snapped); uniform **−π/4 quadratic (chirp) phase law** anchored at the union's lowest line; whole chirp scaled to `max(|scales|)` (per-band Levels intentionally not per-line — operator decision). Strictly `P`-periodic ⇒ continuous phase when tiled. The tone sum is evaluated as a length-`P` inverse DFT — the transform length *is* the comb period, so it is an exact, efficiency-equivalent evaluation with no 2^N spectral quantization. - **`create_lfm_tones_old` deleted**; dead code cleaned; `__all__` updated. - **Unified dispatch** `multi_tones.synthesize_comb_block` + geometry single source `comb_block_samples` / `comb_period_samples`: `comb.py` and `comb_iq.py` both route `oversampling == 0` → periodic-chirp builder (deterministic, RNG-free, iteration-cap-irrelevant) and `k > 0` → the seeded PSO IFFT builder. The `iteration_cap == 0` IFFT fallback inside `create_multi_tones` is untouched (behavior unchanged; only its stale "linear-phase" docstring corrected). - **Validation L13** for `k = 0` now bounds one comb period (`round(Fe/spacing)/Fe`) — the old `2^floor` bound could pass durations too short for a full period. - **Multi-band gaps**: carved by the existing BandIsolation post-chain stage (Q26) when enabled — no in-generator notch (operator decision, option C). - **spec.md** §3.1 / §5.2.2 / L13 updated: `k = 0` documented as the periodic-chirp mode. ## Tests - `create_lfm_tones`: exact period (67 = `round(200e6/3e6)`, NOT 2^N); tiled spectrum has lines ONLY at `fe/P` multiples across the union span (gap lines included) with uniform magnitude; peak = `max(|scales|)`; empty/zero-scale/out-of-band → zeros; input validation. - Dispatch: k=0 RNG-free and seed-independent; k>0 identical to a direct `create_multi_tones` call. - End-to-end: `comb` and `comb-iq` with `CombOversampling = 0` produce exactly `P`-periodic output under hard tiling. - `pytest` 1306 passed / 64 skipped (headless UI tier) · `ruff check gensig tests` clean · `mypy gensig tests` clean.

ykp added 1 commit

2026-10-07 18:07:27 +02:00

fix(comb): k=0 periodic-chirp comb builder replaces the warped 2^N LFM path

test / test (ubuntu-latest) (pull_request) Successful in 2m17s

Details

e43c027163

Operator-confirmed design (k=0 = periodic-chirp mode; the iteration_cap
fallback and the k>0 PSO IFFT path stay untouched):

- create_lfm_tones rewritten as the direct periodic builder: ONE block of
  P = round(Fe/spacing) samples (NOT a power of two), full union comb (a
  line at EVERY Fe/P multiple across the union span, edges nearest-
  snapped), uniform -pi/4 quadratic (chirp) phase law anchored at the
  union's lowest line. Every line is on the exact Fe/P grid, so the
  block is strictly P-periodic: continuous-phase when tiled, comb spacing
  = Fe/P (spacing rounded to whole samples) with no 2^N bin warping. The
  tone sum is evaluated as a length-P inverse DFT (transform length IS
  the comb period — exact, no spectral quantization). Per-band Levels
  are not respected line-by-line (uniform chirp): the whole chirp is
  scaled to max(|scales|); inter-band gaps are carved by the BandIsolation
  post-chain stage (Q26) when enabled.
- create_lfm_tones_old deleted (2^N IFFT quantization warped the comb
  spacing; no band filtering, duplicate-bin overwrite, empty-max crash,
  phase discontinuity at the block boundary).
- Shared dispatch multi_tones.synthesize_comb_block + geometry single
  source multi_tones.comb_block_samples/comb_period_samples: comb.py AND
  comb_iq.py route k=0 to the periodic-chirp builder and k>0 to the PSO
  IFFT builder; validation L13 (k=0) now bounds one comb period.
- spec.md 5.2.2/3.1/L13: k=0 documented as the periodic-chirp mode; the
  create_multi_tones fallback docstring corrected (quadratic, not the
  MATLAB linear phase — behavior unchanged).
- Tests: block period/periodicity, exact-grid union comb with gap lines,
  uniform scale + peak reference, guards, dispatch equivalence; k=0
  end-to-end periodicity for comb and comb-iq.

ykp merged commit 0d03e85350 into master

2026-10-07 18:10:37 +02:00

ykp referenced this pull request from a commit

2026-10-07 18:10:38 +02:00

Merge pull request 'fix(comb): k=0 periodic-chirp comb builder replaces the warped 2^N LFM path' (#50) from fix-lfm-periodic-comb into master

ykp deleted branch fix-lfm-periodic-comb

2026-10-07 18:10:44 +02:00