Skip to content

FIX: Avoid setting 'p' internal metric parameter when sklearn wouldn't set it#2945

Open
david-cortes-intel wants to merge 3 commits intouxlfoundation:mainfrom
david-cortes-intel:no_p_eucl
Open

FIX: Avoid setting 'p' internal metric parameter when sklearn wouldn't set it#2945
david-cortes-intel wants to merge 3 commits intouxlfoundation:mainfrom
david-cortes-intel:no_p_eucl

Conversation

@david-cortes-intel
Copy link
Contributor

@david-cortes-intel david-cortes-intel commented Feb 13, 2026

Description

KNN classes have an internal attribute effective_metric_params_ which uses a 'p' parameter for minkowski, but it looks like sklearnex is setting this parameter for other metrics too, which leads to sklearn throwing a warning about this parameter being unused when falling back to it, plus ending up with different public-facing attributes than sklearn.


Checklist:

Completeness and readability

  • I have commented my code, particularly in hard-to-understand areas.
  • Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details).
  • I have resolved any merge conflicts that might occur with the base branch.

Testing

  • I have run it locally and tested the changes extensively.
  • All CI jobs are green or I have provided justification why they aren't.
  • I have extended testing suite if new functionality was introduced in this PR.

@david-cortes-intel david-cortes-intel added the bug Something isn't working label Feb 13, 2026
@david-cortes-intel
Copy link
Contributor Author

/intelci: run

@codecov
Copy link

codecov bot commented Feb 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
azure 79.83% <100.00%> (?)
github 81.85% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
sklearnex/neighbors/common.py 92.26% <100.00%> (-0.23%) ⬇️
sklearnex/neighbors/knn_classification.py 98.64% <ø> (ø)
sklearnex/neighbors/knn_regression.py 100.00% <ø> (ø)
sklearnex/neighbors/knn_unsupervised.py 98.24% <ø> (ø)

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@david-cortes-intel david-cortes-intel changed the title [Do NOT merge yet] FIX: Avoid setting unused 'p' internal metric parameter when falling back to sklearn [Do NOT merge yet] FIX: Avoid setting 'p' internal metric parameter when sklearn wouldn't set it Feb 13, 2026
@david-cortes-intel david-cortes-intel changed the title [Do NOT merge yet] FIX: Avoid setting 'p' internal metric parameter when sklearn wouldn't set it FIX: Avoid setting 'p' internal metric parameter when sklearn wouldn't set it Feb 13, 2026
@david-cortes-intel david-cortes-intel marked this pull request as ready for review February 13, 2026 13:15
@david-cortes-intel
Copy link
Contributor Author

CI error is in model builders, doesn't look related to this PR:

tests/test_model_builders.py::test_treelite_unsupported PASSED
OSError: [WinError 6] The handle is invalid

@david-cortes-intel
Copy link
Contributor Author

/intelci: run

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.

1 participant