onnx / sklearn-onnx

Convert scikit-learn models and pipelines to ONNX
Apache License 2.0
543 stars 99 forks source link

NotImplementedError: metric='cosine' is not implemented. #635

Closed mikhail-barg closed 3 years ago

mikhail-barg commented 3 years ago

I'm trying to use ONNX format as an universal container for models transfer, and I've found that I'm not able to export a simple kNN model with cosine distance, which is very common in NLP:

clf = KNeighborsClassifier(7,weights='distance',metric='cosine',n_jobs=-1)
clf.fit(vectors_train, y_train)
...
initial_type = [('float_input', FloatTensorType([None, 4]))]
onx = convert_sklearn(knn, initial_types=initial_type)

gives

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/proj/python/onnx/lib/python3.6/site-packages/skl2onnx/convert.py", line 162, in convert_sklearn
    remove_identity=not intermediate)
  File "/proj/python/onnx/lib/python3.6/site-packages/skl2onnx/common/_topology.py", line 1087, in convert_topology
    conv(scope, operator, container)
  File "/proj/python/onnx/lib/python3.6/site-packages/skl2onnx/common/_registration.py", line 29, in __call__
    return self._fct(*args)
  File "/proj/python/onnx/lib/python3.6/site-packages/skl2onnx/operator_converters/nearest_neighbours.py", line 442, in convert_nearest_neighbors_classifier
    many = _convert_nearest_neighbors(operator, container)
  File "/proj/python/onnx/lib/python3.6/site-packages/skl2onnx/operator_converters/nearest_neighbours.py", line 268, in _convert_nearest_neighbors
    **distance_kwargs)
  File "/proj/python/onnx/lib/python3.6/site-packages/skl2onnx/operator_converters/nearest_neighbours.py", line 92, in onnx_nearest_neighbors_indices_k
    **kwargs)
  File "/proj/python/onnx/lib/python3.6/site-packages/skl2onnx/algebra/complex_functions.py", line 105, in onnx_cdist
    metric))
NotImplementedError: metric='cosine' is not implemented.

which is true in current master: https://github.com/onnx/sklearn-onnx/blob/a8267e7ba946d8b0596951060e5dca39fec47439/skl2onnx/algebra/complex_functions.py#L83-L105

And actually searching through the repository, there's no single mention of cosine in the whole code base.

So, I'd like to clarify, is there any point in waiting for it to be implemented in not-too-distant future, or it's more reasonable to abandon hope and move somewhere else?

xadupre commented 3 years ago

A PR within the new two weeks would be ok for you?

mikhail-barg commented 3 years ago

Probably yes. Looking forward to it, thanks!

xadupre commented 3 years ago

Fix is implemented in #641.

danitico commented 1 year ago

Hi @xadupre ! This is not working with sklearn version 1.1.1 and sklearn-onxx version 1.13

xadupre commented 1 year ago

The same error is happening?

danitico commented 1 year ago

@xadupre Yeah! The same traceback as in the first message of this issue

Traceback (most recent call last):
  File "/Users/daniel/Desktop/themis/train_and_convert_to_onnx.py", line 24, in <module>
    onx = convert_sklearn(model, initial_types=initial_type)
  File "/Users/daniel/Library/Caches/pypoetry/virtualenvs/themis-iQntZxE_-py3.9/lib/python3.9/site-packages/skl2onnx/convert.py", line 184, in convert_sklearn
    onnx_model = convert_topology(
  File "/Users/daniel/Library/Caches/pypoetry/virtualenvs/themis-iQntZxE_-py3.9/lib/python3.9/site-packages/skl2onnx/common/_topology.py", line 1421, in convert_topology
    topology.convert_operators(container=container, verbose=verbose)
  File "/Users/daniel/Library/Caches/pypoetry/virtualenvs/themis-iQntZxE_-py3.9/lib/python3.9/site-packages/skl2onnx/common/_topology.py", line 1256, in convert_operators
    self.call_converter(operator, container, verbose=verbose)
  File "/Users/daniel/Library/Caches/pypoetry/virtualenvs/themis-iQntZxE_-py3.9/lib/python3.9/site-packages/skl2onnx/common/_topology.py", line 1062, in call_converter
    conv(self.scopes[0], operator, container)
  File "/Users/daniel/Library/Caches/pypoetry/virtualenvs/themis-iQntZxE_-py3.9/lib/python3.9/site-packages/skl2onnx/common/_registration.py", line 26, in __call__
    return self._fct(*args)
  File "/Users/daniel/Library/Caches/pypoetry/virtualenvs/themis-iQntZxE_-py3.9/lib/python3.9/site-packages/skl2onnx/operator_converters/nearest_neighbours.py", line 442, in convert_nearest_neighbors_classifier
    many = _convert_nearest_neighbors(operator, container)
  File "/Users/daniel/Library/Caches/pypoetry/virtualenvs/themis-iQntZxE_-py3.9/lib/python3.9/site-packages/skl2onnx/operator_converters/nearest_neighbours.py", line 265, in _convert_nearest_neighbors
    top_indices, top_distances = onnx_nearest_neighbors_indices_k(
  File "/Users/daniel/Library/Caches/pypoetry/virtualenvs/themis-iQntZxE_-py3.9/lib/python3.9/site-packages/skl2onnx/operator_converters/nearest_neighbours.py", line 90, in onnx_nearest_neighbors_indices_k
    dist = onnx_cdist(X, Y, metric=metric, dtype=dtype,
  File "/Users/daniel/Library/Caches/pypoetry/virtualenvs/themis-iQntZxE_-py3.9/lib/python3.9/site-packages/skl2onnx/algebra/complex_functions.py", line 106, in onnx_cdist
    raise NotImplementedError("metric='{}' is not implemented.".format(
NotImplementedError: metric='cosine' is not implemented.
samwarren commented 1 year ago

still having this issue as well