fix(math): make resonance points transformer-independent and consistent#127
Merged
Conversation
…nt (A6) The five places that build resonance point-sets each used the transformer-corrected quarter-wave, so a Unun/Balun choice silently shifted the physical resonance geometry, and the sets disagreed with one another: - non-resonant avoid-set (calculate_non_resonant_optima) - resonant-compromise optimizer - OCFD split clearance optimizer - on-screen resonant_points_in_window - exported resonant_points_in_window All five now use the uncorrected quarter-wave (71.32/f·VF), which is a property of the wire geometry, not the feed. The optimizers, the on-screen list and the export now always agree and never move with the transformer ratio. The transformer-length heuristic still adjusts the displayed *build* lengths (corrected_*), which is unchanged. Deliberately does NOT adopt the earlier half-wave avoid-set idea: the review showed it introduces a second, conflicting definition of "resonance" versus the display/export/compromise sets. Keeping the tool's documented, unified "avoid all quarter-wave resonances" model is consistent and defensible for a tuner-fed non-resonant wire. - advise: note that the recommended length is the same for every ratio (resonance is independent of the feed) so the constant column isn't misread - docs/math.md §5/§6: points are uncorrected and transformer-independent - test: resonance_point_optimizers_are_transformer_independent guards all three optimizers against per-ratio drift Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… helper Addresses the #127 code-review findings: - [2][3][4] The previous commit switched the five resonance-point sites to the fully-raw quarter_wave_m, which dropped the *conductor-diameter* correction (a real geometry effect) along with the transformer heuristic (intended). A thick-conductor build could then be recommended sitting on a real resonance. Introduce WireCalculation::resonant_quarter_wave_m = quarter_wave_m x conductor_diameter_correction_factor(d) — conductor-corrected but transformer-independent — and use it everywhere. - [7] Extract one shared band_resonant_points_m() helper; all five consumers (non-resonant avoid-set, compromise, OCFD clearance, on-screen list, export) now go through it, so the harmonic loop can't drift between copies again. - [6] The non-resonant avoid-set uses a 1 m edge pad (for honest boundary clearance) while the others list strictly in-window points; docs/comments no longer overclaim the lists are byte-identical, only that the harmonic positions come from the same feed-independent formula. - [5] Mirror the CLI's "length is the same for every ratio" note in the TUI advise panel. - tests: resonant_points_keep_conductor_diameter_correction guards the physical correction is retained (thick < thin, and != raw quarter-wave). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
Applied the high-effort review findings in
All 367 lib tests + integration pass; clippy + fmt clean. |
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
Follow-up to #125. Fixes the resonance-point handling that the #125 review flagged: five separate places built resonance point-sets from the transformer-corrected quarter-wave, so choosing a Unun/Balun silently shifted the physical resonance geometry, and the sets disagreed with each other.
All five now use the uncorrected quarter-wave (
71.32/f · VF) — resonance is a property of the wire, not the feed:calculate_non_resonant_optima)resonant_points_in_windowresonant_points_in_windowThey now always agree, and none shift with the transformer ratio. The transformer-length heuristic still adjusts the displayed build lengths (
corrected_*) — unchanged.Review findings addressed (from #125)
Design decision (deliberate)
This does not adopt the earlier half-wave avoid-set idea. The review showed it introduces a second, conflicting definition of "resonance" versus the display/export/compromise sets. Keeping the tool's documented, unified "avoid all quarter-wave resonances" model (staying in the moderate-reactance region between resonances) is internally consistent and defensible for a tuner-fed non-resonant wire. A high-Z/low-Z point-type distinction would be a larger UX feature; noted for future work.
Test plan
resonance_point_optimizers_are_transformer_independentguards all three optimizers against per-ratio drift (asserts corrected lengths DO change but resonance points do NOT)cargo testall pass (366 lib + integration),clippyclean,fmtclean🤖 Generated with Claude Code
https://claude.ai/code/session_01Jf8uBU8AYxkrG8fa8yo6tC