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

Cats&Dogs model improvement #254

Merged
merged 10 commits into from
Oct 12, 2023

Conversation

ermanok
Copy link
Contributor

@ermanok ermanok commented Sep 19, 2023

  • Dataloader changed to augment data randomly at each epoch
  • Training & QAT policies updated for optimized model training

ermanok and others added 6 commits July 28, 2023 23:38
Augmentation approach fixed. Random augmentations added to be applied dynamically while loading the data samples.
Old dataloader removed.
Linter errors fixed.
Copy link
Contributor

@MaximGorkem MaximGorkem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@@ -0,0 +1,4 @@
---
start_epoch: 30
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not very late, considering the 200 epochs. We might want to drop "late" from the filename.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

album.ShiftScaleRotate(shift_limit=0.05, scale_limit=0.05, rotate_limit=15, p=0.5),
album.RandomCrop(height=resize_size[0], width=resize_size[1]),
album.HorizontalFlip(p=0.5),
album.Normalize(mean=(0.0, 0.0, 0.0), std=(1.0, 1.0, 1.0))])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question: Doesn't this normalize all images & all channels independently? It is different than dividing the entire dataset by 255, which we employ in the other vision examples. Whichever correct way is needed to be applied to all examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both approaches are very similar to each other. The previous appraoch divides the dataset by 255 while this one divides with the maximum value of the sample. Both should provide similar results and it requires to make extensive experiments to understand if one is correct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. I assumed that it divides each channel with its own maximum value. Doublechecked the documentation and it divides with max possible, which is 255.

@ermanok ermanok requested a review from rotx-eva September 20, 2023 13:04
rotx-eva
rotx-eva previously approved these changes Sep 25, 2023
album.ShiftScaleRotate(shift_limit=0.05, scale_limit=0.05, rotate_limit=15, p=0.5),
album.RandomCrop(height=resize_size[0], width=resize_size[1]),
album.HorizontalFlip(p=0.5),
album.Normalize(mean=(0.0, 0.0, 0.0), std=(1.0, 1.0, 1.0))])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. I assumed that it divides each channel with its own maximum value. Doublechecked the documentation and it divides with max possible, which is 255.

@@ -1,2 +1,2 @@
#!/bin/sh
python train.py --epochs 250 --optimizer Adam --lr 0.001 --wd 0 --deterministic --compress policies/schedule-catsdogs.yaml --model ai85cdnet --dataset cats_vs_dogs --confusion --param-hist --embedding --device MAX78000 "$@"
python train.py --epochs 200 --optimizer Adam --lr 0.001 --wd 0 --deterministic --compress policies/schedule-catsdogs.yaml --qat-policy policies/qat_policy_late_cd.yaml --model ai85cdnet --dataset cats_vs_dogs --confusion --param-hist --embedding --device MAX78000 "$@"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that "qat_policy_late_cd.yaml" should be replaced with "qat_policy_cd.yaml".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct and fixed...

fixes the script after changing qat policy filename changed
@rotx-eva rotx-eva merged commit aae63bd into analogdevicesinc:develop Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants