Skip to content

NTN: warn and flag when sigma_v collapses to the boundary - #56

Merged
davidhbernstein merged 3 commits into
mainfrom
fix/sigma-v-boundary-warning
Oct 2, 2026
Merged

davidhbernstein merged 3 commits into
mainfrom
fix/sigma-v-boundary-warning

Conversation

@davidhbernstein

Copy link
Copy Markdown
Owner

Closes the actionable half of a defect found while scoping gap A56. Not reported externally.

The symptom

sfm(model_name = "NTN") can return lambda = 972468, six NA standard errors, and no message of any kind.

tHN has warned and set $thn_sigma_u_at_bound for the mirror case — sigma_u collapsing onto zero — since 1.2.0. The other component was never covered.

The mechanism: the fit is correct, the silence is the defect

On such a sample NTN has no interior maximum in lambda. The profile log-likelihood rises monotonically toward a finite limit as lambda grows without bound. Measured by continuation from the reported solution (rand = 2):

lambda profile logLik
1e1 −190.3374
1e2 −188.5223
1e5 −188.1011
9.72e5 — what sfm() returns −188.1003
1e9 … 1e15 −188.1001876 (flat to 7 dp)

So the boundary is the maximum likelihood estimate and running there is right. But the likelihood is flat in lambda at that point, so the Hessian is singular and no finite standard error for lambda exists — which is why all six come back NA. That now gets said, instead of being left for the user to infer from missing numbers.

A methodological note, since it changed my own conclusion: a fixed-start profile reports −209.13 at lambda = 1e6 where the truth is −188.10, because it fails to find sfm()'s own point. That reads as "the optimizer ran away" and is the opposite of the right answer. Taking the lambda = Inf limit analytically is also invalid — dropping the log Phi term unbalances the -n log Phi(mu/sigma) normalising constant, leaving something unbounded as sigma -> 0 (it ran to sigma = 1.5e-149). Continuation from the reported solution is what works, and needs no analytic limit.

Two things verified before writing any code

Scope. I scanned the models that report a lambda/sigma reparameterization rather than assuming this was general. Over 8 seeds, only NTN runs away (3 of 8). NHN shares the parameterization and never does; NE, NR, NU and NG report sigv/sigu directly. So this is model-specific, matching the existing thn_* and NGB2 warnings in the same file rather than inventing a package-wide check from one model's evidence.

The threshold is not tuned. The criterion is sigma_v/sigma < 1e-3, the same scale-free ratio thn_sigma_u_at_bound uses. Over 12 samples it separates the regimes by five orders of magnitude:

sigma_v/sigma standard errors
boundary fits (rand 2, 4, 5, 11, 12) 4.9e−08 … 1.3e−06 0 of 6
interior fits (rand 1, 3, 6–10) 0.126 … 0.442 6 of 6

The threshold falls in empty space. The second test block asserts that clearance explicitly, so a change which narrows the gap fails there rather than quietly making the flag sensitive to the cut.

Implementation note

The flag is appended to the results list rather than folded into it. NTN shares its results block with seven other models, and re-listing twelve values against twelve names to add one field is how a name/value misalignment gets introduced. $ntn_sigma_v_at_bound is always present for NTN and always logical, so callers need not test for its existence.

Evidence

  • Fails on main, passes here — 12 failures against c94721a, 0 on this branch, 40 assertions. main was pinned with git archive c94721a rather than a worktree, after a worktree silently drifted to an older commit twice — which would have made "fails on main" a claim about the wrong tree.
  • tools::checkDocFiles() clean with the new \value item; 50 Rd files parse. No \usage change.
  • Full suite (installed build, NOT_CRAN=true): 0 failures, 5295 passing, 43 warnings, 8 skips, 39.5 min. The warning count is unchanged from main's, so the new warning fires in no existing test.
  • The four plm::pdata.frame: already a pdata.frame errors documented as expected locally did not occur.
  • CI on five platforms.

Deliberately not in this PR

