fix(solver): let a collinear split receive a plane wave, as it already transmits - #448
Merged
Conversation
Four identical "resolve effective wire endpoint list" blocks — three in `nec_solver/src/linear.rs`, one in `nec-cli/src/solve_session.rs` — trip `clippy::needless_late_init` on clippy 1.98. CI pins 1.97.1 and does not see them, so they are not a CI failure today; but `.githooks/pre-commit` runs the workspace clippy with `-D warnings` against whatever toolchain is installed, so on a 1.98 host every commit in this repo needs `--no-verify`. A quality gate that has to be bypassed in order to commit is not a gate. Mechanical: clippy's own suggested form, no behaviour change. `nec_solver`'s 232 lib tests pass unchanged and the workspace clippy is clean afterwards. Unrelated to the plane-wave fix on this branch; separated into its own commit for that reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018p7FxX7QMWNJVNp9LbaLkB
…y transmits
`build_planewave_hallen` grouped segments by raw `GW` card while its delta-gap
sibling `build_hallen_rhs` grouped by merged conductor — and while
`solve_hallen_planewave`, which consumes what it builds, was already being
handed the merged list by its caller. So a straight wire written as two
collinear `GW` cards was seen as a junction and refused outright, though the
same geometry driven by `EX 0` or `EX 4` solves (FND-142).
The merged list now drives all three uses at once: the junction test, the
segment grouping, and the along-wire coordinate. Fixing only the junction test
would have been the worse bug — see the sabotage below.
Gated by an equality, not a similarity: the pair is segmented identically
(25 + 25 against 50 over the same span), so every segment midpoint coincides
and nothing differs but the card boundary. Measured relative agreement 8.1e-12.
Sabotage-verified in two halves, because this one-line change does two things:
- revert the merge outright -> the deck is refused again, test fails;
- keep the merged junction test but restore the per-`GW` grouping -> the deck
SOLVES, at relative error 1.0004. That is what the blanket refusal had been
protecting against, and it is why the coordinate half matters as much as
the gate half.
A genuine bent junction is still refused, pinned by its own test: a bend is not
a collinear continuation, so the merge is a no-op there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018p7FxX7QMWNJVNp9LbaLkB
`scripts/check-test-catalog-counts.py` failed CI on #448 exactly as designed: the receive-twin invariance test and the bent-junction refusal test took the integration subtotal from 514 to 516. Numbers re-derived with the checker, not typed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018p7FxX7QMWNJVNp9LbaLkB
dc0sk
added a commit
that referenced
this pull request
Sep 7, 2026
With no `EX` card the Hallén system is homogeneous and solves to the zero
vector. All four frontends solved it; they then split on what they did with the
answer, and that split is why the check belongs in none of them:
- the CLI printed a full `CURRENTS` table of exactly `0.000000e0`, a
`RADIATION_PATTERN` of `-999.9900`, and `diag: ... abs_res=0.000000e0
rel_res=0.000000e0` — a flat response advertised as perfect convergence —
and exited 0;
- the GUI's currents and pattern views drew the same zeros as a coloured
overlay, with no caveat at all;
- the worker and `fnec_py` did refuse, but only after the wasted solve and
with the wrong reason: "no driven feedpoint (EX voltage source) found in
deck", which is the sentence a plane-wave receive deck gets — a feedpoint
problem, not a missing-card one.
The GUI had nothing to check with. The only no-`EX` test in the tree was a
`NoExCardValidator` declared *inside* the CLI's own `main` (FND-145).
The check now lives in `validate::pre_solve_error`, the gate all four frontends
call. That placement is the fix, not an implementation detail: a guard in the
Hallén RHS builders — the obvious spot, since that is where the zero vector is
born — would have been INERT for `--solver pulse` and `--solver continuity`,
which drive off `build_excitation` instead. Measured before: hallen, pulse,
continuity and sinusoidal each exit 0 with 51 zero rows; only mpie refused.
Measured after: all five exit 1 with zero bytes on stdout, swept by a test that
takes the mode list from the binary's own usage line rather than typing it.
The CLI's validator stays, promoted to error level and taking its sentence from
the shared predicate. It runs before the geometry build, so it skips the FR and
sweep-config parsing, the exec probe, `build_geometry` and the pairwise crossing
scan — it does NOT save a matrix fill, as an earlier draft of this message
claimed; `pre_solve_error` already refuses ahead of any assembly. It keeps EP-4's
`DeckValidator` integration demonstrated. Its cost is recorded rather than
glossed: the CLI now has two independent routes to this refusal, so the CLI's own
tests cannot discriminate a regression in the shared one.
BREAKING, deliberately, and stated more carefully than the first draft did.
`docs/json-output-schema.md` documented `[]` and exit 0 for this deck under
"Absence of feedpoint data" — NOT under that file's "Stability guarantee"
section, which covers the field set. Documented, not guaranteed; consumers
relied on it either way. That paragraph is rewritten here with a changelog
entry. A related consequence, measured: a deck with neither `EX` nor `FR` also
goes exit 0 -> exit 1, because validators run ahead of the FR check. A deck with
`EX` but no `FR` is unchanged at exit 0 and zero bytes — that is FND-084, still
open, and the rewrite no longer re-asserts the false general claim it corrects.
fnec diverges from nec2c here, conditionally: given an undriven deck WITH an
execute card, nec2c exits 0 and prints a zero currents table; with an `RP` it
prints `-nan` gains rather than a floor value; with neither it never executes.
Measured by review against this host's nec2c, and attributed as such in the code
comment rather than presented as the author's own measurement.
Sabotage-verified in two halves:
- drop the check from `pre_solve_error`: the GUI, worker and `nec_solver`
tests fail — and every CLI test still PASSES, because of that second route.
That is why each frontend gets its own gate rather than trusting the CLI's;
- demote the CLI validator back to a warning: the CLI's own test fails.
Both routes must go before `deck_validator.rs` notices, which is the third
sabotage that was actually run.
Two test fixtures changed. The GUI's `a_sweep_with_no_solvable_points_...` used
a no-`EX` deck as its "prepares but fails to solve" case; that class now fails
at prepare, so the test FAILS loudly — it was replaced because it broke, not to
head off a silent pass, which is what an earlier draft said. A plane-wave
receive deck replaces it: a real class rather than a degenerate one, and it
still exercises the per-point failure this pins. The worker's test expected
`NoFeedpoint`, reached by solving to zeros and then failing to price; it now
gets `UnsupportedConfig` naming the missing drive, distinguishing "nothing
drives this" from "the drive is receive-only". Both already crossed the wire as
`unsupported_config`, so only `error_message` changed.
Ledger: FND-145 new and fixed. FND-112 closed by measurement, and credited to
#432 (`4e7170f`) which actually fixed it, not to this PR — re-measured at
max_ma=1.232144e0 and +2.147 dBi against the audit's 0e0 and -999.99 floor.
FND-108 re-scoped, its central grep claim being false now. FND-146/147/148/149
recorded, not fixed. FND-070/FND-084 explicitly NOT closed.
Also carries the `### Fixed` changelog entry for #448's FND-142, which that PR
landed without one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018p7FxX7QMWNJVNp9LbaLkB
dc0sk
added a commit
that referenced
this pull request
Sep 7, 2026
With no `EX` card the Hallén system is homogeneous and solves to the zero
vector. All four frontends solved it; they then split on what they did with the
answer, and that split is why the check belongs in none of them:
- the CLI printed a full `CURRENTS` table of exactly `0.000000e0`, a
`RADIATION_PATTERN` of `-999.9900`, and `diag: ... abs_res=0.000000e0
rel_res=0.000000e0` — a flat response advertised as perfect convergence —
and exited 0;
- the GUI's currents and pattern views drew the same zeros as a coloured
overlay, with no caveat at all;
- the worker and `fnec_py` did refuse, but only after the wasted solve and
with the wrong reason: "no driven feedpoint (EX voltage source) found in
deck", which is the sentence a plane-wave receive deck gets — a feedpoint
problem, not a missing-card one.
The GUI had nothing to check with. The only no-`EX` test in the tree was a
`NoExCardValidator` declared *inside* the CLI's own `main` (FND-145).
The check now lives in `validate::pre_solve_error`, the gate all four frontends
call. That placement is the fix, not an implementation detail: a guard in the
Hallén RHS builders — the obvious spot, since that is where the zero vector is
born — would have been INERT for `--solver pulse` and `--solver continuity`,
which drive off `build_excitation` instead. Measured before: hallen, pulse,
continuity and sinusoidal each exit 0 with 51 zero rows; only mpie refused.
Measured after: all five exit 1 with zero bytes on stdout, swept by a test that
takes the mode list from the binary's own usage line rather than typing it.
The CLI's validator stays, promoted to error level and taking its sentence from
the shared predicate. It runs before the geometry build, so it skips the FR and
sweep-config parsing, the exec probe, `build_geometry` and the pairwise crossing
scan — it does NOT save a matrix fill, as an earlier draft of this message
claimed; `pre_solve_error` already refuses ahead of any assembly. It keeps EP-4's
`DeckValidator` integration demonstrated. Its cost is recorded rather than
glossed: the CLI now has two independent routes to this refusal, so the CLI's own
tests cannot discriminate a regression in the shared one.
BREAKING, deliberately, and stated more carefully than the first draft did.
`docs/json-output-schema.md` documented `[]` and exit 0 for this deck under
"Absence of feedpoint data" — NOT under that file's "Stability guarantee"
section, which covers the field set. Documented, not guaranteed; consumers
relied on it either way. That paragraph is rewritten here with a changelog
entry. A related consequence, measured: a deck with neither `EX` nor `FR` also
goes exit 0 -> exit 1, because validators run ahead of the FR check. A deck with
`EX` but no `FR` is unchanged at exit 0 and zero bytes — that is FND-084, still
open, and the rewrite no longer re-asserts the false general claim it corrects.
fnec diverges from nec2c here, conditionally. Measured first-hand against this
host's nec2c on a 21-segment dipole with FR and no EX: with an `XQ`, exit 0 and
a `CURRENTS AND LOCATION` table of `0.0000E+00`; with an `RP`, exit 0 with
`-nan` gains and `EFFICIENCY = -nan` rather than a floor value; with neither,
exit 0 and no currents or radiation section at all — it never executes. The
review reported this first and I took it second-hand into a code comment; the
comment now carries my own measurement instead, because a project that leans on
nec2c parity should not cite the oracle on someone else's word. (Getting there
needed a short path: nec2c aborts with "Input file name too long" on the
scratchpad, exiting 255 with no output — which reads exactly like "nec2c refuses
undriven decks", the opposite of the truth.)
Sabotage-verified in two halves:
- drop the check from `pre_solve_error`: the GUI, worker and `nec_solver`
tests fail — and every CLI test still PASSES, because of that second route.
That is why each frontend gets its own gate rather than trusting the CLI's;
- demote the CLI validator back to a warning: the CLI's own test fails.
Both routes must go before `deck_validator.rs` notices, which is the third
sabotage that was actually run.
Two test fixtures changed. The GUI's `a_sweep_with_no_solvable_points_...` used
a no-`EX` deck as its "prepares but fails to solve" case; that class now fails
at prepare, so the test FAILS loudly — it was replaced because it broke, not to
head off a silent pass, which is what an earlier draft said. A plane-wave
receive deck replaces it: a real class rather than a degenerate one, and it
still exercises the per-point failure this pins. The worker's test expected
`NoFeedpoint`, reached by solving to zeros and then failing to price; it now
gets `UnsupportedConfig` naming the missing drive, distinguishing "nothing
drives this" from "the drive is receive-only". Both already crossed the wire as
`unsupported_config`, so only `error_message` changed.
Ledger: FND-145 new and fixed. FND-112 closed by measurement, and credited to
#432 (`4e7170f`) which actually fixed it, not to this PR — re-measured at
max_ma=1.232144e0 and +2.147 dBi against the audit's 0e0 and -999.99 floor.
FND-108 re-scoped, its central grep claim being false now. FND-146/147/148/149
recorded, not fixed. FND-070/FND-084 explicitly NOT closed.
Also carries the `### Fixed` changelog entry for #448's FND-142, which that PR
landed without one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018p7FxX7QMWNJVNp9LbaLkB
dc0sk
added a commit
that referenced
this pull request
Sep 7, 2026
#449) With no `EX` card the Hallén system is homogeneous and solves to the zero vector. All four frontends solved it; they then split on what they did with the answer, and that split is why the check belongs in none of them: - the CLI printed a full `CURRENTS` table of exactly `0.000000e0`, a `RADIATION_PATTERN` of `-999.9900`, and `diag: ... abs_res=0.000000e0 rel_res=0.000000e0` — a flat response advertised as perfect convergence — and exited 0; - the GUI's currents and pattern views drew the same zeros as a coloured overlay, with no caveat at all; - the worker and `fnec_py` did refuse, but only after the wasted solve and with the wrong reason: "no driven feedpoint (EX voltage source) found in deck", which is the sentence a plane-wave receive deck gets — a feedpoint problem, not a missing-card one. The GUI had nothing to check with. The only no-`EX` test in the tree was a `NoExCardValidator` declared *inside* the CLI's own `main` (FND-145). The check now lives in `validate::pre_solve_error`, the gate all four frontends call. That placement is the fix, not an implementation detail: a guard in the Hallén RHS builders — the obvious spot, since that is where the zero vector is born — would have been INERT for `--solver pulse` and `--solver continuity`, which drive off `build_excitation` instead. Measured before: hallen, pulse, continuity and sinusoidal each exit 0 with 51 zero rows; only mpie refused. Measured after: all five exit 1 with zero bytes on stdout, swept by a test that takes the mode list from the binary's own usage line rather than typing it. The CLI's validator stays, promoted to error level and taking its sentence from the shared predicate. It runs before the geometry build, so it skips the FR and sweep-config parsing, the exec probe, `build_geometry` and the pairwise crossing scan — it does NOT save a matrix fill, as an earlier draft of this message claimed; `pre_solve_error` already refuses ahead of any assembly. It keeps EP-4's `DeckValidator` integration demonstrated. Its cost is recorded rather than glossed: the CLI now has two independent routes to this refusal, so the CLI's own tests cannot discriminate a regression in the shared one. BREAKING, deliberately, and stated more carefully than the first draft did. `docs/json-output-schema.md` documented `[]` and exit 0 for this deck under "Absence of feedpoint data" — NOT under that file's "Stability guarantee" section, which covers the field set. Documented, not guaranteed; consumers relied on it either way. That paragraph is rewritten here with a changelog entry. A related consequence, measured: a deck with neither `EX` nor `FR` also goes exit 0 -> exit 1, because validators run ahead of the FR check. A deck with `EX` but no `FR` is unchanged at exit 0 and zero bytes — that is FND-084, still open, and the rewrite no longer re-asserts the false general claim it corrects. fnec diverges from nec2c here, conditionally. Measured first-hand against this host's nec2c on a 21-segment dipole with FR and no EX: with an `XQ`, exit 0 and a `CURRENTS AND LOCATION` table of `0.0000E+00`; with an `RP`, exit 0 with `-nan` gains and `EFFICIENCY = -nan` rather than a floor value; with neither, exit 0 and no currents or radiation section at all — it never executes. The review reported this first and I took it second-hand into a code comment; the comment now carries my own measurement instead, because a project that leans on nec2c parity should not cite the oracle on someone else's word. (Getting there needed a short path: nec2c aborts with "Input file name too long" on the scratchpad, exiting 255 with no output — which reads exactly like "nec2c refuses undriven decks", the opposite of the truth.) Sabotage-verified in two halves: - drop the check from `pre_solve_error`: the GUI, worker and `nec_solver` tests fail — and every CLI test still PASSES, because of that second route. That is why each frontend gets its own gate rather than trusting the CLI's; - demote the CLI validator back to a warning: the CLI's own test fails. Both routes must go before `deck_validator.rs` notices, which is the third sabotage that was actually run. Two test fixtures changed. The GUI's `a_sweep_with_no_solvable_points_...` used a no-`EX` deck as its "prepares but fails to solve" case; that class now fails at prepare, so the test FAILS loudly — it was replaced because it broke, not to head off a silent pass, which is what an earlier draft said. A plane-wave receive deck replaces it: a real class rather than a degenerate one, and it still exercises the per-point failure this pins. The worker's test expected `NoFeedpoint`, reached by solving to zeros and then failing to price; it now gets `UnsupportedConfig` naming the missing drive, distinguishing "nothing drives this" from "the drive is receive-only". Both already crossed the wire as `unsupported_config`, so only `error_message` changed. Ledger: FND-145 new and fixed. FND-112 closed by measurement, and credited to #432 (`4e7170f`) which actually fixed it, not to this PR — re-measured at max_ma=1.232144e0 and +2.147 dBi against the audit's 0e0 and -999.99 floor. FND-108 re-scoped, its central grep claim being false now. FND-146/147/148/149 recorded, not fixed. FND-070/FND-084 explicitly NOT closed. Also carries the `### Fixed` changelog entry for #448's FND-142, which that PR landed without one. Claude-Session: https://claude.ai/code/session_018p7FxX7QMWNJVNp9LbaLkB Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
dc0sk
added a commit
that referenced
this pull request
Sep 8, 2026
Eighteen changes since v0.17.0: remediation of the 2026-08-28 whole-project audit (#432-#442), a documentation-honesty cluster (#443-#447), and two solver fixes (#448, #449). Both criticals and every high from that audit are closed. The ledger went 137 findings / 35 open -> 149 / 33 open, zero critical, zero high; it grew because fixing things found things. Versions: workspace 0.17.0 -> 0.18.0, fnec_py 0.8.0 -> 0.9.0 in both its `Cargo.toml` and its `pyproject.toml` (the lockstep every release since v0.14.0 has used). Both lockfiles refreshed, SBOM regenerated and committed. **The changelog covered 2 of the 18 PRs.** `[Unreleased]` held entries for #448 and #449 only; the entire audit-remediation cluster -- both criticals among it -- had merged without one. Keep a Changelog's own warning is that a partial record "can be as dangerous as not having a changelog", because it reads as complete. Entries written for all eighteen and each verified cited by number, which is a check worth keeping: `for n in 432..449; do grep -q "#$n" docs/changelog.md; done`. Breaking, and the release notes carry a migration for each: a deck with no `EX` is refused (stdout now empty rather than `[]`, exit 1 rather than 0); a deck with neither `EX` nor `FR` likewise; `GM` decks change geometry, because fnec had been losing wires in silence against NEC-2 semantics; and `EX 4` impedances shift, since a current source is now the unit-voltage solve rescaled exactly. Claims swept rather than carried forward. Two were wrong and are corrected: - the plane-wave split/whole agreement was quoted at 8.1e-12, which came from a different incidence angle; re-measured at this commit on the matched pair it is 1.1e-12, against the test's 1e-9 gate; - "1655 items checked" by the doc-attachment gate was true at #441 and is 1681 today, so both documents now date the figure instead of letting a stale number read as current. Re-measured at this commit: no-`EX` exits 1 with 0 bytes on stdout in both text and JSON; `EX`-but-no-`FR` still exits 0 with 0 bytes (FND-084, open, and deliberately untouched); neither-`EX`-nor-`FR` exits 1; the oversized `RP` is refused; corpus 50 cases; 1093 tests; ledger 149/33. No dependency changed between v0.17.0 and v0.18.0: 526 packages before and after, and the only entries that moved are the nine workspace members' own versions. Established by set difference over `(name, versionInfo)` pairs -- not by hashing the sets, which is randomised per process and fooled an earlier release. Removed-feature sweep for the one `### Removed` entry: `FNEC_ACCEL_STUB_GPU` survives only in the retirement records that document its removal and in a schema note that calls it retired -- no live advertising. Gate: scripts/check-all.sh, EXIT=0, all 15 gates passed. The version-consistency check was sabotage-verified once for this repo: `check-binding-version.py` passes at 0.9.0/0.9.0, fails naming both files when pyproject alone says 0.9.1, and passes again restored. Claude-Session: https://claude.ai/code/session_018p7FxX7QMWNJVNp9LbaLkB Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
What
build_planewave_hallengrouped segments by rawGWcard while its delta-gap siblingbuild_hallen_rhsgrouped by merged conductor — and whilesolve_hallen_planewave, which consumes what it builds, was already being handed the merged list by its caller. Builder and solver disagreed about what a wire is.So a straight wire written as two collinear
GWcards was refused outright:…though the same geometry driven by
EX 0orEX 4solves. That is FND-142, the receive twin of FND-140.The fix
One line: the builder takes
merge_collinear_wire_endpointsinstead ofwire_endpoints_from_segs. The merged list drives all three uses at once — the junction test, the segment grouping, and the along-wire coordinate. On geometry with no collinear split the merge is a documented strict no-op, so no deck that solved before changes.Evidence
Gated by an equality, not a similarity. The pair is segmented identically (25 + 25 against 50 over the same span), so every segment midpoint coincides and nothing differs but the card boundary:
Sabotage-verified in two halves, because this one-line change does two things:
GWgrouping restoredThe second is the informative one: with the old grouping the answer is 100% wrong, which is exactly what the blanket refusal had been protecting against — and why the coordinate half matters as much as the gate half.
A genuine bent junction is still refused, pinned by its own test.
Second commit
chore: silence needless_late_init— four identical endpoint-resolution blocks trip clippy 1.98. CI pins 1.97.1 and does not see them, but.githooks/pre-commitruns clippy-D warningsagainst the installed toolchain, so on a 1.98 host every commit here needs--no-verify. A gate that must be bypassed to commit is not a gate. Mechanical, no behaviour change.🤖 Generated with Claude Code
https://claude.ai/code/session_018p7FxX7QMWNJVNp9LbaLkB