Skip to content

Conversation

@Pentaflouride
Copy link

Implemented a multilayered GCN for task 2 (using the the facebook dataset). The driver.py file runs by simply running the file without any parameters. The output will be the model training, the model getting tested, a graph of the accuracy and loss of the validation data set and a tsne of the embedded data set classified.
The readme file is as described in the task.
The model.py file has the model wrapped in a function.
The notebook file has more raw outputs which were omitted from the model.py file. Things like shapes and types of data.
The rest of the files are either images (so they can be added to the readme file) or the data itself (so I can load it in my script).
I've taken inspirations from 2 blog posts. One made by Anton Ruberts and huonw

Name: Paul Turculetu
Student number: 45249435
Course: COMP3710

Kind regards,
Paul

@gayanku
Copy link
Collaborator

gayanku commented Nov 17, 2021

  • Submitted on Oct 27

  • Commits' over 3 days (25 to 27)

  • Good details in commits and PR.

  • Readme is well done with background, results, diagrams.

  • multi layer (3) layer GCN, Good accuracy (+85%), Semi supervised split.

  • Valid for EASY (subject to not implementing own GCN model).

  • You need to implement your own GCN (convolution layers and model) rather than use the GCN imported from stellargraph.layer which is a 3rd party library.

  • In future, code structure and modularization can be improved by moving the preprocessing ( and creating normalized Adj) and TSNE visualization outside of the main learning code (train, val and test). Your use of a single composite function to do everything can be somewhat less flexible for from a code re-sue and maintenance perspective at times.

  • Your diver file is also redundant as it only calls run_model() which contains all your code.

@shakes76
Copy link
Owner

TF/Torch Usage

Adequate use and implementation

Good Practice (Design/Commenting)

Good spacing and comments
Header blocks missing -1

Algorithm

Driver Script present
Shows Usage & Demo & Visualisation & Data usage
Module NOT present (all in one function) -1
No Data leakage
Difficulty: Easy -10

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

I can't delete model.py since that is what the driver file calls. I'm assuming that the notebook was supposed to be deleted. The images are there for the README file.
@Pentaflouride
Copy link
Author

Deleted the notebook file since it did not do anything. I cannot delete model.py because driver.py runs model.py. I also cannot get rid of the images because they are used in the README file. The other files are data files.

@shakes76
Copy link
Owner

Data files (and CSV files) should not be in the repository, the .py files are fine. Please remove the data files.

@shakes76
Copy link
Owner

Feedback marks awarded. Still can't merge though because of the .npy file present.

@Pentaflouride
Copy link
Author

Pentaflouride commented Nov 23, 2021 via email

@shakes76 shakes76 merged commit bf86e8c 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