Fix confirmed bugs from the issue tracker (#17, #18, #19, #26, #27)

tests/test_kernel_units_and_backends.py

@@ -0,0 +1,130 @@
+"""
+Regression tests for bugs reported in the issue tracker.
+
+Covers:
+  #26 - adaptive bandwidth must be in the same distance units as the distances it
+        normalizes (angular vs cosine-distance).
+  #27 - nmslib index data type must match the space name it is paired with.
+  #19 - t-SNE must use the metric matching the input it is actually given.
+  #18 - evaluation pipeline must accept sparse CSR input.
+  #17 - topo.eval.trustworthiness must remain importable.
+"""
+import numpy as np
+import pytest
+from scipy.sparse import csr_matrix, find
+
+from topo.base.ann import kNN
+from topo.tpgraph.kernels import (
+    _adap_bw,
+    _angularize_graph,
+    _cosine_distance_to_angle,
+    compute_kernel,
+)
+
+
+def _toy(n=200, p=20, seed=0):
+    return np.random.default_rng(seed).normal(size=(n, p))
+
+
+# ── #26 ───────────────────────────────────────────────────────────────────────
+def test_adaptive_bandwidth_matches_distance_units():
+    """adap_sd and dists must both be angular, so their ratio is O(1)."""
+    X, k = _toy(), 10
+    K = kNN(X, metric="cosine", n_neighbors=k, backend="sklearn", n_jobs=1)
+
+    K_ang = _angularize_graph(K, "cosine", True)
+    adap = _adap_bw(K_ang, k)
+    _, _, dists = find(K_ang)
+
+    # angles live in [0, pi]; cosine distances in [0, 2]
+    assert dists.max() <= np.pi + 1e-9
+    assert adap.max() <= np.pi + 1e-9
+
+    # the same bandwidth derived from unconverted distances is ~2x smaller, which is
+    # precisely the mismatch that made the kernel collapse
+    adap_cosine_units = _adap_bw(K, k)
+    assert adap.mean() > 1.8 * adap_cosine_units.mean()
+
+    ratio = dists / (adap[np.asarray(find(K_ang)[0])] + 1e-10)
+    assert 0.5 < ratio.mean() < 2.0, f"normalized distance out of scale: {ratio.mean()}"
+
+
+def test_angular_conversion_is_a_noop_for_non_cosine_metrics():
+    X = _toy()
+    K = kNN(X, metric="euclidean", n_neighbors=10, backend="sklearn", n_jobs=1)
+    assert _angularize_graph(K, "euclidean", True) is K
+    assert _angularize_graph(K, "cosine", False) is K
+
+
+def test_euclidean_kernel_unaffected_by_use_angular():
+    X = _toy()
+    a = compute_kernel(X, metric="euclidean", n_neighbors=10, adaptive_bw=True,
+                       use_angular=True, backend="sklearn", n_jobs=1)
+    b = compute_kernel(X, metric="euclidean", n_neighbors=10, adaptive_bw=True,
+                       use_angular=False, backend="sklearn", n_jobs=1)
+    np.testing.assert_allclose(a.toarray(), b.toarray())
+
+
+def test_cosine_kernel_weights_do_not_collapse():
+    """With consistent units the weights stay in a usable range."""
+    X = _toy()
+    W = compute_kernel(X, metric="cosine", n_neighbors=10, adaptive_bw=True,
+                       backend="sklearn", n_jobs=1)
+    assert np.isfinite(W.data).all()
+    # under the unit mismatch the mean weight fell to ~0.006
+    assert W.data.mean() > 0.05, f"kernel weights collapsed: {W.data.mean()}"
+
+
+def test_cosine_distance_to_angle_bounds():
+    d = np.array([0.0, 1.0, 2.0])
+    np.testing.assert_allclose(_cosine_distance_to_angle(d),
+                               [0.0, np.pi / 2, np.pi], atol=1e-12)
+
+
+@pytest.mark.parametrize("expand", [False, True])
+@pytest.mark.parametrize("adaptive", [True, False])
+def test_kernel_builds_for_all_bandwidth_configurations(expand, adaptive):
+    X = _toy()
+    W = compute_kernel(X, metric="cosine", n_neighbors=10, adaptive_bw=adaptive,
+                       expand_nbr_search=expand, backend="sklearn", n_jobs=1)
+    assert W.shape == (X.shape[0], X.shape[0])
+    assert np.isfinite(W.data).all()
+
+
+# ── #27 ───────────────────────────────────────────────────────────────────────
+try:  # nmslib is an optional backend and does not build everywhere
+    import nmslib  # noqa: F401
+    _HAS_NMSLIB = True
+except ImportError:
+    _HAS_NMSLIB = False
+
+requires_nmslib = pytest.mark.skipif(not _HAS_NMSLIB, reason="nmslib not installed")
+
+
+@requires_nmslib
+@pytest.mark.parametrize("dense", [True, False])
+@pytest.mark.parametrize("sparse_input", [True, False])
+def test_nmslib_space_matches_data_type(dense, sparse_input):
+    """A dense index must never be paired with a *_sparse space name."""
+    from topo.base.ann import NMSlibTransformer
+    X = _toy(n=120, p=15)
+    X = csr_matrix(X) if sparse_input else X
+    t = NMSlibTransformer(n_neighbors=10, metric="cosine", dense=dense, n_jobs=1).fit(X)
+    if dense:
+        assert "sparse" not in t.space
+    elif sparse_input:
+        assert "sparse" in t.space
+
+
+@requires_nmslib
+def test_knn_nmslib_accepts_dense_without_sparsifying():
+    """Dense input must not trigger a conversion warning, and must give the same graph."""
+    X = _toy(n=120, p=15)
+    import warnings
+    with warnings.catch_warnings(record=True) as caught:
+        warnings.simplefilter("always")
+        K_dense = kNN(X, metric="cosine", n_neighbors=10, backend="nmslib", n_jobs=1)
+    assert not [w for w in caught if "does not support dense" in str(w.message)]
+    K_sparse = kNN(csr_matrix(X), metric="cosine", n_neighbors=10,
+                   backend="nmslib", n_jobs=1)
+    assert K_dense.shape == K_sparse.shape

topo/base/ann.py

@@ -106,10 +106,9 @@ def kNN(X, Y=None,
             warn("Only the 'sklearn' backend supports Y. Falling back to 'sklearn'...")
             backend = 'sklearn'
     if backend == 'nmslib':
-        if isinstance(X, np.ndarray):
-            warn("nmslib does not support dense matrices. Converting to array...")
-            X = csr_matrix(X)
-        # Construct an approximate k-nearest-neighbors graph
+        # nmslib handles dense input natively (DataType.DENSE_VECTOR), so dense arrays are
+        # passed through rather than sparsified; NMSlibTransformer picks the matching
+        # space and data type from the input it is given.
         nbrs = NMSlibTransformer(n_neighbors=n_neighbors,
                                       metric=metric,
                                       p=p,
@@ -118,6 +117,7 @@ def kNN(X, Y=None,
                                       M=M,
                                       efC=efC,
                                       efS=efS,
+                                      dense=isinstance(X, np.ndarray),
                                       verbose=verbose).fit(X)
     elif backend == 'hnswlib':
         if issparse(X):
@@ -289,7 +289,9 @@ def fit(self, data):
         if self.n_jobs == -1:
             self.n_jobs = cpu_count()
 
-        self.space = {
+        # NOTE: the space name must match the index's data type, so it is chosen
+        # alongside it below (see `use_sparse_index`), not up front.
+        sparse_spaces = {
             'sqeuclidean': 'l2_sparse',
             'euclidean': 'l2_sparse',
             'cosine': 'cosinesimil_sparse_fast',
@@ -303,18 +305,20 @@ def fit(self, data):
             'bit_hamming': 'bit_hamming',
             'levenshtein': 'leven',
             'normleven': 'normleven'
-        }[self.metric]
+        }
         start = time.time()
         # see more metrics in the manual
         # https://github.com/nmslib/nmslib/tree/master/manual
         if self.metric == 'lp' and self.p < 1:
             print('Fractional L norms are slower to compute. Computations are faster for fractions'
                   ' of the form \'1/2ek\', where k is a small integer (i.g. 0.5, 0.25) ')
         if self.dense:
-            self.nmslib_ = nmslib.init(method=self.method,
-                                       space=self.space,
-                                       data_type=nmslib.DataType.DENSE_VECTOR)
-
+            # A dense index cannot consume sparse rows: each row's stored values would be
+            # read as the whole vector, so rows with differing nnz give differing lengths.
+            if issparse(data):
+                if self.verbose:
+                    print('Dense index requested for sparse input. Densifying...')
+                data = data.toarray()
         else:
             if issparse(data) == True:
                 if self.verbose:
@@ -330,7 +334,9 @@ def fit(self, data):
 
         index_time_params = {'M': self.M, 'indexThreadQty': self.n_jobs, 'efConstruction': self.efC, 'post': 2}
 
-        if issparse(data) and (not self.dense) and (not isinstance(data, np.ndarray)):
+        use_sparse_index = issparse(data) and (not self.dense) and (not isinstance(data, np.ndarray))
+        if use_sparse_index:
+            self.space = sparse_spaces[self.metric]
             if self.metric not in ['levenshtein', 'normleven', 'jansen-shan']:
                 if self.metric == 'lp':
                     self.nmslib_ = nmslib.init(method=self.method,

topo/eval/__init__.py

@@ -1,3 +1,7 @@
 from .global_scores import global_score_pca, global_score_laplacian
 from .local_scores import knn_spearman_r, knn_kendall_tau, geodesic_distance, geodesic_correlation
+# Re-exported for backwards compatibility: TopOMetry's own implementation was removed
+# in favour of scikit-learn's in 4974acf5 (2023-07-05), but `topo.eval.trustworthiness`
+# had been documented and used, so the name is kept available here.
+from sklearn.manifold import trustworthiness
 from .rmetric import RiemannMetric, get_eccentricity

topo/layouts/projector.py

@@ -318,8 +318,25 @@ def fit(self, X, **kwargs):
                 _HAS_MCTSNE = False
             if not _HAS_MCTSNE:
                 from sklearn.manifold import TSNE
-            self.estimator_ = TSNE(n_components=self.n_components,
-                                   metric='precomputed', n_iter=self.num_iters)
+            # `metric` must follow the input actually handed to this Projector.
+            # TopOGraph.project() passes eigenbasis *coordinates* with metric=graph_metric
+            # for coordinate-based methods, so hardcoding 'precomputed' here made t-SNE
+            # interpret coordinates as a square distance matrix.
+            if _HAS_MCTSNE:
+                self.estimator_ = TSNE(n_components=self.n_components,
+                                       metric=self.metric, n_iter=self.num_iters)
+            else:
+                # scikit-learn >= 1.2 defaults to init='pca', which is rejected when
+                # metric='precomputed'. The spectral initialization TopOMetry already
+                # computed is valid for both cases and is what the other projections use.
+                # `n_iter` was renamed `max_iter` in scikit-learn 1.5 and removed in 1.7.
+                from inspect import signature as _signature
+                _iter_kw = ('max_iter' if 'max_iter' in _signature(TSNE.__init__).parameters
+                            else 'n_iter')
+                self.estimator_ = TSNE(n_components=self.n_components,
+                                       metric=self.metric,
+                                       init=self.init_Y_,
+                                       **{_iter_kw: self.num_iters})
             self.Y_ = self.estimator_.fit_transform(X)
 
         elif self.projection_method == 'MAP':

topo/pipes.py

@@ -155,7 +155,7 @@ def eval_models_layouts(TopOGraph, X,
     import numpy as np
     if issparse(X) == True:
         if isinstance(X, csr_matrix):
-            data = X.todense()
+            data = X.toarray()
             gc.collect()
     if issparse(X) == False:
         if not isinstance(X, np.ndarray):

topo/topograph.py

@@ -3191,7 +3191,7 @@ def eval_models_layouts(self, X,
             print('Computing PCA for comparison...')
         if issparse(X) is True:
             if isinstance(X, csr_matrix):
-                data = X.todense()
+                data = X.toarray()
             else:
                 data = X
         else:

topo/tpgraph/kernels.py

@@ -44,18 +44,38 @@ def _cosine_knn_requires_unit_vectors(backend: str) -> bool:
     """Backends that expect unit-norm vectors for 'cosine' space."""
     return backend in ('hnswlib', 'faiss')
 
-def _cosine_distance_to_angle_from_sparse_triplets(x_idx, y_idx, dists):
+def _cosine_distance_to_angle(dists):
     """
-    Given triplets of cosine *distance* d = 1 - cos in [0, 2],
-    convert to angle θ = arccos(cos) with cos = 1 - d.
-    Returns in-place modified dists (angles in radians).
+    Convert cosine *distance* d = 1 - cos in [0, 2] to angle θ = arccos(cos) in [0, pi].
     """
-    # cos = 1 - d
-    # clamp to [-1, 1] before arccos
-    cos_vals = 1.0 - dists
-    cos_vals = np.clip(cos_vals, -1.0, 1.0)
+    # cos = 1 - d; clamp to [-1, 1] before arccos
+    cos_vals = np.clip(1.0 - dists, -1.0, 1.0)
     return np.arccos(cos_vals)
 
+
+def _angularize_graph(K, metric, use_angular):
+    """
+    Return a copy of the kNN graph `K` with its stored cosine distances replaced by
+    angles, or `K` itself when no conversion applies.
+
+    The adaptive bandwidth is derived from the graph's stored distances, so the
+    conversion has to happen *before* `_adap_bw` is called -- otherwise the bandwidth
+    is in cosine-distance units while the distances it normalizes are in radians.
+    """
+    if metric == 'cosine' and use_angular:
+        K_ang = K.copy()
+        K_ang.data = _cosine_distance_to_angle(K_ang.data)
+        return K_ang
+    return K
+
+
+def _cosine_distance_to_angle_from_sparse_triplets(x_idx, y_idx, dists):
+    """
+    Deprecated: kept for backwards compatibility. Prefer `_cosine_distance_to_angle`.
+    `x_idx` and `y_idx` are unused.
+    """
+    return _cosine_distance_to_angle(dists)
+
 def _ensure_nonneg_and_finite(arr, eps=0.0):
     arr = np.where(np.isfinite(arr), arr, 0.0)
     if eps > 0.0:
@@ -163,6 +183,7 @@ def compute_kernel(X, metric='cosine',
     adap_sd_new = None
     pm_new = None
     new_K = None
+    dists_new = None
     if n_jobs == -1:
         from joblib import cpu_count
         n_jobs = cpu_count()
@@ -216,8 +237,12 @@ def compute_kernel(X, metric='cosine',
             dens_dict['unweighted_adjacency'] = A
             dens_dict['adaptive_bw'] = adap_sd
     else:
+        # Work in a single distance convention from here on: if angular distances are
+        # requested, convert the graph up front so that the adaptive bandwidth and the
+        # distances it normalizes are in the same units (radians).
+        K_scaled = _angularize_graph(K, metric, use_angular)
         if adaptive_bw:
-            adap_sd = _adap_bw(K, k)
+            adap_sd = _adap_bw(K_scaled, k)
             # Get an indirect measure of the local density
             pm = np.interp(adap_sd, (adap_sd.min(), adap_sd.max()), (2, k))
             if return_densities:
@@ -227,8 +252,9 @@ def compute_kernel(X, metric='cosine',
                 new_k = int(k + (k - pm.max()))
                 new_K = kNN(X, metric=metric, n_neighbors=new_k,
                             backend=backend, n_jobs=n_jobs, **kwargs)
-                adap_sd_new = _adap_bw(new_K, new_k)
-                x_new, y_new, dists_new = find(new_K)
+                new_K_scaled = _angularize_graph(new_K, metric, use_angular)
+                adap_sd_new = _adap_bw(new_K_scaled, new_k)
+                x_new, y_new, dists_new = find(new_K_scaled)
                 # Get an indirect measure of the local density
                 pm_new = np.interp(
                     adap_sd_new, (adap_sd_new.min(), adap_sd_new.max()), (2, new_k))
@@ -238,11 +264,7 @@ def compute_kernel(X, metric='cosine',
                     dens_dict['adaptive_bw_nbr_expanded'] = adap_sd_new
                     dens_dict['expanded_neighborhood_graph'] = new_K
                     dens_dict['knn_expanded'] = new_K
-        x, y, dists = find(K)
-
-        # If using cosine metric and 'use_angular', convert cosine distance (=1-cos) to angle (radians)
-        if metric == 'cosine' and use_angular:
-            dists = _cosine_distance_to_angle_from_sparse_triplets(x, y, dists)
+        x, y, dists = find(K_scaled)
 
         # Numerical guards for distances (important for arccos and exponent)
         # For cosine distance we expect [0, 2]; for angles [0, pi]; Euclidean ≥ 0.
@@ -252,6 +274,13 @@ def compute_kernel(X, metric='cosine',
             dists = np.clip(dists, 0.0, np.pi)
         else:
             dists = np.maximum(dists, 0.0)
+        if expand_nbr_search and dists_new is not None:
+            if metric == 'cosine' and not use_angular:
+                dists_new = np.clip(dists_new, 0.0, 2.0)
+            elif metric == 'cosine' and use_angular:
+                dists_new = np.clip(dists_new, 0.0, np.pi)
+            else:
+                dists_new = np.maximum(dists_new, 0.0)
         # Normalize distances
         if adaptive_bw:
             # Alpha decaying: the kernel adaptively decays depending on neighborhood density