The same scan found a second and more serious NTN failure: 3 of 12 fits stop 0.27–1.67 log-likelihood units below the supremum while reporting conv 0 and a full set of standard errors — confident output at a point that is not the MLE. That needs a multistart over lambda (the shape of fix #40 applied to NNAK), is a behavioural change rather than a diagnostic, and belongs in its own PR. Also untouched: NW, whose runaway is sigu -> 4e-5 with shape k -> 0.13, a different boundary that I have not profiled.

🤖 Generated with Claude Code

sfm(model_name = "NTN") could return lambda = 972468, six NA standard
errors, and no message of any kind. tHN has warned and set
$thn_sigma_u_at_bound for the MIRROR case -- sigma_u collapsing onto
zero -- since 1.2.0. The other component was never covered.

Why the fit is right and the silence is the defect. On such a sample NTN
has no interior maximum in lambda: the profile log-likelihood rises
monotonically toward a finite limit as lambda grows without bound.
Measured by continuation from the reported solution, rand = 2:

    lambda      profile logLik
    1e1         -190.3374
    1e2         -188.5223
    1e5         -188.1011
    9.72e5      -188.1003   <- what sfm() returns
    1e9..1e15   -188.1001876  (flat to 7 dp)

So the boundary IS the maximum likelihood estimate and running there is
correct. But the likelihood is flat in lambda at that point, so the
Hessian is singular and no finite standard error for lambda exists --
which is why all six come back NA. Reported now rather than inferred by
the user from missing numbers.

Two things verified before writing any code.

SCOPE. Scanned the models that report a lambda/sigma reparameterization
before assuming this was general: over 8 seeds only NTN runs away (3 of
8). NHN shares the parameterization and never does; NE, NR, NU and NG
report sigv/sigu directly. So this is model-specific, matching the
existing thn_* and NGB2 warnings in the same file rather than inventing
a package-wide check on one model's evidence.

THE THRESHOLD IS NOT TUNED. The criterion is sigma_v/sigma < 1e-3, the
same scale-free ratio thn_sigma_u_at_bound uses. Over 12 samples it
separates the regimes by five orders of magnitude: boundary fits at
4.9e-08 to 1.3e-06, interior fits at 0.126 to 0.442. The threshold falls
in empty space. test-ntn-sigma-v-bound.R asserts that clearance
explicitly, so a change which narrows the gap fails there rather than
quietly making the flag sensitive to the cut.

The flag is APPENDED to the results list, not folded into it. NTN shares
its results block with seven other models, and re-listing twelve values
against twelve names to add one field is how a name/value misalignment
gets introduced.

Verified to fail on main and pass here: 12 failures against c94721a,
0 on this branch, 40 assertions. main was pinned with `git archive
c94721a` rather than a worktree, after a worktree silently drifted to an
older commit twice -- which would have made "fails on main" a claim about
the wrong tree.

tools::checkDocFiles() clean with the new \value item. Full suite on this
branch, installed build: 0 failures, 5295 passing, 43 warnings, 8 skips,
39.5 min. The warning count is unchanged from main's, so the new warning
does not fire in any existing test. The four plm::pdata.frame errors
documented as expected locally did not occur.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@davidhbernstein
davidhbernstein enabled auto-merge (squash) October 2, 2026 10:33
davidhbernstein and others added 2 commits October 2, 2026 06:36
NEWS.md only, and the expected conflict: #54 and this branch both insert at
the top of `## Bug fixes`. Resolved by keeping BOTH entries with a blank
line between, main's merged entry first.

Verified after resolving: no conflict markers, both entries present once,
#54's bhhh_ok/bhhh_hint code and its test present, and this branch's
ntn_sigma_v_at_bound and its test present.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CLAUDE.md: notes/code_history/ holds the long explanatory blocks, and R/
keeps one or two lines saying what the code does. I had put a 16-line
block above ntn_sigma_v_at_bound -- the continuation profile, the
threshold separation, the two measurement traps -- which is exactly what
that rule exists to prevent.

R/sfm.R now carries two lines and a pointer. The full account is a new
section in notes/code_history/sfm.md, anchored at the assignment:
why the boundary fit is correct, the lambda profile table, why 1e-3 is
not a tuned cut, and the two ways of measuring it that gave the WRONG
answer first (a fixed-start profile, and taking the limit analytically).

That note had to be written: the pointer I first used named a file that
did not discuss this at all -- the detail was in FUNCTIONALITY_GAPS.md,
not in code_history -- so the comment would have pointed at nothing.

Comment-only change. The full suite was green on this branch before it
(0 failures, 5312 passing, 39.1 min) and test-ntn-sigma-v-bound.R
re-run after it is still 40/40.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@davidhbernstein

Copy link
Copy Markdown
Owner Author

Updated for the NEWS.md conflict with #54, and for the comment-length rule.

Conflict resolved by keeping both entries with a blank line between, #54's merged entry first. Verified by content rather than by a clean merge: no conflict markers, each entry present exactly once, and both sides' code and tests actually in the tree — #54's bhhh_ok/bhhh_hint and test-vcov-bhhh-advice.R, and this branch's ntn_sigma_v_at_bound and test-ntn-sigma-v-bound.R.

Long comment moved out of R/. CLAUDE.md puts the long explanatory blocks in notes/code_history/ and keeps one or two lines in R/. I had written a 16-line block above the assignment — the profile table, the threshold separation, the measurement traps — which is what that rule exists to prevent. R/sfm.R now reads:

## sigma_v on its boundary: lambda -> Inf is the supremum, so the fit is
## right but flat and has no standard errors. notes/code_history/sfm.md.
ntn_sigma_v_at_bound <- isTRUE(sig_v / sqrt(sig_u^2 + sig_v^2) < 1e-3)

The full account is now a section in notes/code_history/sfm.md (outside the package, so it reaches neither the tarball nor this repo). Worth noting the pointer I first wrote was false — that detail lived in FUNCTIONALITY_GAPS.md, not in code-history — so the note had to be written rather than just referenced.

Evidence after the merge: full suite 0 failures, 5312 passing, 43 warnings, 8 skips, 39.1 min — 5255 from main plus 17 from #54's test and 40 from this one. test-ntn-sigma-v-bound.R re-run after the comment trim is still 40/40.

Two of my long blocks are already on main (data.processing.R 20 lines, sfm.R 9 lines, ~6 in sfareg_methods.R). Those get their own PR once this lands — mixing comment cleanup into a bug fix, or creating a self-conflict on sfm.R, both seemed worse than waiting.


Generated by Claude Code

@davidhbernstein
davidhbernstein merged commit f8b888e into main Oct 2, 2026
5 checks passed
davidhbernstein added a commit that referenced this pull request Oct 5, 2026
Windows CI failed with six failures, none of them in the #55 test: three in
#56's test-ntn-sigma-v-bound.R (a boundary seed no longer flagged, no warning,
standard errors no longer all NA), one at its line 81 (an INTERIOR seed moved
the other way), and two in test-vcov-bhhh-advice.R, whose fixture reported
lambda = 378.5 where it needs > 1e4. Four other platforms passed.

