Skip to content

Add optional L2 instance normalization for Euclidean kNN#7309

Open
gmolledaj wants to merge 1 commit into
biolab:masterfrom
gmolledaj:feature/knn-l2
Open

Add optional L2 instance normalization for Euclidean kNN#7309
gmolledaj wants to merge 1 commit into
biolab:masterfrom
gmolledaj:feature/knn-l2

Conversation

@gmolledaj

Copy link
Copy Markdown
Contributor
Issue

Implements the proposed solution discussed in #7270.

Description of changes

This PR adds an optional "Normalize instances (L2)" setting to the kNN widget.

When enabled, each input instance is normalized to unit L2 norm before learning by adding an internal preprocessor to the learner pipeline while preserving any existing preprocessors.

The option is available only when the Euclidean metric is selected, since L2-normalized Euclidean distance provides the same nearest-neighbor ranking as cosine similarity for embeddings. This makes the widget more suitable for text embedding workflows without requiring a Python Script widget or globally modifying the preprocessing pipeline.

Includes
  • Code changes
  • Tests
  • Documentation

@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.77419% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.97%. Comparing base (2ab65aa) to head (d3d90ca).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7309      +/-   ##
==========================================
- Coverage   88.98%   88.97%   -0.02%     
==========================================
  Files         336      336              
  Lines       74544    74575      +31     
==========================================
+ Hits        66336    66351      +15     
- Misses       8208     8224      +16     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@janezd

janezd commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@gmolledaj, thanks for a well-made PR. I have a few comments.

  1. Learners have default preprocessors -- in case of KNN (and most others models based on SKL fitters), they

    • exclude instances with undefined classes,
    • replace discrete variables with dummy variables,
    • remove variables without any defined values and
    • impute remaining missing values.

    These preprocessors are applied only if there are no user-defined preprocessors, i.e. preprocessors given as an input to the widget. In case there are, it's the user's responsibility to handle any categorical or missing data (or ensure there is None). The solution in this PR adds a preprocessor to the list that is meant for user-defined preprocessors. While this is a smart idea, it will prevent the application of default preprocessors listed above. The current PR will fail for this data set if normalization is checked: data.xlsx.zip

    My knowledge of Orange's classification and regression models are limited (I wrote the first implementation and the current implementation is attempt by other to clean my mess), but I think the simplest implementation that would follow this implementation would be to override the method preprocess of Orange.regression.knn.KNNRegressionLearner and Orange.classification.knn.KNNLearner.

    Let @markotoplak and @VesnaT tell whether there are any more appropriate methods to override instead.

    The SKL model also allows passing a callable as metric, but this would (I'm guessing here) prevent the model to use advanced methods to search for neighbours (like various trees), so preprocessing is probably a better approach.

  2. How should this normalization treat categorical variables? Default preprocessing replaces it with dummy indicator variables; I suppose we shouldn't touch them? In this case, the preprocessing from the previous point should be applied before calling super().preprocess() and only preprocess numeric variables.

  3. The widget also offers Manhattan distance. Would it make sense to offer (L1?) normalization for it?

  4. Regarding the GUI: Distance (and perhaps some other widgets) offer "Euclidean" and "Euclidean (normalized)" (as well as Manhattan and Manhattan (normalized)) as two separate options. We could follow this pattern here, too. It needs clicking and, in particular, no code required to enable and disable the checkbox.

    The downside is that this would probably reorders the options in the combo, which would change the indices and invalidate the current workflows. I volunteer to write the migration.

    If we go this route, combo can be replaced with radio buttons. It is more consistent with Distances widget and the user sees what is available, on the other hand most classification/regression widgets prefer combos. I don't have a strong preference here.

@gmolledaj

Copy link
Copy Markdown
Contributor Author

Thank you very much for the detailed review and for identifying the interaction with the learner's default preprocessors. I had not realized that adding the L2 normalizer to the user-defined preprocessor sequence would suppress the default preprocessing pipeline. I agree that this must be corrected.

Overriding the learner's preprocess method seems like a better integration point, although I would prefer to wait for @markotoplak and @VesnaT to confirm the most appropriate hook before changing the implementation.

Regarding categorical variables, my original use case consists of document embeddings, so the input is normally continuous. More generally, I agree that dummy indicator variables should probably not be affected by instance normalization. The exact ordering is important, however, because normalization must also coexist correctly with missing-value imputation. I would be happy to implement whichever ordering the maintainers consider consistent with Orange's preprocessing architecture.

Concerning Manhattan distance, my original proposal was specifically motivated by the exact neighbour-ranking equivalence between cosine distance and Euclidean distance on L2-normalized vectors. L1 normalization combined with Manhattan distance does not provide the same cosine-equivalence argument, so I would prefer not to add it merely for symmetry unless there is a separate use case for it.

Presenting this as separate options such as Euclidean and Euclidean (normalized) also sounds reasonable and would simplify the GUI logic. Before following that pattern, I would only like to confirm that “normalized” in the Distances widget refers to the same per-instance normalization rather than feature-wise normalization. If it does, consistency with that widget would be a strong argument in favour of this design.

Thank you also for offering to write the settings migration. Once there is agreement on the preprocessing hook and GUI design, I will be glad to revise the PR accordingly.

@janezd

janezd commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

I'm sorry, my bad. I have read that your intention was to implement cosine distance, but I forgot it and reverted back to thinking about feature-wise normalization. Thanks for your patience. :)

This shows that @markotoplak is right: if this is a cosine distance, why not call it such? Computing Euclidean distance on normalized rows (rather than passing metric="cosine" to SKL) is just an implementational detail. If a user wants cosine distance, (s)he will look for it under that name, especially if working with texts.

I need to revisit my points.

  1. Still applies.
  2. I think that the proper approach would be to show an error if data includes categorical features. The assumption of cosine distance is that features are "homogenous" in the sense they measure "same things", like word frequencies. If some features are categorical, then - I think - cosine distance doesn't apply.
  3. L1 of course no longer makes sense. (But this, I think, supports @markotoplak 's case that we should use the term cosine, not L2).
  4. If we call it cosine, it belongs to the combo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants