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

Topic recognition #469

Open
wants to merge 63 commits into
base: topic-recognition
Choose a base branch
from
Open

Conversation

4lon
Copy link

@4lon 4lon commented Oct 21, 2022

Hello Teaching Team,

This is a request to merge my attempt at VQVAE using OASIS brain dataset.

Let me know if there are any problems.

Regards,
Alon Nusem - 44801582

…, reimplemented the dataset, however this current implementation is loading every image into an array, that's far from ideal so refactoring would be good time permitting.
…rall however time is limited. If I have time I intend to work back to this and improve the model
…gainst other data partitions or save sample outputs.
… dictionary, and updated directory to dataset.
@SiyuLiu0329
Copy link
Collaborator

This is an initial inspection, no action is required at this point

  • recon: OK
  • image gen: Model seems to be working with minor issues
  • code could use more comments

@shakes76
Copy link
Owner

Good Practice (Design/Commenting, TF/Torch Usage)

Adequate use and implementation
Good spacing and comments
Header blocks missing -1

Recognition Problem

Solves problem (poor generations) -1
Driver Script present
File structure present
Shows Usage & Demo & Visualisation & Data usage
Module present
Commenting could be more informative -1
No Data leakage
Difficulty: Hard

Commit Log

Meaningful commit messages
Progressive commits used

Documentation

ReadMe acceptable, could use more info on model/background -1
Good Description and Comments
Markdown used PDF submitted

Pull Request

Successful Pull Request (Working Algorithm Delivered on Time in Correct Branch)
Feedback required, remove model files, repo is not supposed to have models because of size, fix conflicts -2
Request Description OK, but could use more info -1

@4lon
Copy link
Author

4lon commented Nov 24, 2022

Apologies for the conflicts and including the model files, I have merged the changes and removed the model files. I think this should address all the feedback. Please let me know if there are any other issues.

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

Successfully merging this pull request may close these issues.

4 participants