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

[FIX] owhierarchicalclustering: Fix performance on deselection #2670

Merged
merged 1 commit into from
Oct 12, 2017

Conversation

ales-erjavec
Copy link
Contributor

@ales-erjavec ales-erjavec commented Oct 10, 2017

Issue

The widget is slow to respond when the selection is changed in such a way that the number of groups diminishes significantly.

Example:

File (vehicle.tab) -> Distances (Euclidean) -> Hierarchical clustering (Average)

In the Selection box select and set Height ratio: 0.1%. Switch between that and Top N: 3 Going from Height ratio to top N is significantly slower then the other way.

Description of changes

The code (set_selected_items) had quadratic time complexity in the number of deselected items.
Fixed by a more judicious placement of calls to _re_enumerate_selections

Includes
  • Code changes
  • Tests
  • Documentation

@ales-erjavec ales-erjavec changed the title owhierarchicalclustering: Fix performance on deselection [FIX] owhierarchicalclustering: Fix performance on deselection Oct 10, 2017
@ajdapretnar
Copy link
Contributor

Hmm, it is still very slow (I can't really tell the difference in time, it just feels very long). You can say it can't be done, that's fine. I just noticed this when working on a 33180 x 149 data set.

@jerneju
Copy link
Contributor

jerneju commented Oct 12, 2017

@ales-erjavec Please restart Travis and AppVeyor.

The code had quadratic time complexity in the number of deselected items.
Fixed by a more judicious placement of calls to `_re_enumerate_selections`
@codecov-io
Copy link

Codecov Report

Merging #2670 into master will increase coverage by 0.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2670      +/-   ##
==========================================
+ Coverage    75.5%   75.51%   +0.01%     
==========================================
  Files         332      332              
  Lines       58014    58014              
==========================================
+ Hits        43803    43810       +7     
+ Misses      14211    14204       -7

@jerneju jerneju merged commit 09515ed into biolab:master Oct 12, 2017
@ales-erjavec ales-erjavec deleted the fixes/hclust-remove-items-fix branch November 14, 2017 08:55
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.

4 participants