Skip to content

Fix KNN Regressor weighted prediction - #419

Merged
andrewdalpino merged 4 commits into
masterfrom
fix-knn-weighted-regression
Aug 18, 2026
Merged

Fix KNN Regressor weighted prediction#419
andrewdalpino merged 4 commits into
masterfrom
fix-knn-weighted-regression

Conversation

@andrewdalpino

Copy link
Copy Markdown
Member

src/Regressors/KNNRegressor.php:211-224 (root cause :232-246) — HIGH In weighted mode, predictSample builds $weights in distance order but array_values($labels) in original-index order, and Stats::weightedMean pairs them positionally — so each label is multiplied by the weight of a different neighbor.

Verified: train [[1],[2],[3]], labels [0,10,30], k=3 weighted, predict 2.0 → returns 10.0, correct is 12.5.
Verified: [[1],[5],[10]], [0,5,100], k=3 weighted, predict 10 → 8.55, correct is 79.60.
The classifier sibling KNearestNeighbors.php:229-233 does this correctly (aligns by key: $weights[$labels[$i]] += ...).
Fix: build weights keyed by neighbor index and pass $labels + $weights un-reordered.

@andrewdalpino
andrewdalpino requested a lite review from Copilot August 15, 2026 21:42
@andrewdalpino andrewdalpino added the bug Something isn't working label Aug 15, 2026
@andrewdalpino
andrewdalpino requested a review from a team August 15, 2026 21:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes incorrect weighted predictions in KNNRegressor by ensuring the per-neighbor weights remain aligned with the corresponding neighbor labels when distances are sorted by proximity.

Changes:

  • Key the computed $weights by the original neighbor index (as preserved by asort(...)+array_slice(..., true)), instead of building a reindexed list.
  • Pass $labels to Stats::weightedMean() without reindexing, so weights/labels are paired by key rather than by position.
  • Add regression tests covering the misalignment case, a boundary/edge case, and correct enforcement of the k limit in weighted mode.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/Regressors/KNNRegressor.php Fixes weighted prediction by aligning weights with label keys preserved through distance sorting/slicing.
tests/Regressors/KNNRegressorTest.php Adds regression tests to ensure weighted predictions are correct and stable for key alignment + k limiting.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@apphp

apphp commented Aug 16, 2026

Copy link
Copy Markdown

LGTM

@andrewdalpino
andrewdalpino merged commit a9f71ea into master Aug 18, 2026
12 checks passed
@andrewdalpino
andrewdalpino deleted the fix-knn-weighted-regression branch August 18, 2026 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants