Skip to content

Align sine fit comparison fixture#91

Merged
Arcadia-1 merged 1 commit into
Arcadia-1:mainfrom
chenzc24:codex/sinfit-refinement-policy
Jul 8, 2026
Merged

Align sine fit comparison fixture#91
Arcadia-1 merged 1 commit into
Arcadia-1:mainfrom
chenzc24:codex/sinfit-refinement-policy

Conversation

@chenzc24

@chenzc24 chenzc24 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix the sine-fit comparison test to use the actual MATLAB reference folder name, run_sinfit.
  • Add the missing run_sinfit MATLAB reference outputs for sinewave_noise_200uV.
  • Make the Python integration fixture use MATLAB sinfit's refinement budget explicitly (max_iterations=100, tolerance=1e-12) while leaving the public fit_sine_4param defaults unchanged.

Rationale

This is a compare-fixture/policy alignment, not a core sine-fit algorithm change. MATLAB run_sinfit.m calls sinfit(read_data), whose default niter is 100 and tol is 1e-12. Python's public default intentionally remains lighter (max_iterations=1), but the MATLAB parity fixture should exercise the same refinement budget as MATLAB.

The previous compare test looked for test_sinfit, while the MATLAB runner and run_all_comparisons.py use run_sinfit, so the comparison failed before reaching numeric parity.

Experiment

Dataset: reference_dataset/sinewave_noise_200uV.csv

Default Python fixture vs MATLAB reference:

  • freq max abs diff: 2.30204744156e-13
  • mag max abs diff: 4.28363233773e-10
  • dc max abs diff: 1.9206858326e-14
  • phi max abs diff: 2.93023383335e-10
  • data_fit max abs diff: 4.76205318234e-10
  • convergence: False, n_iterations=1, emits RuntimeWarning

MATLAB-compatible fixture (max_iterations=100, tolerance=1e-12) vs MATLAB reference:

  • freq max abs diff: 2.4302782009e-13
  • mag max abs diff: 2.22044604925e-16
  • dc max abs diff: 9.99200722163e-16
  • phi max abs diff: 8.4354745411e-13
  • data_fit max abs diff: 7.06010805374e-11
  • convergence: True, n_iterations=2, no warning

Tests

  • uv run --with pytest pytest tests/integration/test_sine_fit.py -q --artifact-root E:/.tmp_artifacts/sinfit-refinement-policy
  • uv run --with pytest pytest tests/compare/test_compare_sine_fit.py -q --comparison-output-root E:/.tmp_artifacts/sinfit-refinement-policy/test_output
  • uv run --with pytest pytest tests/unit/fundamentals/test_verify_fit_sine_4param.py -q
  • uv run --with pytest pytest tests/unit/aout/test_error_fit_options.py -q
  • git diff --check

@Arcadia-1
Arcadia-1 merged commit 29218b0 into Arcadia-1:main Jul 8, 2026
1 check passed
@chenzc24
chenzc24 deleted the codex/sinfit-refinement-policy branch July 9, 2026 06:07
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.

2 participants