fix(base): pad predict_proba to the schema's class count - #399
Open
tachyonicClock wants to merge 1 commit into
Open
fix(base): pad predict_proba to the schema's class count#399tachyonicClock wants to merge 1 commit into
tachyonicClock wants to merge 1 commit into
Conversation
MOA's getVotesForInstance sizes its return array by the classes seen so far during training, not by the schema, so MOAClassifier.predict_proba could silently return an array shorter than schema.get_num_classes(). Votes are indexed by class index, so the fix is to zero-pad trailing entries rather than reorder anything. Also removes DynamicEnsembleMemberSelection's duplicate predict_proba override, which hand-rolled the same padding but kept a stale total <= 1e-2 reject rule that was deliberately dropped from the base class. Fixes adaptive-machine-learning/backlog#96 Assisted-by: claude-code:claude-sonnet-5
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.
Problem
MOAClassifier.predict_probacan return an array shorter thanschema.get_num_classes().MOA's
getVotesForInstancesizes its return array by the classes seen so far, not by theschema. Votes are indexed by class index, so the missing entries are always trailing
zeros — this is truncation, not reordering.
Verified against the current
moa.jar: 19 of 25 classifiers incapymoa.classifierreturned a short array after 20 training instances on
CovtypeTiny. The 6 that lookedcorrect were the scikit-learn and torch backed learners, which don't go through
MOAClassifier. So this is one bug in the shared wrapper, not a per-learner bug.predictis unaffected (argmax over a truncated array still gives the right index), andthe evaluators build their own vote template. Only
predict_probacallers see it.Refs adaptive-machine-learning/backlog#96
Changes
Pad in
MOAClassifier.predict_proba— zero-padvotestoschema.get_num_classes()before the total-is-usable check and the normalisation.Pads only when short; a longer array is left alone. The existing reject rules (empty,
non-finite total, total <= 0) are unchanged — padding an empty array still yields all
zeros, so the "no prediction" case still returns
None.Delete the duplicate override in
DynamicEnsembleMemberSelection— it hand-rolledthe same padding, which is why it was one of the 6 that looked correct. It also carried
the old
total <= 1e-2reject rule that was deliberately removed from the base class(see
test_optimise_flag_does_not_change_results). Now dead weight.Fix the existing unit test's fake classifier —
test_predict_proba_only_rejects_absent_predictionsbuilds via
MOAClassifier.__new__and set onlymoa_learner, so readingself.schemawould raise. It now gets a fake schema reporting 2 classes, matching its 2-element vote
cases. No
getattrguard was added to the implementation to accommodate the test.Regression test across all classifiers — the existing shape assertion in
test_classifiersonly passed because it usesElectricityTiny(2 classes) after afull prequential run, by which point both classes have been seen. Added a subtest using
a fresh learner on
RandomTreeGenerator(num_classes=7)(no dataset download) thatasserts shape
(7,)and that values sum to 1, skipping whenpredict_probareturnsNonesince abstaining is allowed. Reusestest_case.learner_constructor, so everyregistered classifier is covered.
Upstream
The underlying bug is in MOA itself:
HoeffdingTree.getVotesForInstancedelegates to theleaf's
getClassVotes, which returnsobservedClassDistribution.getArrayCopy()— aDoubleVectortrimmed to the highest class index observed. Notably thetreeRoot == nullbranch a few lines below does size correctly with
inst.dataset().numClasses().moa.jaris a prebuilt binary pulled from the URL ininvoke.yml, so the Java fix can'tland here. A bug report for
Waikato/moahas been drafted separately. The CapyMOA paddingis worth keeping either way, since it covers every MOA learner rather than one class.
Verification
capymoa.classifieronCovtypeTinyafter 20 instances: all25 return length 7. Before the fix, 19 returned 5.
pytest tests/test_classifiers.py tests/test_evaluation.py tests/test_ssl_classifiers.py—79 passed, 1 skipped, 52 subtests passed.
pytest tests— 290 passed, 25 skipped.pytest --doctest-modules src— 129 passed.invoke fmt— no changes.🤖 Generated with Claude Code