Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENH] Silhouette plot: Accept distance matrix on input #4313

Merged
merged 2 commits into from
Jan 10, 2020

Conversation

ales-erjavec
Copy link
Contributor

Issue

Silhouette plot does not accept distance matrix on input.

Description of changes
  • Accept either table or a distance matrix (with an attached table) on input. If distance matrix is provided the associated 'Distance' GUI controls are disabled.
Includes
  • Code changes
  • Tests
  • Documentation

@codecov
Copy link

codecov bot commented Jan 6, 2020

Codecov Report

Merging #4313 into master will increase coverage by <.01%.
The diff coverage is 95.27%.

@@            Coverage Diff             @@
##           master    #4313      +/-   ##
==========================================
+ Coverage   86.75%   86.75%   +<.01%     
==========================================
  Files         396      396              
  Lines       71660    71738      +78     
==========================================
+ Hits        62169    62238      +69     
- Misses       9491     9500       +9

@ajdapretnar
Copy link
Contributor

The widget crashes on Bhattacharyya distance. The rest works fine.

---------------------------- ValueError Exception -----------------------------
Traceback (most recent call last):
  File "/Users/ajda/orange/orange-canvas-core/orangecanvas/scheme/signalmanager.py", line 936, in __process_next
    if self.__process_next_helper(use_max_active=True):
  File "/Users/ajda/orange/orange-canvas-core/orangecanvas/scheme/signalmanager.py", line 974, in __process_next_helper
    self.process_node(selected_node)
  File "/Users/ajda/orange/orange-canvas-core/orangecanvas/scheme/signalmanager.py", line 605, in process_node
    self.send_to_node(node, signals_in)
  File "/Users/ajda/orange/orange-widget-base/orangewidget/workflow/widgetsscheme.py", line 792, in send_to_node
    self.process_signals_for_widget(node, widget, signals)
  File "/Users/ajda/orange/orange-widget-base/orangewidget/workflow/widgetsscheme.py", line 833, in process_signals_for_widget
    widget.handleNewSignals()
  File "/Users/ajda/orange/orange3/Orange/widgets/visualize/owsilhouetteplot.py", line 247, in handleNewSignals
    self._update()
  File "/Users/ajda/orange/orange3/Orange/widgets/visualize/owsilhouetteplot.py", line 375, in _update
    self._matrix[~mask, :][:, ~mask], labels, metric="precomputed")
  File "/Users/ajda/miniconda3/envs/o3/lib/python3.7/site-packages/sklearn/metrics/cluster/_unsupervised.py", line 234, in silhouette_samples
    **kwds))
  File "/Users/ajda/miniconda3/envs/o3/lib/python3.7/site-packages/sklearn/metrics/pairwise.py", line 1592, in pairwise_distances_chunked
    n_jobs=n_jobs, **kwds)
  File "/Users/ajda/miniconda3/envs/o3/lib/python3.7/site-packages/sklearn/metrics/pairwise.py", line 1718, in pairwise_distances
    check_non_negative(X, whom=whom)
  File "/Users/ajda/miniconda3/envs/o3/lib/python3.7/site-packages/sklearn/utils/validation.py", line 979, in check_non_negative
    raise ValueError("Negative values in data passed to %s" % whom)
ValueError: Negative values in data passed to `pairwise_distances`. Precomputed distance  need to have non-negative values.
-------------------------------------------------------------------------------

@ales-erjavec
Copy link
Contributor Author

Distances should be non-negative and that includes Bhattacharyya distance which by definition cannot be negative (probably a floating point math rounding issue in the implementation).

@janezd
Copy link
Contributor

janezd commented Jan 8, 2020

If Bhattacharyya distance can return negative distances, it has to be fixed. Silhouette won't be the only widget affected. @AndrejaKovacic, could you check please?

@janezd janezd self-assigned this Jan 10, 2020
@janezd janezd merged commit 667af57 into biolab:master Jan 10, 2020
@ales-erjavec ales-erjavec deleted the silhouette-plot-dist-matrix-input branch March 16, 2020 13:44
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.

3 participants