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

46376066_William_Parker_ADNI_Classification_Merge #473

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

Conversation

parker334
Copy link

A request to merge the files of PatternFlow\recognition\46376066_ADNI into the main repository. Shows a implementation for a model which classifies whether brain scans are indicative of Alzheimer's or not. It was trained on and processes the given pre-processed ADNI Alzheimer's data. Implemented with Tensorflow.

Includes the modules.py, dataset.py, train.py and predict.py and a README.md with associated images.

Please read README.md for explanation of late pull request and lack of commits. (see original_git_log.txt).

@LinfengLiu98
Copy link
Collaborator

LinfengLiu98 commented Nov 9, 2022

Model design: It seems like you designed a ViT instead of a perceiver.

Model training: training ACC seems to be a bit low. Highly likely training loop/ model design has bugs.

Git logs: Only have two git commits.

RE: "However when used to predict on 10 positve test data it was 100% accurate." From the training plot you provided, it seems like you model is giving all 1s when you test 10 positive test data. You can try 10 negative data, it will probably give you all 1s as well.

@shakes76
Copy link
Owner

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

Adequate use and implementation (some issues with the testing procedure) -2
Good spacing and comments
Header blocks missing -1

Recognition Problem

Solves problem (poor performance) -1
Driver Script present
File structure present
Shows Usage & Demo & Visualisation & Data usage
Module present (ViT or Perceiver?) -1
Commenting
No Data leakage (testing issues) -1
Difficulty: Hard

Commit Log

Meaningful commit messages
Progressive commits used, few commits used (evidence in txt file), no action taken -2

Documentation

ReadMe OK, but more info and discussion of output/results provided -1
Good Description and Comments
Markdown used PDF submitted

Pull Request

Successful Pull Request (Working Algorithm Delivered on Time in Correct Branch)
No Feedback required, but not mergeable without proper commit history, no mark deducted, but will not be merged sorry.
Request Description good

@shakes76 shakes76 added the Unmergeable Cant be merged for history or other reason label Nov 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Perceiver Transformer Unmergeable Cant be merged for history or other reason
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants