Repository navigation
Name the standard errors ttsfm() could not compute, instead of saying nothing (A53 item 2) - #63
Merged
Merged
Conversation
… nothing On a TTHN fit whose Hessian is indefinite the negative-variance coordinates lose their standard error -- correctly, since none exists for them there -- but the fit reported NaN in those cells and warned about nothing, after minutes of work. Measured on main at 52c3bff, macOS, A53's design: seed secs logLik min eig max eig neg diag(H^-1) SEs warnings 3049 395 -214.2819 -0.00209627 24862 3 of 6 3 of 6 NONE 3002 428 -190.4871 -0.00311802 30578.7 2 of 6 4 of 6 NONE Seed 3002 reproduces the row A53 recorded to every printed digit, so that measurement now holds on a second platform and a later commit. .se_missing_report() names those parameters in the fit's own reported names and reports the Hessian's extreme eigenvalues. TTNE and TTHN warn with it and record $se_unresolved, which print() and summary() report too, since a warning does not survive saveRDS() -- the same reason .sfa_report_boundary() exists. The message does not pick a reading: an indefinite Hessian here is EITHER a point that is not a maximum OR a boundary solution, where the full-space Hessian need not be positive definite. A53 item 6 is explicit that 3002 (sigma_v collapsed onto its bound) and 3049 (interior, unpinned) are different phenomena, so the warning states both and points at tt_boundary_report() and vcov(type = "bhhh"). Three wordings -- indefinite, near-singular, and "could not be read" -- so no claim is made about eigenvalues never computed. solve() on a singular Hessian is now caught; it was unguarded and discarded a fit that had already taken minutes. psfm() and selsfm_kts() already guard theirs. A53 items 3 and 4, which Hessian estimator each path reports, are untouched: they change the standard errors existing fits report. horserace/FUNCTIONALITY_GAPS.md A53. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
davidhbernstein
added a commit
that referenced
this pull request
Oct 23, 2026
Two conflicts, both routine, and one of them worth a note. NEWS.md: both branches added an entry under the top "## Bug fixes". Both kept, A53's first since it is already on main. R/matrix_utils.R: both branches inserted their new helper at the same place -- between .log_nr_g's three-line comment and .log_nr_g itself -- so each had silently orphaned that comment onto its own function, and main carries the same wart from #63. Resolved by putting .sfa_se_from_var() and .se_missing_report() above the comment and re-attaching it to .log_nr_g, which this merge therefore also fixes on main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Oct 23, 2026
davidhbernstein
added a commit
that referenced
this pull request
Oct 27, 2026
… (A65) (#66) * Name the standard errors the other five entry points could not compute (A65) zsfm(), lcsfm(), copsfm(), ivsfm() and selsfm() each took their standard errors as suppressWarnings(sqrt(diag(solve(opt$hessian)))). On an indefinite Hessian the negative-variance coordinates come back NaN, and suppressWarnings() discarded R's own "NaNs produced" -- the only signal there was. Nothing set a flag, and the convergence code read 0. Reproduced on zsfm("ZISF"), data_gen_cs(N = 25, rand = 8, sig_u = 1, sig_v = .3, cons = .5, beta1 = .5, beta2 = .5, mu = .5, a = 5), 0.5s to fit: eigenvalues 3.48e6 ... -1474, two of six standard errors NaN -- one of them the slope x2, whose true value is 0.5 -- conv 0, no warnings, no flags, and print()/summary() said nothing beyond printing NaN. All five now call .se_missing_report(), the model-agnostic helper #63 added for ttsfm(), on the REPORTED standard errors and names, so copsfm()'s and ivsfm()'s delta-method scales carry through correctly. $se_unresolved is appended to the results list rather than re-listed against its name vector. solve() is wrapped too: the sum(colMeans(hessian)) == 0 test caught only an all-zero Hessian, so a singular but non-zero one errored and discarded a finished fit. The test file fails on main (3 failures: no warning, no flag) and passes 17 of 17 here, and includes a false-positive guard -- the same design at N = 300 must stay silent with no flag. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Derive A65's expectations from the fit instead of pinning one platform's The test hard-coded the landing point of a single fit on macOS arm64: the names of the two parameters that lose a standard error, and the count 2 of 6. Which point the optimizer stops at, and the sign of a small eigenvalue there, is BLAS-dependent -- exactly what made #62 fail on all five CI platforms a few days ago, and exactly what notes from that round said not to do again. Every expectation is now derived from the fit in front of it: the reported parameters must be the ones whose standard error is non-finite, n_missing must be that count, n_par the length of the vector, and "indefinite" must hold iff a negative eigenvalue was read. The regime is a skip condition, not an assertion, in both directions -- the first test skips where every standard error is finite, since there is then nothing for the report to say, and the false-positive guard skips where the N = 300 fit is not healthy, since its claim is about a healthy fit. expect_silent() is gone too: it demanded total silence from a fit, which is a stronger claim than the test needs. 12 of 12 pass against the branch build and 2 of 4 fail against main, so it still discriminates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * NEWS: conv 0 on the A65 reproduction is machine-specific The entry reported "conv 0, and zero warnings" as the headline. The zero warnings is the defect and holds; conv 0 does not travel. On all five CI platforms the same fit reports convergence 52, so there the convergence code carries a signal and only the missing standard errors are unexplained. On this Mac it reports 0, which is the fully silent case. The CI run also establishes something the local run could not: the warning fired on macOS, ubuntu devel, ubuntu release and windows -- the only failing expectation there was the convergence code -- so the indefinite-Hessian regime is reached on four of the five platforms. ubuntu oldrel-1 does not reach it, which is why the rewritten test skips rather than asserts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Extend A65 to psfm(), which has a seventh site and a third spelling psfm() carries the same two defects at seven Hessian-diagonal sites, covering GTRE and TRE (which share one), GTRE_Z, TRE_Z, GTRE_FML, TFE_WMLE, TFE and FD. Four of the seven left the inverse unguarded, so a singular but non-zero Hessian discarded a finished PANEL fit -- the most expensive kind this package produces. Their guard tests any(hessian == 0), which catches a zero ENTRY rather than singularity. The count recorded two days ago was six, and it was wrong. FD spells the same quantity sqrt(diag(qr.solve(opt$hessian))) -- qr.solve, and unclamped -- which makes it invisible to A64's sweep, to A65's first pass, and to the .sfa_se_from_var(diag(solve(...))) pattern that found the other six. It surfaced only because a replace-all asserted 4 occurrences and got 5; the sweep was then redone on $hessian itself, spelling-agnostically, which is what should have happened first. Deliberately excluded after that sweep: psfm()'s two other Hessian inversions (the sandwich H_inv, the delta-method vc), which already guard solve() and take their standard errors through a Jacobian, so the report's eigenvalue narrative does not map onto them as directly. $se_unresolved is appended at eight assemblies -- GTRE and TRE share a site but keep separate results lists -- and the insertion was scoped to each enclosing if (model_name ...) block so it could not leak into GTRE_SEQ1/SSFE/PL80/GTRE_SEQ2, where .sm does not exist. Two instances came out of the existing suite, which is why the local warning count moved: * psfm("FD") at test-psfm.R:23 is GENUINE -- 3 of 5 standard errors missing, smallest eigenvalue -0.00119 against a largest of 17400. So the site every sweep missed is reachable in the package's own tests, and had been reporting three unexplained NAs there all along. * lcsfm("LCM_CN") at test-a52-opt-shape.R:91 was a FALSE POSITIVE of mine. That call passes optHessian = FALSE, so there is no Hessian by request; LCM_CN is the one branch whose standard-error test does not mention optHessian, and so the one place the guard was missing. Now guarded. Test uses A64's own design, psfm("GTRE_FML") on data_gen_p(t = 5, N = 40, rand = 1, ...) -- 3 seconds, three of seven parameters with a negative variance estimate -- gated on the regime from the start, since A64's end-to-end case had to be gated after all five CI platforms disagreed with this Mac. Full local suite on the final code: pass 5556, fail 0, warn 49, skip 9, against a 48-warning baseline -- the one addition being the genuine FD report. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- 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.
ttsfm()now names the standard errors it could not computeOn a fit whose Hessian is indefinite, the negative-variance coordinates lose
their standard error. That part is correct -- no standard error exists for them
at that point -- but the fit reported
NaNin those cells and said nothingat all, after minutes of work.
Measured on
main(52c3bff), macOS, reading each fit's own stored Hessian.Design is A53's:
data_gen_cs(N = 300, rand = <seed>, sig_u = .5, sig_v = .2, cons = 5, beta1 = 1.5, beta2 = 2, a = 5, mu = .1),model_name = "TTHN".Seed 3002 reproduces the row A53 already recorded (
-190.4871,-0.003118,4/6) to every printed digit, so that measurement now holds on a second platform
and a later commit. The
NaNcount equals the negative-diagonal countparameter for parameter.
The change
.se_missing_report()(R/matrix_utils.R) names the parameters whose standarderror is missing, in the fit's own reported names, and reports the Hessian's
smallest and largest eigenvalue.
ttsfm()'s"TTNE"and"TTHN"brancheswarn with it and record it as
$se_unresolved, whichprint()andsummary()also report -- a warning does not survivesaveRDS(), which is thesame reason
.sfa_report_boundary()exists.The message does not pick a reading. An indefinite Hessian here is either
a point that is not a maximum or a boundary solution, where the Hessian in the
full parameter space need not be positive definite. A53's item 6 is explicit
that seeds 3002 and 3049 are not the same phenomenon -- 3002's
sigma_vhascollapsed onto its bound, 3049's is interior and merely unpinned -- so the
warning states both and points at
tt_boundary_report()for the boundary caseand
vcov(type = "bhhh")for a covariance that needs no Hessian. Threewordings: indefinite, positive-definite-but-near-singular, and "the Hessian
could not be read", the last so that no claim is made about eigenvalues that
were never computed.
solve()on a singular Hessian is now caught. It was unguarded, so anexactly singular Hessian threw away a fit that had already taken minutes;
psfm()andselsfm_kts()already guarded theirs. It now reports the standarderrors as missing, which the new warning then explains.
What this does NOT do
A53's items 3 and 4 -- making the substituted and unsubstituted paths report
the same Hessian estimator -- are untouched and still need David's call,
because they change the standard errors existing fits report. This PR only
makes the fit say what it could not compute. Item 5 is refuted and recorded as
such (R's
sqrt()is element-wise, so there was never anything to implement).The helper is model-agnostic; the other entry points that return
NaNstandard errors silently could call it next, and that is deliberately not in
this PR.
Tests
tests/testthat/test-a53-missing-se-report.R, 30 assertions. The mechanism isdriven on hand-built Hessians rather than through a fit, because the fits that
reach this state sit on flat ridges -- which is exactly what does not reproduce
across platforms (the lesson from #54, #57 and #59). Covered: nothing reported
when every standard error is finite; the right parameters named with the right
eigenvalues; both readings present in the message; the unreadable-Hessian and
near-singular wordings; positional fallback labels; and the
print()NOTE froma constructed fit. The live reproduction is a seed-3002 fit behind
SFA_TEST_SLOW, since one TTHN fit is ~400s.Local suite
Branch build, macOS,
load_package = "installed",NOT_CRAN=true,TESTTHAT_CPUS=2: pass 5517, fail 0, warn 48, skip 9 (the skip is theSFA_TEST_SLOWfit).Worth noting against expectation: the warning count is unchanged from the
A64 branch's run of the same suite (also 48). Adding a warning did not add one
here, because no fit in the suite reaches an indefinite TTHN Hessian -- the
state needs ~400s fits on specific seeds. So this change adds no noise to the
existing suite. No same-invocation
mainbaseline was taken this round(the machine was at load 32 throughout), so 48 is reported, not compared --
though two independent branches agreeing at 48 is itself suggestive.
🤖 Generated with Claude Code