Code Monkey home page Code Monkey logo

Comments (4)

josefhoppe avatar josefhoppe commented on September 17, 2024

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 from CombinatorialComplex except for s. 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
  • 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 the adjacency_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'?)
  • k_hop_coincidence_matrix: doesn't work as coincidence_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 rank None, doesn't return anything and no error for rank None -> Easy to accidentally use incorrectly, like in get_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.

josefhoppe avatar josefhoppe commented on September 17, 2024

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)

@mhajij @ninamiolane

from toponetx.

mhajij avatar mhajij commented on September 17, 2024

I agree with you. Let’s remove it.

from toponetx.

mhajij avatar mhajij commented on September 17, 2024

I’ll discuss the bugs here later today in details.

from toponetx.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo 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.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.