Train WildFire #64
Merged
Train WildFire #64
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #64 +/- ##
=======================================
Coverage 83.85% 83.85%
=======================================
Files 19 19
Lines 706 706
=======================================
Hits 592 592
Misses 114 114 |
frgfm
requested changes
May 12, 2020
Comment on lines
72
to
87
| if ratios['test'] > 0: | ||
| fire_ids = {'train': self._get_fire_ids_for_one_split(n_samples_train), | ||
| 'val': self._get_fire_ids_for_one_split(n_samples_val)} | ||
| fire_ids['test'] = [id_ for id_ in self._fire_id_to_size | ||
| if id_ not in (fire_ids['train'] + fire_ids['val'])] | ||
| # Finish exhaustion | ||
| for fire_id_test in fire_ids['test']: | ||
| del self._fire_id_to_size_to_exhaust[fire_id_test] | ||
|
|
||
| else: | ||
| fire_ids = {'train': self._get_fire_ids_for_one_split(n_samples_train)} | ||
| fire_ids['val'] = [id_ for id_ in self._fire_id_to_size if id_ not in fire_ids['train']] | ||
| # Finish exhaustion | ||
| for fire_id_test in fire_ids['val']: | ||
| del self._fire_id_to_size_to_exhaust[fire_id_test] | ||
| fire_ids['test'] = [] |
There was a problem hiding this comment.
Isn't this supposed to be in #63 ?
There was a problem hiding this comment.
Could you cancel out this modif please?
Comment on lines
161
to
164
| # if wildfire.metadata['fire_id'].nunique() != wildfire.metadata['fire_id'].max() + 1: | ||
| # warnings.warn(f"Inconsistent Fire Labeling. Maybe try to label the fire again\n" | ||
| # f"Distinct values of ids({wildfire.metadata['fire_id'].nunique()}" | ||
| # f" ≠ {wildfire.metadata['fire_id'].max() + 1})", Warning) |
test/test_datasets_wildfire_split.py
Outdated
Comment on lines
96
to
104
| def test_splitting_with_test_to_zero(self): | ||
| ratios = {'train': 0.81, 'val': 0.19, 'test': 0} | ||
|
|
||
| splitter = WildFireSplitter(ratios, seed=42) | ||
| splitter.fit(self.wildfire) | ||
|
|
||
| for (set_, ratio_) in splitter.ratios_.items(): | ||
| self.assertAlmostEqual(ratio_, ratios[set_], places=2) | ||
|
|
frgfm
requested changes
May 19, 2020
| normalize = transforms.Normalize(mean=[0.485, 0.456, 0.406], std=[0.229, 0.224, 0.225]) | ||
|
|
||
| train_transforms = transforms.Compose([ | ||
| transforms.RandomResizedCrop(size=args.resize // 7, scale=(0.8, 1.0)), |
There was a problem hiding this comment.
Sounds like a pretty big loss of information with a relative output size of 1/7. Perhaps we should directly resize to target size?
There was a problem hiding this comment.
It was a mistake, I didn't want to lose so much information. Thank you, that's corrected. I also deleted one of the two crops.
| transforms.RandomRotation(degrees=15), | ||
| transforms.ColorJitter(), | ||
| transforms.RandomHorizontalFlip(), | ||
| transforms.CenterCrop(size=args.resize), # Image net standards |
There was a problem hiding this comment.
Generally CenterCrop is used for eval but not train set. Having two crop transformations seems unnecessary. It might be better to stick with RandomResizedCrop but discard CenterCrop
| ]) | ||
|
|
||
| val_transforms = transforms.Compose([ | ||
| transforms.Resize(size=args.resize // 7), |
frgfm
reviewed
May 20, 2020
frgfm
reviewed
May 20, 2020
Comment on lines
72
to
87
| if ratios['test'] > 0: | ||
| fire_ids = {'train': self._get_fire_ids_for_one_split(n_samples_train), | ||
| 'val': self._get_fire_ids_for_one_split(n_samples_val)} | ||
| fire_ids['test'] = [id_ for id_ in self._fire_id_to_size | ||
| if id_ not in (fire_ids['train'] + fire_ids['val'])] | ||
| # Finish exhaustion | ||
| for fire_id_test in fire_ids['test']: | ||
| del self._fire_id_to_size_to_exhaust[fire_id_test] | ||
|
|
||
| else: | ||
| fire_ids = {'train': self._get_fire_ids_for_one_split(n_samples_train)} | ||
| fire_ids['val'] = [id_ for id_ in self._fire_id_to_size if id_ not in fire_ids['train']] | ||
| # Finish exhaustion | ||
| for fire_id_test in fire_ids['val']: | ||
| del self._fire_id_to_size_to_exhaust[fire_id_test] | ||
| fire_ids['test'] = [] |
There was a problem hiding this comment.
Could you cancel out this modif please?
test/test_datasets_wildfire_split.py
Outdated
Comment on lines
96
to
104
| def test_splitting_with_test_to_zero(self): | ||
| ratios = {'train': 0.81, 'val': 0.19, 'test': 0} | ||
|
|
||
| splitter = WildFireSplitter(ratios, seed=42) | ||
| splitter.fit(self.wildfire) | ||
|
|
||
| for (set_, ratio_) in splitter.ratios_.items(): | ||
| self.assertAlmostEqual(ratio_, ratios[set_], places=2) | ||
|
|
There was a problem hiding this comment.
Could you cancel out this modification? The rest of the PR looks good!
Akilditu
reviewed
May 21, 2020
Akilditu
reviewed
May 21, 2020
Akilditu
reviewed
May 21, 2020
Akilditu
pushed a commit
to Akilditu/seb_pyro-vision
that referenced
this pull request
Jun 3, 2020
* Allow test ratio to zero in WildFireSplitter * fix flake8 * add training script for Wildfire * add a test to improve code coverage * put back the warning on FireIds * Fix size in RandomCrop and delete CenterCrop * add centercrop to get square images * remove change already made in PR pyronear#63
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The purpose of this PR is to add a basic training loop for the wildfire dataset. This is far from being optimal in terms of performance but it can be considered as a first baseline.