Add FTRL-Proximal logistic regression (lr+ftrl / sparse-lr+ftrl) - #6
Conversation
The branch was reviewed as ready to merge with no Critical or Important defects, but four items needed closing first: 1. make-lr+ftrl / make-sparse-lr+ftrl asserted alpha/beta/lambda1/lambda2 before coercing them to single-float. A positive double like 1d-50 passed (assert (< 0.0 alpha)) against the double, then underflowed to exactly 0.0 once coerced -- alpha divides in every weight derivation, so the first train call filled weight/bias with NaN and signalled nothing. Both constructors now coerce first and assert against the coerced values. 2. The comment above ftrl-weight-of claimed the materialized WEIGHT cache is exact because w_i is a pure function of (z_i, n_i). It is also a function of the four setf-able meta-parameter slots; setting one post-construction desyncs the cache. Extended the comment to say so. 3. multiclass-ovr-sparse-lr+ftrl's comment claimed a meaningful L1 would zero the entire model on iris -- measurably false (non-zero counts 12/11/9/8 and accuracy 87.3/87.3/86.0/79.3% at lambda1 0/1/10/100). Replaced it with the real justification: lambda1=0.0 keeps this test about the multiclass wrapper's name-resolution, not regularization. 4. lr+ftrl-weight-cache-invariant and lr+ftrl-dense-sparse-agree both compare against ftrl-weight-of itself, so a consistently wrong ftrl-weight-of would satisfy both. Added ftrl-weight-of-known-values, pinning four hand-derived values against Algorithm 1's formula. Also extended lr+ftrl-rejects-bad-parameters with negative-beta and negative-lambda2 cases the reviewer found uncovered. Suite: 82 deftest / 291 assertions / 0 failures / exit 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two wrappers resolve different functions: MAKE-ONE-VS-REST caches <TYPE>-WEIGHT and <TYPE>-BIAS and scores each class itself, while MAKE-ONE-VS-ONE caches <TYPE>-PREDICT and votes. A learner whose -PREDICT were missing or misnamed would have passed the existing test and failed only here, so this was a real gap rather than a duplicate. Measured 90.67% after ten epochs on iris.scale, against one-vs-rest's 87.33%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1a9b1141f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ;; leaves the cache one update stale per coordinate -- measured at 89 of 123 | ||
| ;; coordinates wrong on a1a, with accuracy indistinguishable either way. | ||
| (let* ((fx (f input weight (lr+ftrl-bias learner))) | ||
| (sigmoid-val (sigmoid (* training-label fx))) |
There was a problem hiding this comment.
Avoid overflowing the sigmoid for large negative margins
When a valid example produces a sufficiently negative margin, this expands to exp(-margin) and can signal floating-point overflow instead of returning a gradient near its limiting value. For example, with one feature valued 1000.0, alpha=0.1, and conflicting consecutive labels, the first FTRL update yields a score around 100 and the second evaluates exp(100.0); SBCL configurations that trap overflow will abort training. The sparse path has the same issue at line 1144. Use a sign-dependent sigmoid formulation so the exponential is always evaluated with a non-positive argument.
Useful? React with 👍 / 👎.
Adds
lr+ftrlandsparse-lr+ftrl— logistic regression trained by FTRL-Proximal. Thisis the first learner in the library that drives weights to exactly zero, so a model
can be shrunk by dropping features rather than only accepting sparse input.
Measured on
t/dataset/a1aover ten passes:lambda10 leaves 113 of 123 weightsnon-zero,
lambda110 leaves 70 — 43% of the model exactly zero — at a cost of 0.56accuracy points (84.80% → 84.24%).
The algorithm
McMahan, Holt, Sculley et al., Ad Click Prediction: a View from the Trenches, KDD 2013,
Algorithm 1. Per coordinate the state is
(z_i, n_i)and the weight is derived, notstored:
API
All positional, like every other learner — which is what lets
make-one-vs-restandmake-one-vs-oneapplytheir&rest learner-paramswith no special case.The one design decision worth reviewing
define-learner's generated-PREDICTreads aweightslot, but FTRL has no weightvector to give it. The derived weight is materialized into a
weightslot as a cache.That is exact because
w_iis a pure function of(z_i, n_i), which change only for thecoordinates an update touches — but only if the cache is refreshed at the END of the
update. Refreshing at the start leaves each coordinate one update stale: measured at
89 of 123 coordinates wrong, with accuracy indistinguishable from correct either way.
lr+ftrl-weight-cache-invariantis the test that catches this, and it is the only thingthat can — no accuracy assertion would.
What this bought
The whole branch is additive: 508 insertions, 0 deletions. Because FTRL presents as an
ordinary binary learner with
weightandbias, nothing existing needed to change —dim-of,n-class-of,sparse-learner?,save,restore,one-vs-restandone-vs-oneall work untouched. Tests check that rather than assume it:make-one-vs-restwithsparse-lr+ftrlreaches 87.33% oniris.scale.The sparse update is genuinely O(nnz). Note for contrast that
sparse-lr+adam's update issparse only in its gradient — its moment updates sweep the full dimension. That is
pre-existing and out of scope here, but is not something this learner copies.
No scratch vectors: the update is coordinate-local, so storage is
weight,z,nandnothing else.
Verification
Golden values cannot validate the code they came from, so independent grounds came first:
implementation, one during review — each recomputing
wfrom(z, n)with no cacheand using the paper's 0/1-label gradient
(p−y)xinstead of this repo's ±1 form. Bothwere compared against the implementation over a full a1a pass at three parameter
settings: max |Δw| ≈ 2.5e-7 to 3.6e-7. These exercise the
sigma_i·w_iterm and the L1branch, which the all-zero first update cannot reach.
equalponweight,zandnafterboth 1 and 10 passes.
ftrl-weight-of-known-valuespins the weight derivation against four hand-derivedliterals, so the two invariant tests above are no longer circular.
Suite: 67 → 82
deftestforms, 239 → 291 assertions, 0 failures. No existing test,golden value, or assertion was modified.
Non-goals
No CLI flag (
lr+sgdandlr+adamare library-only for the same reason — FTRL's fourhyperparameters map onto none of
-gamma/-eta/-c), noexample/entry, noprobability output, and no multiclass-native FTRL.
🤖 Generated with Claude Code