fix(math): stop double-counting the ~0.95 end-effect in resonant lengths - #125
Merged
Conversation
The length coefficients (142.6465/f m etc.) are the classic imperial handbook rules (468/936/234/1005 ft), which already include the ~0.95 bare-wire end-effect shortening. The default velocity factor of 0.95 multiplied it a second time, so every default length came out ~5% short (a 40 m dipole was 19.09 m and would resonate near 7.47 MHz, outside the band), disagreeing with the project's own NEC reference decks (~20.07 m). Fix by treating VF as an additional insulated-wire multiplier defaulting to 1.0 (bare wire); the coefficients are unchanged. - cli/app defaults + interactive prompt: velocity 0.95 -> 1.0 - docs/math.md §1: document the VF convention; §4 Z_ref drift; §10 trap dipole is a wire-budget estimate; README/architecture disclaimers - fnec_validation: horizontal wire matching its docstring + corpus decks, forced odd segment count, mismatch target aligned to the model's own R - calculations: exact METERS_TO_FEET reciprocal, loop-sampling epsilon, frequency-validity caveat on nec_calibrated_dipole_r - tests: corpus expectations to physical reference (~20.09 m), new absolute handbook-length regression guard, robust JSON-precision check Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Clears the RustSec advisory (unsoundness in Error::downcast_mut) that the CI `cargo audit --deny warnings` gate now flags. anyhow is a transitive lock entry only (not reachable in the binary), so this is a lockfile-only, zero-risk bump; it fails identically on main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses code-review findings on the A1 core: - fnec_validation: keep the mismatch reference at 70 Ω (reverting the 62.94 Ω change) so pass/warn/reject classification stays calibrated - README / architecture: velocity-factor default is 1.0 (bare wire), not 0.95 — docs now match the shipped default - export contract test: assert the half_wave_m field specifically is formatted to 2 decimals, instead of scanning for any 2-decimal number - tests: reuse METERS_TO_FEET instead of re-deriving 1.0/0.3048 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dc0sk
force-pushed
the
fix/antenna-math-vf-double-count
branch
from
July 3, 2026 23:42
9992365 to
9e865ee
Compare
dc0sk
changed the title
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
1 participant
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
Fixes a CRITICAL RF-math error: the resonant-length coefficients (
142.6465/fm etc.) are the classic imperial handbook rules (468/936/234/1005ft), which already include the ~0.95 bare-wire end-effect shortening. The default velocity factor of0.95multiplied it a second time, so every default length came out ~5% short — a 40 m dipole was19.09 mand would resonate near 7.47 MHz, outside the band — disagreeing with the project's own NEC reference decks (cut at ~20.07 m) and a self-contradicting corpus test.Fix: treat VF as an additional insulated-wire multiplier defaulting to
1.0(bare wire); coefficients unchanged. 40 m dipole now20.09 m, loop43.14 m, quarter10.05 m— all matchingcorpus/*.nec.Changes
cli/AppConfigdefault + interactive prompt: velocity0.95 → 1.0docs/math.md§1 documents the VF convention; §4 Z_ref drift fixed; §10 trap-dipole reframed as a wire-budget estimate; README/architecture disclaimersfnec_validation: horizontal wire matching its docstring +corpus/decks, forced odd segment count (mismatch target kept at 70 Ω)calculations: exactMETERS_TO_FEET = 1/0.3048, loop-sampling epsilon, frequency-validity caveat onnec_calibrated_dipole_rchore(deps): bumpanyhow1.0.102 → 1.0.103 to clear RUSTSEC-2026-0190 (transitive lock entry; the CIcargo auditgate now flags it)Not in this PR (follow-up)
The non-resonant avoid-set physics (avoid half-wave, not all quarter-wave multiples) and decoupling resonance points from the transformer heuristic were prototyped but deferred to a dedicated follow-up — a review showed they need coordinated updates across all five resonance-point consumers (advise/export/display) plus UX decisions that shouldn't be rushed into this fix.
Test plan
cargo test— all pass (365 lib + integration)cargo clippy --all-targets— cleancargo fmt --check— clean🤖 Generated with Claude Code
https://claude.ai/code/session_01Jf8uBU8AYxkrG8fa8yo6tC