Skip to content

Conversation

@shunyuLiu
Copy link

Hi Shakes and team,

This is my work using PyTorch to finish GCN model to carry out a semi supervised multi-class node classification using Facebook Large Page-Page Network dataset.
gnc.py: main script, trains the model and evaluates it
model.py: contains load data and GCN layers
facebook.npz contains all files
README.md: Description of my work

Thanks for a great course, look forward to any feedback.
Kind Regards,
shunyu Liu

@gayanku
Copy link
Collaborator

gayanku commented Nov 17, 2021

  • PR on Nov 1

  • Meaningful commit and PR messages

  • Readme OK with background, instructions.

  • multi layer (3) layer GCN.

  • Code looks ok and with no obvious problems.

  • Valid for NORMAL (subject to no results/ TSNE)

  • No accuracy reported or submitted. No TSNE diagram submitted.

  • Visualization image links in the readme are broken. Did you forget git -add?

  • Semi supervised split (140/300/1000) does not fully use the dataset.

  • code structure and modularisation is bit poor. Model code mixed with load_data() might make re-use difficult, cluttered. In future keep Layers and Models in a sperate file.

  • You should add .DS_Store to .gitignore so they don't get committed

  • Keep all your import statements in one place in a file

  • Use a driver file and a main()

@shakes76
Copy link
Owner

shakes76 commented Nov 22, 2021

TF/Torch Usage

Adequate use and implementation

Good Practice (Design/Commenting)

Good spacing and comments
Header blocks

Algorithm

Driver Script present
Shows Usage & Demo & Visualisation & Data usage
Module present
No Data leakage
Difficulty: Normal -5

Commit Log

Meaningful commit messages
Progressive commits used

Documentation

Good Description and Comments
Markdown used PDF submitted

Pull Request

Successful Pull Request (Working Algorithm Delivered on Time in Correct Branch)
Please remove model files, these will clog up the repository. Your pull request will not be merged until they are removed. Feedback marks will be awarded when these are removed. -2
Request Description OK

Remove model.py to avoid clog up the repository
@shunyuLiu
Copy link
Author

New version updated: delete mode files

@shakes76
Copy link
Owner

Not the model.py file, which is important, but delete the model weights (.npz file) which is 36 MB. Please revert the model.py deletion and delete the .npz file before I can merge. Feedback attempted so will award marks though.

@shakes76 shakes76 merged commit 209a606 into shakes76:topic-recognition Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants