`SCConv` needs a full revision

pyt-team / TopoModelX

Topological Deep Learning

MIT License

219 stars 79 forks source link

`SCConv` needs a full revision #253

Open gbg141 opened 9 months ago

gbg141 commented 9 months ago

Two important concerns about SCConv simplicial model:

[ ] Implementation of normalization: The original paper consider normalized incidence and adjacency matrices, but the proposed normalization procedure in the tutorial does not work. Currently, the model runs without any normalization.
[ ] Improving readability: The notation of incidence and adjacency matrices is difficult to follow; this needs to be carefully revised for the model to be further used.

mhajij commented 9 months ago

@gbg141 I have started point 1 above,

https://github.com/pyt-team/TopoNetX/pull/176

please take a look at that PR, and let me know if other normalization are needed. I am going to work on the suggestions in that PR in there today and if you have more suggestions let me know.

gbg141 commented 9 months ago

Really nice and practical addition to TopoNetX! For sure point 1 can be easily solved by making use of the implemented normalizations (in fact, compute_bunch_normalized_matrices precisely does the desired transformation, SCConv is the model defined in that very same paper).

No further suggestions from my side.