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

VIT - S46878467 #176

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

VIT - S46878467 #176

wants to merge 17 commits into from

Conversation

HaadiQureshi
Copy link

I had an approved late submission of 1 week, submitting on time. Regards

This is a test to ensure the repository and commits are correctly set up
Converting them to a format suitable for machine learning, organizing them into groups, and creating labels
Set-up data loading pipelines for training, validation, and testing
Slight modification to code as error persisted
Completely changed dataset.py as previously it dealt with 2d images, now updated for 3d
define the core components of your recognition model using classes or functions.
Increased Epoch to get high accuracy
Just copied the accuracies from the output file in rangpur and plotted it against the epoch
Testing to see what would give highest accuracy.
This gave 0.68
First proper update on readme.md. Still need to add graphs
@LinfengLiu98
Copy link
Collaborator

LinfengLiu98 commented Nov 8, 2023

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

Difficulty: Hard

Readme:

  • Missing model details. Need to explain what the model structure is and how it works to show your understanding of it.
  • Only the test accuracy plot is shown. No training accuracy and validation accuracy.

Commit messages: good, need more detail on which has been modified instead of just uploading files.

Code:

  • Uses Vision transformer
  • Comments mainly in the model design. Less comments in the data loader training loop and prediction file.
  • Good design

Functionality/Performance:

  • No patient-level split
  • Model is overfitting
  • Accuracy is only 0.53

General comments:
PR is in the main branch

  • Solves the problem appropriately although performance is poor
  • No patient-level split so the model is overfitting. Also need to do some hyperparameter search which hasn't been shown in the report.

@shakes76 shakes76 added the Extension Extension approved label Nov 20, 2023
@shakes76
Copy link
Owner

Marking

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

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

Recognition Problem

Solves problem poor performance < 0.6 -1
Driver Script present
File structure present but code needs to be in own folder -2
Shows Usage & Demo & Visualisation & Data usage
Module present
Commenting
No Data leakage, no patient level split -1
Difficulty: Hard

Commit Log

Meaningful commit messages
Progressive commits used

Documentation

ReadMe acceptable, no refs -1
Model/technical explanation missing -1
Good Description and Comments
Markdown used and PDF submitted

Pull Request

Successful Pull Request (Working Algorithm Delivered on Time in InCorrect Branch) -2
Feedback required, need to move code into own folder and use correct branch -2
Request Description minimal -1

@shakes76 shakes76 added the question Further information is requested label Nov 20, 2023
@shakes76
Copy link
Owner

Feedback marks possible +2 if the requested changes are made (see above).

@wangzhaomxy
Copy link
Collaborator

No feedback attempt and no feedback marks granted.

@wangzhaomxy wangzhaomxy added Question_attempt and removed question Further information is requested labels Nov 21, 2023
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.

5 participants