-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A simplicial complex has dimension equal to the size of its maximal s… #336
Conversation
…et minus one. If, for example, a simplicial complex has dimension one, then, it has simplices of at most two vertices, with at least one having exactly two vertices. Currently, the method was graph_to_clique_complex was considering that the dimension of a simplex is exactly the number of vertices in it. This commit solves this problem. Also, it only copies the edge attributes from the networkx graph if the resulting simplicial complex has dimension greater or equal than one.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #336 +/- ##
==========================================
+ Coverage 97.36% 97.40% +0.04%
==========================================
Files 38 38
Lines 3561 3545 -16
==========================================
- Hits 3467 3453 -14
+ Misses 94 92 -2 ☔ View full report in Codecov by Sentry. |
I'm on the fence on this one. The dimension that is meant here is the sizes of the cliques, not of the simplicial complex. We call the later one rank throughout the code base. The direction to go would be to either
|
As I user, I would expect max_dim to correspond to the dim of the object I am generating. Is there any other use of the term dimension for graphs that coincides with the length and not the length minus one in the literature? @mhajij By the way, the pull request is also solving a problem when the user is requesting to generate a point cloud. It is not a usual thing, but it can happen. |
@rballeba @ffl096 TopoNetX/toponetx/classes/cell_complex.py Line 229 in 7b8fa8b
whereas rank identifies the "dimension" of a particular skeleton in the larger complex. In other words : 0=< rank<= max_dim |
@USFCA-MSDS Therefore, this means that we can merge this pull request right? @ffl096 |
My point above still stands: If we want to change the behaviour, the parameter must be renamed. As of right now, this is a hidden break of any consumer code (existing code will run without any error but with changed result!). The docstring should be updated as well, it still describes the current behaviour. |
@ffl096 I see this in the docstring:
This is what was wrong before. Could you point me what should I change from the docstring? Regarding changing the code. I'm not a usual maintainer, so I would prefer to avoid larger changes in the codebase. If you think that this pull request does not fulfill the needs of the project, we can discard it. Either way, if we discard this modification, we should remark that max_dim is no the maximum dimension of the simplicial complex. |
It was not. It specifically talks about the dimensions of cliques. I see that this may be confusing and hence I am not opposed to this change. However, if the function should be about the simplex rank, this has to be stated there.
Not sure what you mean by larger changes to the codebase? Its just about renaming the parameter. |
@ffl096 I have not found any standard definition of dimension of a clique. Could you point to any reference where this term is used explicitly? If we see the clique as a simplex, then it has dimension equal to the number of vertices minus one. What I have found is the size of a clique, defined as its number of vertices https://mathworld.wolfram.com/Clique.html About renaming the parameter, you mean renaming the parameter of the function or all the parameters related to the dimension of cliques? |
Rename |
A simplicial complex has dimension equal to the size of its maximal set minus one. If, for example, a simplicial complex has dimension one, then, it has simplices of at most two vertices, with at least one having exactly two vertices. Currently, the method graph_to_clique_complex was considering that the dimension of a simplex is exactly the number of vertices in it. This commit solves this problem. Also, it only copies the edge attributes from the networkx graph if the resulting simplicial complex has dimension greater or equal than one.