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
[MRG] Modifies T-SNE for sparse matrix #10206
Closed
thechargedneutron
wants to merge
60
commits into
scikit-learn:master
from
thechargedneutron:tsne_csr
Closed
Changes from all commits
Commits
Show all changes
60 commits
Select commit
Hold shift + click to select a range
b14d689
initial commit
thechargedneutron 8f10631
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron a224e68
changes added
thechargedneutron 390cb86
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron 92d2912
changes added
thechargedneutron b8fc385
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron fb9a76e
changes added
thechargedneutron 9e109a9
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron 500f0c4
check for sparse added
thechargedneutron 2d985bd
changes added
thechargedneutron fdfbf11
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron 4c2cf9a
check for non-negativity done
thechargedneutron 164a232
tests added
thechargedneutron 960d2dd
added backport of getnnz
thechargedneutron 1e53a47
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron c51e60f
added sparse in error message
thechargedneutron 5d8e59d
ensure_min_sample restored
thechargedneutron c394077
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron a11bd8e
backport for downcast_intp_index added
thechargedneutron c4005fb
pep8 fialures corrected
thechargedneutron 9a47517
spelling corrected
thechargedneutron 5fc3f04
newline added
thechargedneutron ef809a6
_swap removed
thechargedneutron 016cb39
corrections added
thechargedneutron abcbb91
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron 6c8d913
swap removed
thechargedneutron 833726d
changes added
thechargedneutron fc9c138
checking of sparse matrix changed
thechargedneutron aa0da86
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron 2a7edfc
indentation corrected
thechargedneutron 4084ad0
spelling corrected
thechargedneutron 49f275c
distance check moved inside precomputed distance check
thechargedneutron 509e573
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron dcd7b79
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron 8c933fd
tests removed
thechargedneutron 1511104
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron 768c3d3
started adding tests
thechargedneutron c7d1270
added non negative test in csr matrix
thechargedneutron 38d639d
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron bb97e05
test for high perplexity added
thechargedneutron 4107d64
redundant code removed
thechargedneutron 31be9b2
restored with better error message
thechargedneutron e0dd45c
import statement added
thechargedneutron a582b16
tests added and error message corrected
thechargedneutron acf180a
minor mistakes corrected
thechargedneutron d4851fe
changes added
thechargedneutron ec7ac78
pep8 corrected
thechargedneutron b03c37c
changes added
thechargedneutron dd404e4
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron 27f41fc
dtype corrected
thechargedneutron c472657
query_is_train logic added
thechargedneutron 7c7a240
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron acb9c2e
tests added
thechargedneutron eba780a
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron d105231
pep8 corrected
thechargedneutron ab82925
Tests for sparse support in tSNE and Neighbors (#4)
jnothman e6bc8aa
previous review comments
thechargedneutron cb3127d
number of neighbors mismatch changed
thechargedneutron 788d890
initial changes
thechargedneutron 76b0e69
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn…
thechargedneutron File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://gist.github.com/thechargedneutron/c2f95ac46a9ab61beb0de8a91a9a533f
Running the above code (tests provided by @jnothman ) on this branch produces the following
dist
matrix:And the corresponding indices of
dist
are:And the expected result of indices are:
But in row 1 for example, how are we supposed to get 4 as an index while it is passed as a sparse array with a zero at that position (visible from indices output). Am I missing out on something?
I know this issue is taking too long. Sorry for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This dist matrix give you 2 neighbors, 3 if you include self as explicit zeros.
The expected result of indices corresponds to 3 neighbors, without self.
The fix is replacing in the gist
nn.kneighbors_graph(X_train.copy(), mode='distance')
bynn.kneighbors_graph(None, mode='distance')
, which is the correct syntax to have 3 neighbors without self.In
base.py
you also have to uncomment:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh my mistake, you are testing the explicit diagonal case...
The expected result of indices is wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TomDLT Is the fix that you provided above in the gist still valid? (after you accepted there's fault in your above comment?) . I am not familiar with generating CSR Matrix from
kneighbors_graph
.