Skip to content
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

Clarify duplicate implementation of SCN/SCN2 #160

Closed
ninamiolane opened this issue Aug 24, 2023 · 3 comments
Closed

Clarify duplicate implementation of SCN/SCN2 #160

ninamiolane opened this issue Aug 24, 2023 · 3 comments
Assignees

Comments

@ninamiolane
Copy link
Collaborator

ninamiolane commented Aug 24, 2023

What?

There were two implementations of SCN by challenge participants:

  • SCN2 only works for simplicial complexes of rank 2, and
  • SCN works for simplicial complexes of any rank.

However, we should check that the difference between these two implementations only lies in the ranks used.

Why?

Because it is confusing to have two very close implementations of SCN without knowing their precise difference.

Where?

The files to modify are:

  • topomodelx/nn/simplicial/scn_layer.py
  • topomodelx/nn/simplicial/scn2_layer.py
  • test/nn/simplicial/test_scn_layer.py
  • test/nn/simplicial/test_scn2_layer.py
  • tutorials/simplicial/scn_train.ipynb
  • tutorials/simplicial/scn2_train.ipynb

How?

We should look at the following:

  • Do they use the same neighborhood matrices?
    • If not, the docstrings' sections "See Also" should explain additional differences in more details.
  • Do they use the same activation?
  • etc.
@georg-bn
Copy link
Contributor

Hi, I'm one of the implementers of SCCN.

I believe that SCN and SCCN are two different models from Yang et al 2022. There is some confusion in that they are both labelled SCN in Figure 11 of the Architectures of Topological Deep Learning paper. SCCN should be the leftmost "SCN" in that figure.

@ninamiolane
Copy link
Collaborator Author

ninamiolane commented Aug 25, 2023

Thank you very much for stepping in! This is incredibly helpful. I see the two models in Yang et al. 2022 and the confusion arising from their misnaming in Figure 11. Thus:

  • SCN: Simplex Convolutional Network (rightmost Yang22c in figure 11): happens to be implemented up to rank 2 in topomodelx.
  • SCCN: Simplicial Complex Convolutional Network (leftmost Yang22c in figure 11)

Thus, TODOs for us:

  • Rename the leftmost SCN of Figure 11 of the Architectures of Topological Deep Learning paper into SCCN.
  • Rename the files of scn_layer -> sccn_layer.py, test_scn_layer.py -> test_sccn_layer.py, scn_train.ipynb -> sccn_train.ipynb

@ninamiolane
Copy link
Collaborator Author

We will update the Figure 11 offline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants