Comments (4)
After looking into it, I'm not sure what is useful and what isn't.
It feels a bit like a lot of the code was copied from CombinatorialComplex: For example, graph connectivity has a parameter s
for how connected cells need to be
- This also breaks a lot of stuff since the
s
parameter is passed to functions that don't support it - This also brings different implementations of adjacency matrix (one general, three seperate for node, edge, cell)
CellComplex._incidence_to_adjacency()
is copied 1:1 fromCombinatorialComplex
except fors
. Maybe generalize and move it to a utility package?
I also have some minor remarks where I'm not sure what the best way to proceed is:
-
CellComplex.incidence_matrix()
for nodes returns a$1\times n$ matrix instead of$0\times n$ (seems incorrect, but maybe something depends on this behavior?)- with
index=True
, it returns an empty row index list, but matrix has one row. - with
index=True
it also returns the indices in the wrong (inconsistent with higher ranks) order
- with
- Interesting Property of the CellComplex Laplacian (may be correct, but unexpected): If two cells
(1,2,3,4)
and(1,2,4,3)
exist,(1,2)
and(3,4)
induce the same orientation on one cell and opposite orientations on the other. This means the laplacian will have an entry of$0 (=-1 + 1)$ for these two edges.- In
CellComplex
, this also affects theadjacency_matrix
as it takes the laplacian instead of using the square of the absolute of the incidence matrix. (would the correct adjacency then be 2 for twice adjacent or 1 for 'yes, adjacent'?)
- In
-
k_hop_coincidence_matrix
: doesn't work ascoincidence_matrix
doesn't exist. (also, I'm not sure why we have it. It's just the coincidence of the k-hop (lower and upper) adjacency, isn't it? the last step seems trivial enough to leave it to the user unless the overall concept is very important) -
get_cell_attributes
: default rankNone
, doesn't return anything and no error for rankNone
-> Easy to accidentally use incorrectly, like inget_filtration
(see my bugfix in 0d096a4). I suggest to remove the default value.
Finally, I found some things that are straightforward bugs (I just listed them so I won't forget about it):
-
CellComplex.restrict_to_cells()
discards attributes
from toponetx.
CellComplex.k_hop_coincidence_matrix
is currently not working since CellComplex.coincidence_matrix
does not exist. Since I don't understand its purpose: Do we need it?
It seems relatively trivial to compute for the user, so unless it is frequently needed for something I'd suggest we remove it (along with CellComplex.k_hop_incidence_matrix
; but that one currently works)
from toponetx.
I agree with you. Let’s remove it.
from toponetx.
I’ll discuss the bugs here later today in details.
from toponetx.
Related Issues (20)
- Edge Features of COSEG
- remove_singletons in ColoredHyperGraph throws error. HOT 2
- Unreachable code in ColoredHyperGraph, ColoredHyperGraphView, and CombinatorialComplex
- Switch from sparse matrices to sparse arrays
- Add `CITATION.cff` file after JMLR submission
- Breaking downstream change HOT 3
- Clarification of order-preserving rule for CCC HOT 1
- Definition of CCC and the logic that follows from it. HOT 11
- Package is breaking because of python version 3.12.0 HOT 2
- pyproject.toml not enforcing build constraints
- Docstrings Missing HOT 1
- Changing lint and formatter HOT 6
- Import Convention for TopoNetX HOT 1
- Invalid return type for disconnected elements in `algorithms.distance` module
- OFF files to NPZ files
- Typos and errors in [01_simplicial_complexes.ipynb] HOT 2
- Wrong documentation on function incidence_to_adjacency
- Orientation of simplicial complexes HOT 1
- Make `gudhi` dependency optional
- Deprecations to be removed in 0.2.0
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from toponetx.