Fix clol:test for regression learners, and migrate the test suite to rove - #3
Merged
Merged
Conversation
CLOL:TEST always calls <TYPE>-TEST with both :quiet-p and :stream, but the -TEST generated by DEFINE-REGRESSION-LEARNER accepted only :quiet-p. Every call of (clol:test rls-learner data) therefore signalled UNKNOWN-KEYWORD-ARGUMENT, which also made clol-predict unusable for RLS and SPARSE-RLS models. Accept :stream and write one predicted value per test datum, mirroring the classifier -TEST. The raw value is printed rather than its rounded sign, since that is the prediction for a regression model. Add the first regression assertions to the test suite (94 -> 96). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The remaining prove forms are parked in a block comment and converted section by section in following commits. Also adds :shadowing-import-from :cl-online-learning :test to the cl-online-learning.test package: CL-ONLINE-LEARNING:TEST and rove's re-exported ROVE/CORE/RESULT:TEST name-conflict under plain :USE (ANSI 11.1.1.2.5), and every learner section calls (test learner data) expecting CL-ONLINE-LEARNING:TEST. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Move the sparse perceptron, AROW, SCW-I, LR+SGD and LR+ADAM prove forms out of the parked block comment into five deftests, following the shape Task 2 established for the dense binary learners. Every weight/bias/accuracy literal is copied verbatim from the parked prove form it replaces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Move the ten dense multiclass learner assertions (five one-vs-rest, five one-vs-one) from the parked prove block into deftests, moving the #| opener past both sections. Each deftest builds its own one-vs-rest/one-vs-one wrapper in a let, removing the setf-reuse coupling the prove forms had between the two sections. Also drops the now-redundant "Read libsvm dataset (Dence, Multiclass)" is form, since iris/iris-dim are already header defparameters with their own deftest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Move the #| opener past both sparse one-vs-rest and one-vs-one multiclass sections, converting the ten prove is/:test forms into deftests. Each deftest now builds its own one-vs-rest/one-vs-one wrapper in a let, removing the prove file's setf-based sharing between the two sections. Drops the redundant sparse iris dataset read (already covered by the read-iris-sparse deftest) and the section's format/defvar scaffolding. Only the RLS regression section remains parked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CLAUDE.md still described the prove-era test setup after tasks 1-7 migrated the suite to rove. Corrects: the test framework and run-tests/auto-detection claim, the test invocation command (now needs sb-ext:disable-debugger to fail correctly on a red suite), the CI form, the "no single-test runner" claim, the assertion count in Testing notes, and the stale "RLS has no test coverage" statement (regression-rls covers it as of adbcd21). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- regression-rls now asserts that :STREAM emits the learner's actual predictions (read back and compared to RLS-PREDICT), not just a matching line count, which a constant or the pre-fix value would also have satisfied. - Add regression-sparse-rls, mirroring regression-rls for SPARSE-RLS, which shares the same :STREAM bug fix and previously had no coverage. - Wrap src/rls.lisp:41 to fit the 100-column house style. - Note in ci.yml why `rove cl-online-learning-test.asd` resolves: it depends on install-for-ci.sh's ASDF source-registry tree entry, since rove's CLI does not call load-asd the way the old prove runner did. - Lead CLAUDE.md's test command with the portable `./t/run-test.ros`, demoting the SBCL-only one-liner (with its disable-debugger caveat) to an alternative. Update deftest/assertion counts (36 / 104) to match the two new assertions groups added above.
fail-fast cancelled three matrix jobs the moment ccl-bin on macOS-latest failed to install, so two green SBCL runs and one unfinished CCL run all reported as failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rove reports success when it finds no suites at all, so a run that executed nothing is indistinguishable from a passing one. Observed on ccl-bin in CI, where `rove cl-online-learning-test.asd' printed not one assertion and still exited 0 -- the same silently-green failure this suite was migrated off prove to escape. test-op's :perform now checks that the system registered rove tests before running them, and CI invokes ./t/run-test.ros so it goes through that check instead of calling the rove CLI, which bypasses test-op. This turns the ccl-bin legs from a false green into an honest red until the underlying CCL suite-resolution problem is understood. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
macos-latest runners are arm64 and CCL no longer ships a Darwin binary Roswell can install, so that leg failed at the Roswell install step before any of this project's code ran. Co-Authored-By: Claude Opus 5 (1M context) <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.
Two pieces of work: a bug fix in
src/rls.lisp, and a migration of the test suite from prove to rove.1.
clol:testwas unusable for regression learnersclol:testalways calls<TYPE>-TESTwith both:quiet-pand:stream, but the-TESTgenerated by
define-regression-learneraccepted only:quiet-p:So every
(clol:test rls-learner data)signalled, andclol-predictcould not be used withRLS or SPARSE-RLS models at all. The classifier
-TESTgenerated bydefine-learnerhasaccepted
:streamall along — only the regression macro was left behind, which is why thissurvived: regression had no test coverage.
The fix accepts
:streamand writes one predicted value per test datum, mirroring theclassifier. The raw value is printed rather than its rounded sign, since that is the prediction
for a regression model.
(when stream …)guards the write, avoiding the discarded-stringconsing the classifier does when
streamisnil.End to end, after the fix:
2. prove → rove
t/cl-online-learning.lispwas one flat file of top-level proveisforms. It is now 36deftestforms carrying 104okassertions — the same golden values, plus the new regressioncoverage.
Why migrate. cl-mcp's
run-teststool reported✓ PASS / Passed: 0, Failed: 0for thisrepository even when assertions failed. With one golden value deliberately corrupted,
rosreported
× 1 of 96 tests failedwhile the tool reportedPASS. Three layers compose intothat: the tool's
:provebranch is an unimplemented stub that falls back to a runner treating"no condition signalled" as success, and
prove-asdf:run-test-systemreports failure by returnvalue rather than by signalling. Filed upstream as cl-ai-project/cl-mcp#131. rove is the one
framework that tool supports end to end, including running a single test.
What changed.
cl-online-learning-test.asddepends onroveinstead ofprove;:defsystem-depends-on (:prove-asdf)and the:test-filecomponent type are gone. The component must be aplain
(:file …)— rove resolves a system to its suites throughcomponent-source-files,which only walks
asdf:cl-source-file, so:test-filewould silently yield zero suites.test-op's:performnow fails on both ways a rove run can be green without being good:a red suite (rove reports failure by return value, so the usual idiom would leave
asdf:test-systemexiting 0) and a suite that registered no tests at all. See the CI sectionbelow for why the second check exists.
deftest. The provefile shared learners between the one-vs-rest and one-vs-one sections through top-level
defvars rebound withsetf; that coupling is gone, which is what makes single-testexecution meaningful.
cl-online-learning:test— it name-conflicts with rove'sre-exported
rove/core/result:testunder:use.CLAUDE.mdupdated. Worth knowing: theros -Q run --eval …one-liner needs(sb-ext:disable-debugger)or SBCL enters the debugger whentest-opsignals, hits EOF onclosed stdin, and exits 0 — a green exit for a red suite.
./t/run-test.rosis correctwithout that caveat and is now the documented default.
Golden values are unchanged. Every expected weight vector, bias and accuracy tuple was moved,
not retyped. Verified by extracting all 1771 numeric literals from the pre- and post-migration
files: the added multiset equals the deleted multiset, and the 96 original expected s-expressions
parse identically. Any value that had moved would have been a migration bug, not a value to
regenerate.
3. CI
Three changes, each prompted by something this branch's first CI run exposed.
fail-fast: false. One broken leg was cancelling the other three before they reported,so two genuinely green SBCL runs and one unfinished CCL run all showed as failures.
./t/run-test.rosinstead ofrove cl-online-learning-test.asd. On ccl-bin the rove CLIprinted not one assertion and still exited 0 — the same silently-green failure this
migration exists to escape. Its
run-file-testsresolves the.asdwith(asdf:load-system (pathname-name file))and noload-asd, and found no suites there. Goingthrough
test-opinstead, CCL now runs all 104 assertions. The:performguard describedabove would have caught the false green either way; it is kept as the structural backstop.
ccl-bin×macOS-latestexcluded. Those runners are arm64 and CCL no longer ships aDarwin binary Roswell can install (
Not supported platform arm64), so the leg failed at theRoswell install step before any of this project's code ran.
Remaining matrix:
sbcl-binon ubuntu and macOS,ccl-binon ubuntu — all three running thefull 104 assertions.
Testing
36 deftests, 104 assertions. The
test-opguard was verified against all three outcomes: agreen suite exits 0, a corrupted golden value exits 1, and a suite with no registered deftests
exits 1 with
No rove tests are registered for cl-online-learning-test.New regression coverage — the library previously had none:
regression-rls— RMSE, and that:streamemits one line per datum whose values equalrls-predict. A line-count-only check would pass even if the fix printed the wrong thing.regression-sparse-rls— the same for the sparse variant, which the same bug affected.Known, deliberately out of scope
The regression
-TESTuses(reduce #'+ (mapcar …)), so it rejects a vectortest-datathat-TRAINaccepts viaetypecase— the classifier-TESTusescount-ifand handles both.(clol:test rls-learner (coerce data 'vector))signals aTYPE-ERROR. Emptytest-dataalsodivides by zero. Both predate this branch; leaving them for a separate change kept this diff
focused on the keyword-argument bug.
A clean compile emits two style-warnings from
src/cl-online-learning.lisp, wheredim-ofandn-class-ofreference theone-vs-rest/one-vs-onestructure accessors before thosedefstructs appear. Pre-existing; this branch does not touch that file.🤖 Generated with Claude Code