[ENH] Support sparse Jaccard

[ajdapretnar]

Issue

Jaccard did not support sparse data, making it useless for text mining.

Description of changes

Custom support for sparse Jaccard.

Includes

• Code changes
• Tests
• Documentation

[ajdapretnar]

@lanzagar @thocevar @janezd
This requires some discussion.

1. Before, if Distances are given sparse discrete data, Jaccard didn't work. Currently fixed with a no-so-nice hack.
2. DistMatrix cannot be tested with np.testing.assert_array_equal, because max() returns an int instead of numpy.int64. See: DistMatrix: np.testing.assert_array_equal crashes on two different matrices #3658
3. Distance.Jaccard().fit does not work for any sparse data. How to solve this?

All in all, the code needs to be made nicer and cleaner. Any suggestions welcome.

[janezd]

Is there a reason for not checking metric.supports_sparse instead of metric is not distance.Jaccard?

The condition that specifically checks for Jaccard a few lines later is needed because there is no specific flag signalling whether a metric supports distances by columns. Here, we have a flag to check, unless I overlooked something.

[ajdapretnar]

Jaccard is the only distance that supports discrete attributes via fallback. Other metrics fall back to sklearn's methods, which won't work with discrete.

[codecov]

Codecov Report

Merging #3657 into master will increase coverage by <.01%.
The diff coverage is 89.09%.

@@            Coverage Diff             @@
##           master    #3657      +/-   ##
==========================================
+ Coverage   84.31%   84.32%   +<.01%     
==========================================
  Files         370      370              
  Lines       67856    67899      +43     
==========================================
+ Hits        57214    57255      +41     
- Misses      10642    10644       +2

[codecov]

Codecov Report

Merging #3657 into master will increase coverage by <.01%.
The diff coverage is 97.82%.

@@            Coverage Diff             @@
##           master    #3657      +/-   ##
==========================================
+ Coverage   84.43%   84.44%   +<.01%     
==========================================
  Files         372      372              
  Lines       68111    68143      +32     
==========================================
+ Hits        57511    57543      +32     
  Misses      10600    10600

[janezd]

551effc changes the base class so that it can handle numpy arrays if fallback is not provided. Perhaps we can stop passing numpy arrays to fallbacks and use fallbacks only for sparse data.

fd1fd19 moves @ajdapretnar's code for sparse Jaccard to the proper class and also contains minor fixes in tests.

@ajdapretnar, you still have to compute the probabilities in sparse data fitter for handling missing values when the model is used on dense data.

[janezd]

Jaccard.compute_distances is now ugly. Dense data is handled in compute_distances itself, while sparse data is in another method. It should be symmetric: one method for dense and one for sparse, and compute_distances calls one or another. That is, extract most of the code from compute_distances, except for the first two lines, into a separate method.

sparse_jaccard is also not a very good name: jaccard is already the class name. Maybe _compute_sparse and _compute_dense.

[janezd]

Cosine tests failed because of wrong clipping: sklearn (correctly) clips to [0, 2], while our function for dense matrices (effectively) clipped to [0, 1]. This commit doesn't belong to this PR, but this PR adds tests for comparison between distances on sparse and dense matrices, which revealed this problem, hence its more practical to fix this here.

[ajdapretnar]

If I understand the Cython code correctly, this should be ok. @janezd