Add optional L2 instance normalization for Euclidean kNN#7309
Conversation
Codecov Report❌ Patch coverage is 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:
|
be40ad5 to
d3d90ca
Compare
|
@gmolledaj, thanks for a well-made PR. I have a few comments.
|
|
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. |
|
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 I need to revisit my points.
|
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