backup: 2026-10-05 09:48
This commit is contained in:
@@ -58,3 +58,136 @@ ISJ fails when a class has too few samples in a split (~<20). Affects: cleveland
|
||||
- Patience counter does not increment (early stopping cannot fire)
|
||||
|
||||
All keel configs use =grace_period=50=. Prevents noisy early-epoch models from being selected as best.
|
||||
|
||||
** Naming fix: =_kernelized_roll_fpr_internal='s =fpr= param renamed =cdf_target= (2026-09-10)
|
||||
|
||||
Mentor-audit finding (screenshots, no direct cite): =kernelized_roll_fpr(fpr, ...)= calls
|
||||
=partial(_kernelized_roll_fpr_internal, fpr=1 - fpr, ...)= — the INTERNAL function's =fpr=
|
||||
parameter actually holds =1-fpr= (the CDF target, thesis's =1-α=), not the FPR itself, the
|
||||
opposite of what its name implies. Logic was always correct (verified independently, see
|
||||
next entry); only the name was misleading. Fixed by renaming the internal parameter (and
|
||||
the keyword in the =partial(...)= call) to =cdf_target= — user's preferred name over
|
||||
=alpha= (avoids clashing with the thesis's own use of α to mean the RAW target rate, not
|
||||
its complement). The PUBLIC =kernelized_roll_fpr(fpr, ...)= parameter is untouched — it's
|
||||
correctly named; the complement happens only when forwarding internally. No callers use
|
||||
=fpr== as a keyword arg anywhere in the codebase, so the rename is fully self-contained.
|
||||
Mirrored the identical rename in the thesis's own code listing
|
||||
(=content/appendixs/appendix_roll_code.tex=, which reproduces this function verbatim) so
|
||||
the two don't drift apart.
|
||||
|
||||
** Verified correct (not a bug): =_kernelized_roll_tpr_internal='s =1-yh= and un-complemented =tpr= (2026-09-10)
|
||||
|
||||
While investigating the =fpr=/=cdf_target= naming issue above, also audited two
|
||||
look-alike oddities in the neighboring =_kernelized_roll_tpr_internal= — both checked out
|
||||
as correct, for non-obvious reasons worth remembering before anyone "fixes" them later:
|
||||
|
||||
1. *=1 - yh= instead of =-yh=* for the score transform (thesis's stated general rule says
|
||||
"negate scores", i.e. =-f_theta(x)=). Directly compared =1-yh= vs =-yh= as inputs:
|
||||
identical loss and gradients to ~3e-8 (float noise). Reason: =1-yh = -(yh-1)=, i.e.
|
||||
negation plus a constant shift, and =KernelizedROLLoss= is translation-invariant in its
|
||||
scores (same property underlying the thesis's gradient-balance proof) — so the shift is
|
||||
silently absorbed and does nothing. Added a one-line code comment at the call site
|
||||
(roll.py, right above the =KernelizedROLLoss.apply= call in
|
||||
=_kernelized_roll_tpr_internal=) explaining this, so it doesn't look like a stray bug to
|
||||
a future reader — this comment is the only thing that changed; the surrounding logic
|
||||
was intentionally left as-is.
|
||||
2. *=kernelized_roll_tpr(tpr)= passes =tpr= straight through with no complement*, while
|
||||
=kernelized_roll_fpr(fpr)= complements (=1-fpr=). Looks asymmetric, but is required by
|
||||
the math: reducing FPR@TPR(β) to the TPR@FPR machinery requires complementing the
|
||||
target rate to =(1-β)= first (per the thesis's own reduction rule), and separately,
|
||||
evaluating TPR@FPR at any rate =γ= uses CDF-target =1-γ= internally — chain the two
|
||||
complements: =1-(1-β)=β=. They cancel exactly, so the raw =tpr= value reaching the
|
||||
internal CDF-target IS =β= correctly, no complement needed. The apparent asymmetry
|
||||
between the two wrappers is expected, not a bug.
|
||||
|
||||
Closed the loop with an independent numerical check (not just internal
|
||||
self-consistency): hand-computed =1 - F0_hat(F1_hat^{-1}(1-beta))= (thesis's
|
||||
eq:roll-fpr-at-tpr) completely from scratch in a throwaway script — same normalization,
|
||||
same KDEpy bandwidth selector, same sigmoid kernel, same bisection, but applied directly
|
||||
to the ORIGINAL untransformed (yh, y) with no negation/swap trick at all. Result matched
|
||||
=_kernelized_roll_tpr_internal='s actual output to ~3e-5 (bisection tolerance noise).
|
||||
Confirms the function exactly reproduces the paper's formula. No code fix needed for
|
||||
either point beyond the one comment above.
|
||||
|
||||
** MAJOR: appendix_roll_code.tex was stale relative to impl/src/roll.py (2026-09-10)
|
||||
|
||||
User asked directly: "did we ever update the code [listing] after we changed [the real
|
||||
code]... is the code in the thesis matching actual code?" Answer at the time: NO. Did a
|
||||
full AST-based function-by-function diff (Python =ast= module, not just eyeballing) of
|
||||
every function in appendix_roll_code.tex against impl/src/roll.py. Found:
|
||||
|
||||
Serious (pre-fix or actively-buggy code shown as current):
|
||||
- =_icdf= / =_icdf_batch=: listing showed the OLD signatures entirely (single =v=/=scores=
|
||||
instead of =v0=/=scores_neg=/=v1=/=scores_pos=, old fixed =+-10/v= bracket margin instead
|
||||
of the exact =h0*log(alpha/(1-alpha))= bracket, old "stop on CDF residual" criterion
|
||||
instead of the certified-loss-interval one) -- none of the bisection-bracket /
|
||||
stopping-criterion audit fixes from earlier this session had been mirrored here.
|
||||
=forward()='s calls to these were correspondingly stale (3-arg, not 5-arg).
|
||||
- =backward()=: showed the OLD, numerically-vulnerable =kernel = s_neg*(1-s_neg)= gradient
|
||||
formula -- the EXACT formula independently found to have a live bug (collapses to an
|
||||
all-zero gradient when scores saturate) and replaced with the log-space
|
||||
=_log_sigma_prime=/softmax version earlier this session. The listing never got that fix,
|
||||
so it directly contradicted appendix_kde_grad.tex (which documents the fix as done) --
|
||||
a self-contradiction within the thesis between two appendices.
|
||||
- =_log_sigma_prime= itself was MISSING from the listing entirely (not just stale) --
|
||||
=backward()= as shown couldn't even have called it.
|
||||
|
||||
Minor (comments from this session's smaller fixes not mirrored):
|
||||
- =_torch_mean_std=: listing still showed =torch.std(yh)= (Bessel-corrected), not the
|
||||
=correction=0= MLE fix from earlier the same day (see next entry).
|
||||
- =_kernelized_roll_tpr_internal=: missing the =1-yh= translation-invariance comment.
|
||||
|
||||
Also softened the appendix's opening claim from "reproduced verbatim" to "reproduced ...
|
||||
reformatted for print width ... logic is unchanged" -- it was never literally verbatim
|
||||
(unicode sigma/tau stripped to ASCII, citation tags like =[eq:sigmoid-kernel]= dropped from
|
||||
docstrings, lines rewrapped for column width), so "verbatim" was itself a small overclaim.
|
||||
|
||||
Fixed all of the above directly in appendix_roll_code.tex: ported the current =_icdf=/
|
||||
=_icdf_batch=/=forward=/=backward= logic across (reformatted to the listing's existing
|
||||
ASCII/wrapped style, not raw-pasted), added the missing =_log_sigma_prime= method right
|
||||
after =_sigma_prime= (matching live file ordering), and added the two missing comments.
|
||||
|
||||
VERIFICATION METHOD (worth remembering for next time this comes up): manual reading isn't
|
||||
reliable for this kind of check -- used Python's own =ast= module to extract every
|
||||
function's source from both files and diff them programmatically. First pass (raw source
|
||||
diff) caught line-wrapping noise mixed in with real differences; had to write a stricter
|
||||
second pass that parses both into ASTs, strips docstrings/comments via =ast.unparse=, and
|
||||
diffs the RESULT -- that gave a clean "0 functions differ in actual executable logic"
|
||||
confirmation across all 21 functions in the listing. Recommend this exact
|
||||
=ast.parse=+=ast.unparse=-diff technique any time "does the thesis's code listing match
|
||||
the real implementation" needs checking again -- eyeballing a big code appendix against a
|
||||
live file is exactly how this drift went unnoticed for multiple fix-rounds in the first
|
||||
place.
|
||||
|
||||
Rebuilt thesis clean (nix build exit 0) after the fix, confirmed no new undefined
|
||||
references introduced.
|
||||
|
||||
** Fixed: Gaussian ROLL =_torch_mean_std= used Bessel-corrected std, not MLE (2026-09-10)
|
||||
|
||||
Mentor-audit finding: Appendix A (appendix_gauss_grad.tex) states explicitly "The MLE
|
||||
parameters are: ... σ_k² = (1/|B_k|)Σ(f_θ(x_j)-μ_k)²" — the biased, n-denominator MLE
|
||||
variance. But =_torch_mean_std= (roll.py, Gaussian ROLL helpers) used =torch.std(yh)=,
|
||||
whose default (=correction=1=) is Bessel-corrected (n-1 denominator) — NOT the MLE. Real
|
||||
documentation-vs-implementation mismatch, same family as the AOC-divisor issue, but lower
|
||||
stakes: verified =_torch_mean_std=/=_torch_normal_fit= are ONLY used by
|
||||
=_roll_loss_from_fpr_internal= (Gaussian ROLL), which is NOT among the loss configs used
|
||||
in any reported experiment (only =roll-aoc= and =roll-tpr90=, both KDE-based, are) — so
|
||||
fixing this required no re-running of any experiment.
|
||||
|
||||
Separately checked whether the ANALOGOUS =torch.var(yh, correction=1)= in KDE score
|
||||
normalization (=score_scale=, roll.py ~line 271) has the same problem: it does NOT.
|
||||
Thesis's own definition of the normalization constant (method.tex, sec:roll-numerical-
|
||||
stability) is deliberately generic — =\hat\sigma_\mathcal{B} = sqrt(Var(...))= with no
|
||||
n vs n-1 commitment anywhere, unlike Appendix A's explicit "MLE" language. Nothing textual
|
||||
is being contradicted there. Also, per the thesis's own prop:normalization-invariance,
|
||||
for Silverman's rule (scale-equivariant) the exact value of σ̂_B doesn't affect the loss
|
||||
or gradient AT ALL; for ISJ (already flagged elsewhere as "a heuristic"), the n vs n-1
|
||||
choice is only a sqrt(n/(n-1)) deviation (~2.6% at batch 20, <1% by batch 64) -- verified
|
||||
numerically. Left this one alone entirely, per user's explicit instruction to only fix
|
||||
the Gaussian ROLL side.
|
||||
|
||||
Fix applied: =torch.std(yh)= -> =torch.std(yh, correction=0)= in =_torch_mean_std=, with a
|
||||
one-line comment noting this matches Appendix A's MLE formula rather than torch.std's
|
||||
Bessel-corrected default. Verified: output now matches a hand-computed
|
||||
=sqrt(mean((x-mean(x))^2))= MLE formula exactly (=torch.allclose= true), and the full
|
||||
=roll_loss_from_fpr= forward+backward still runs cleanly end-to-end after the change.
|
||||
|
||||
Reference in New Issue
Block a user