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

Removed layers, simplified architectures #11

Merged
merged 11 commits into from
Feb 4, 2021
Merged

Removed layers, simplified architectures #11

merged 11 commits into from
Feb 4, 2021

Conversation

DomInvivo
Copy link
Collaborator

  • Removal of unused GNNs
  • Creation of ResidualConnection classes to avoid having multiple architectures. Then removed the architectures that relied on these residual connections
  • Added some documentation and unit-tests
  • Refactor PNA to have inheritance and avoid duplicated code
  • Removed duplicated MLPs functions
  • Changed the DGN networks to support both an input and output FeedForward network

This pull-request has a lot of progress in the refactoring of the code, but there is still a lot to do.
The examples will no longer run after the merging, but I want to avoid pull-requests too large, so I think we can still go forward with the PR and do the remaining changes in another PR.

@DomInvivo DomInvivo requested a review from hadim February 2, 2021 23:17
@DomInvivo DomInvivo added the enhancement New feature or request label Feb 2, 2021
Copy link
Contributor

@hadim hadim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Dom for this first cleanup. I haven't run the code as it is mainly a cleaning and refactoring PR.

I am often picky and opinionated when reviewing so feel free to disagree or comment more on my reviews.

The platform also switched to black as a Python code formatted. There is a GA setup to check each commit has formatted code (see https://github.com/invivoai/goli/blob/master/.github/workflows/lint.yml#L31). black is pretty standard and you should be able to configure it on your favourite IDE.

goli/dgl/base_layers.py Outdated Show resolved Hide resolved
goli/dgl/base_layers.py Outdated Show resolved Hide resolved
goli/dgl/base_layers.py Show resolved Hide resolved
goli/dgl/dgl_layers/pna_layer.py Show resolved Hide resolved
goli/expts/config_gnns.yaml Show resolved Hide resolved
@DomInvivo DomInvivo merged commit a5a659a into master Feb 4, 2021
@DomInvivo DomInvivo deleted the GNN-cleaning branch February 4, 2021 18:08
DomInvivo pushed a commit that referenced this pull request Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants