Fix fencepost errors in uniform_erdos_renyi_hypergraph (#653)

* Fixed fencepost errors in uniform_erdos_renyi_hypergraph and added test case that orginally alerted us to error.

* added `fast_random_hypergraph`

---------

Co-authored-by: Nicholas Landry <[email protected]>

benchmarks/generators.py

@@ -10,3 +10,11 @@ def erdos_renyi():
         xgi.random_hypergraph(100, [0.1, 0.001])
 
     benchmark.pedantic(erdos_renyi, rounds=rounds, iterations=iterations)
+
+
+def test_fast_erdos_renyi(benchmark):
+
+    def fast_erdos_renyi():
+        xgi.fast_random_hypergraph(100, [0.1, 0.001])
+
+    benchmark.pedantic(fast_erdos_renyi, rounds=rounds, iterations=iterations)

tests/generators/test_uniform.py

@@ -197,3 +197,10 @@ def test_uniform_erdos_renyi_hypergraph():
         n = 10
         k = 2
         xgi.uniform_erdos_renyi_hypergraph(n, m, k, p_type="test")
+
+    # test specific case that caused failure previously
+    n = 200
+    k = 2
+    p = 0.0185
+    s = 5
+    xgi.uniform_erdos_renyi_hypergraph(n, k, p, p_type="prob", seed=s)

tests/linalg/test_matrix.py

@@ -257,21 +257,26 @@ def test_adjacency_matrix(edgelist1, edgelist4):
     assert np.all(A7 == A7_sp.todense())
 
 
-def test_fix_649(): # make sure diagonal is fully zero when sparse
+def test_fix_649():  # make sure diagonal is fully zero when sparse
 
     H = xgi.load_xgi_data(dataset="email-enron")
-    H.cleanup(isolates=True, singletons=False, connected=True, relabel=False, multiedges=False)
+    H.cleanup(
+        isolates=True, singletons=False, connected=True, relabel=False, multiedges=False
+    )
 
     weighted = True
     order = 1
 
-    adj = xgi.adjacency_matrix(H, order=order, sparse=False, weighted=weighted, index=False)
+    adj = xgi.adjacency_matrix(
+        H, order=order, sparse=False, weighted=weighted, index=False
+    )
 
-    adj_sparse = xgi.adjacency_matrix(H, order=order, sparse=True, weighted=weighted, index=False)
+    adj_sparse = xgi.adjacency_matrix(
+        H, order=order, sparse=True, weighted=weighted, index=False
+    )
     adj_sparse = adj_sparse.todense()
 
-    assert np.all(np.diag(adj_sparse)==0)
-
+    assert np.all(np.diag(adj_sparse) == 0)
 
 
 def test_laplacian(edgelist2, edgelist6):

xgi/generators/uniform.py

@@ -393,10 +393,10 @@ def uniform_erdos_renyi_hypergraph(n, m, p, p_type="prob", multiedges=False, see
     H.add_nodes_from(range(n))
 
     if multiedges:
-        max_index = n**m
+        max_index = n**m - 1
         f = _index_to_edge_prod
     else:
-        max_index = comb(n, m, exact=True)
+        max_index = comb(n, m, exact=True) - 1
         f = _index_to_edge_comb
 
     index = np.random.geometric(q) - 1  # -1 b/c zero indexing
@@ -461,6 +461,12 @@ def _index_to_edge_prod(index, n, m):
     ----------
     https://stackoverflow.com/questions/53834707/element-at-index-in-itertools-product
     """
+    if index < 0 or index >= n**m:
+        warnings.warn(
+            f"_index_to_edge_prod was given index {index} >= {n}**{m}={n**m}."
+            f"Returned hyperedge ({ [(index // (n**r) % n) for r in range(m - 1, -1, -1)] }) may not be correct."
+        )
+
     return [(index // (n**r) % n) for r in range(m - 1, -1, -1)]
 
 
@@ -501,6 +507,11 @@ def _index_to_edge_comb(index, n, m):
     ----------
     https://math.stackexchange.com/questions/1227409/indexing-all-combinations-without-making-list
     """
+    if index < 0 or index >= comb(n, m, exact=True):
+        warnings.warn(
+            f"index {index} >= comb({n}, {m}, exact=True) = {comb(n, m,exact=True)}."
+        )
+
     c = []
     r = index + 1  # makes it zero indexed
     j = -1

xgi/linalg/laplacian_matrix.py

@@ -50,11 +50,7 @@
 from scipy.sparse import csr_array, diags_array, eye_array
 
 from ..exception import XGIError
-from .hypergraph_matrix import (
-    adjacency_matrix,
-    degree_matrix,
-    incidence_matrix,
-)
+from .hypergraph_matrix import adjacency_matrix, degree_matrix, incidence_matrix
 
 __all__ = [
     "laplacian",