Not a better or worse optimum. On the twelve seeds those tests use, the
log-likelihood is unchanged to 1e-4 while lambda moves by factors of 4 to 18
(seed 5: 1.5e6 -> 2.7e7). That is A59's ridge: lambda -> Inf is a supremum,
the likelihood is flat along it, and lambda is NOT IDENTIFIED. The reported
value is wherever the gradient test stops -- Windows at 378.5, macOS at 9.7e5.

The perturbation was arithmetic, not algebra. On those samples mu > 0, so
bb > 0 and every observation already took the direct branch -- but that branch
was not bit-identical to what it replaced:

  -(1 / (2 * sig^2)) * (-eps - mu)^2    before: reciprocal, then multiply
  -(eps + mu)^2 / (2 * sig^2)           the rewrite: divide

Those differ in the last bit, and the sum was re-associated too. One ulp is
enough to move an unidentified lambda by a factor of 20.

So the tilt is now used ONLY where the cancellation is real -- both arguments
below -1e3, the switch .log_phi_tilt() itself uses, below which the direct
form is accurate to 1e-11 (the bound is max(aa, bb)^2 / 2 * 1e-16). The
non-cancelling branch is the pre-#55 expression verbatim, association order
included.

All twelve seeds are then bit-identical to main, and the result is better on
every axis than the loose guard:

                              main   loose   tight
  on the floor AND above OLS     9       0       0
  below the nested NHN         200      13      14   (worst 0.39 -> 0.31)
  LOWER under one likelihood     -      27       2
  total log-likelihood           -  +12123  +12240
  bit-identical to main          -       -    2413 of 2700

The O(lam^2) slope ladder now straddles the switch and is unaffected: 1.9915,
1.9708, 1.9963 against a 2 +- 0.16 budget. The #55 test still fails 23
assertions on main f8b888e.

Full suite: FAIL 0 | WARN 43 | SKIP 8 | PASS 5417, 31 min.

Reported by a peer session from the CI log. My earlier claim that #56's test
"passes unchanged under #57" was made on macOS evidence alone and stated
without naming a platform; it was false on Windows.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant