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] Tree: Sparse Support #2430

Merged
merged 1 commit into from
Jul 21, 2017
Merged

[FIX] Tree: Sparse Support #2430

merged 1 commit into from
Jul 21, 2017

Conversation

jerneju
Copy link
Contributor

@jerneju jerneju commented Jun 23, 2017

Issue

Fixes #2370 .

Description of changes

Work in progress.

Includes
  • Code changes
  • Tests
  • Documentation

@codecov-io
Copy link

codecov-io commented Jun 23, 2017

Codecov Report

Merging #2430 into master will increase coverage by 1.84%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2430      +/-   ##
==========================================
+ Coverage    73.9%   75.74%   +1.84%     
==========================================
  Files         321      321              
  Lines       55993    60239    +4246     
==========================================
+ Hits        41379    45630    +4251     
+ Misses      14614    14609       -5

@jerneju jerneju added the DH2017 label Jun 30, 2017
@nikicc nikicc changed the title [WIP][FIX] Tree: Sparse Support [FIX] Tree: Sparse Support Jul 13, 2017
@nikicc
Copy link
Contributor

nikicc commented Jul 18, 2017

@jerneju is this ready for review? IMO this should also go into the next release?

@jerneju
Copy link
Contributor Author

jerneju commented Jul 18, 2017

@nikicc: Yes, it is ready.

@janezd
Copy link
Contributor

janezd commented Jul 21, 2017

Works, but it's extremely slow even on reasonably sized sparse data.

https://github.com/biolab/orange3/pull/2430/files#diff-e09cca17420838b03154933ede608989R171 changes the column to dense. Since this is done column by column, it is (memory-wise) better than changing the whole matrix to dense but it's time consuming. A better solution - but one requiring a bit more programming - would be to implement a separate _tree_scorers.contingency, branches_from_mapping and build_tree (active_inst[branches == br]) to work with sparse data.

@nikicc said we should merge it as it is, but please start working on a better solution.

@janezd janezd merged commit 6865a45 into biolab:master Jul 21, 2017
@jerneju jerneju deleted the sparse-tree branch July 21, 2017 09:51
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