Method for k-skeleton of a SC

[thomasrobiglio]

I noticed that #178 has been around for a while and thought it would be a nice way to get back in the PR game.

[thomasrobiglio]

I originally planned to do something simpler like:

        if order != max_order:
             bunch = _H.edges.filterby('order', order, 'gt')
             _H.remove_simplex_ids_from(bunch)

but I have noticed that remove_simplex_ids() fails when you remove all the simplices at the same time e.g.:

>>> tetra = xgi.SimplicialComplex([[1,2,3,4]])
>>> bunch = tetra.edges.filterby("order", 0, "gt")
>>> tetra.remove_simplex_ids_from(bunch)
Traceback (most recent call last):
  File "/Users/thomasrobiglio/GitHub/xgi/xgi/utils/utilities.py", line 40, in __getitem__
    return dict.__getitem__(self, item)
KeyError: 4

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/thomasrobiglio/GitHub/xgi/xgi/core/simplicialcomplex.py", line 749, in remove_simplex_id
    supfaces_ids = self._supfaces_id(self._edge[id])
  File "/Users/thomasrobiglio/GitHub/xgi/xgi/utils/utilities.py", line 42, in __getitem__
    raise IDNotFound(f"ID {item} not found") from e
xgi.exception.IDNotFound: 'ID 4 not found'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/thomasrobiglio/GitHub/xgi/xgi/core/simplicialcomplex.py", line 779, in remove_simplex_ids_from
    self.remove_simplex_id(id)
  File "/Users/thomasrobiglio/GitHub/xgi/xgi/core/simplicialcomplex.py", line 757, in remove_simplex_id
    raise XGIError(f"Simplex {id} is not in the Simplicialcomplex") from e
xgi.exception.XGIError: Simplex 4 is not in the Simplicialcomplex

let me know if this is an expected behavior or not, in case I will open an issue.

[maximelucas]

Good catch I think it's a bug.

Worth checking, but after a first quick look, the first 5 edges are:

{0: frozenset({1, 2, 3, 4}),
 1: frozenset({2, 4}),
 2: frozenset({1, 2}),
 3: frozenset({3, 4}),
 4: frozenset({1, 2, 3}),
 ...}

and I think it fails when trying to remove edge 4: frozenset({1, 2, 3}) which does not exist anymore already because it already removed 2: frozenset({1, 2}), and in doing so had to remove 4: frozenset({1, 2, 3}) already to ensure downward closure.

If that's really the case, we just have to catch it or remove edges already removed from the list of edges to remove.
Can you open an issue?

[maximelucas]

In a new paragraph maybe define in a few words what the k-skeleton is, with a reference?

[maximelucas]

Suggested change

	max_order = max(len(edge) for edge in _H._edge.values()) - 1
	max_order = xgi.max_edge_order(_H)

[thomasrobiglio]

I don't think that I can do from ..algorithms.properties import max_edge_order in this module because this would lead to a circular import... this probably hints in the direction of having the function in the convert module then

[maximelucas]

Ah fair, but which part would be circular you think? algorithms.properties doesn't import Simplicialcomplex I think. Worth checking maybe. I think there should be a way of using it there.

[thomasrobiglio]

xgi/tests/conftest.py:6: in <module>
    import xgi
xgi/xgi/__init__.py:1: in <module>
    from . import (
xgi/xgi/core/__init__.py:4: in <module>
    from .simplicialcomplex import SimplicialComplex
xgi/xgi/core/simplicialcomplex.py:19: in <module>
    from ..algorithms.properties import max_edge_order
xgi/xgi/algorithms/__init__.py:1: in <module>
    from . import properties, assortativity, centrality, clustering, connected
xgi/xgi/algorithms/centrality.py:10: in <module>
    from ..convert import to_line_graph
xgi/xgi/convert/__init__.py:1: in <module>
    from . import (
xgi/xgi/convert/bipartite_graph.py:7: in <module>
    from ..generators import empty_hypergraph
xgi/xgi/generators/__init__.py:1: in <module>
    from . import (
xgi/xgi/generators/simplicial_complexes.py:15: in <module>
    from ..core import SimplicialComplex

[maximelucas]

So if order==max_order we do nothing, and if order<max_order then we remove all simplices with d>order right? I don't remember how the k-skeleton is defined heh

[thomasrobiglio]

yes exactly:

The k-skeleton of a simplicial complex is the subcomplex containing all the simplices of the original complex of dimension at most k.

[maximelucas]

Ok so your code looks good but I just realised: this is just like this general recipe we had also for hypergraphs right?

Should we maybe make that into a general function for hypergraphs?
Now the terminology k-skeleton is usually used for simplicial complexes (right?), so either we call the general one something else, or we have a second one for SC that calls the first or something. What do you think?

[thomasrobiglio]

Yes, from my understanding the whole skeleton business is from algebraic topology and thus refers only to SCs... still you're right that this is doing the same as the recipe + it could be applied in the same way to hypergraphs.

If I remember well up to a while ago the to_graph was called something like skeleton... applying the same line of thought we could:

1. Have a general function cut_to_order or some similar name that works for SCs and HGs (located in xgi.convert.higher_order_network? @nwlandry)
2. Specify in the documentation that in the case of a SC this is know as k-skeleton
3. Update the existing recipe with the new function

[nwlandry]

Yes, from my understanding the whole skeleton business is from algebraic topology and thus refers only to SCs... still you're right that this is doing the same as the recipe + it could be applied in the same way to hypergraphs.

If I remember well up to a while ago the to_graph was called something like skeleton... applying the same line of thought we could:

1. Have a general function cut_to_order or some similar name that works for SCs and HGs (located in xgi.convert.higher_order_network? @nwlandry)
2. Specify in the documentation that in the case of a SC this is know as k-skeleton
3. Update the existing recipe with the new function

The skeletons in XGI's closet....

I like the idea of cut_to_order in higher-order network, and maybe we could have k_skeleton in simplex 1) call the cut_to_order function and 2) explicitly check that the input network is an SC. Since it's in the convert module, I would advocate that the in_place method not be supported.

This all sounds like a good plan!

[thomasrobiglio]

thank you @maximelucas and @nwlandry for the feedback, I will implement everything tomorrow/friday.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[nwlandry]

Is this ready for re-review, @thomasrobiglio?

[thomasrobiglio]

Yes! Thank you @nwlandry

[nwlandry]

Looks awesome, @thomasrobiglio! Ready to merge! Thanks so much!