update DALIClassificationLoader to not use deprecated arguments #4925

gan3sh500 · 2020-12-01T04:00:03Z

What does this PR do?

Fixes #4918 issue due to DALI API change. Now uses LastBatchPolicy's FILL and DROP instead of passing fill_last_batch.

Before submitting

Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
Did you read the contributor guideline, Pull Request section?
Did you make sure your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
Did you make sure to update the documentation with your changes?
Did you write any new necessary tests?
Did you verify new and existing tests pass locally with your changes?
If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:

Is this pull request ready for review? (if not, please submit in draft mode)
Check that all items from Before submitting are resolved
Make sure the title is self-explanatory and the description concisely explains the PR
Add labels and milestones (and optionally projects) to the PR so it can be classified; Bugfixes should be including in bug-fix release milestones (m.f.X) and features should be included in (m.X.b) releases.

Did you have fun?

Make sure you had fun coding 🙃

pep8speaks · 2020-12-01T04:00:07Z

Hello @gan3sh500! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-12-18 13:31:30 UTC

Borda · 2020-12-01T08:08:26Z

pl_examples/basic_examples/dali_image_classifier.py


    def __len__(self):
        batch_count = self._size // (self._num_gpus * self.batch_size)
-        last_batch = 1 if self._fill_last_batch else 0
+        last_batch = 0 if self._last_batch_policy == LastBatchPolicy.DROP else 1


will this work also with older DALI versions, if not can we add a simple switch to something for dali.__version__ > 0.28 and the old code for <=0.27

No it will not. I will add some check based on Dali version like you said.

I've pushed change for this. Tests passed except one which cancelled on its own.

codecov · 2020-12-02T04:56:40Z

Codecov Report

Merging #4925 (d8c66e2) into master (81070be) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #4925   +/-   ##
======================================
  Coverage      93%     93%           
======================================
  Files         134     134           
  Lines        9928    9928           
======================================
  Hits         9242    9242           
  Misses        686     686

pl_examples/basic_examples/dali_image_classifier.py

…o dali_fix

tchaton · 2020-12-03T09:02:29Z

Hey @gan3sh500 @Borda , ready for review ?

gan3sh500 · 2020-12-03T11:10:51Z

@tchaton It is ready. I was waiting for @Borda to confirm.

Borda · 2020-12-03T11:47:18Z

@tchaton It is ready. I was waiting for @Borda to confirm.

I made just small update =)

…ning into dali_fix

gan3sh500 · 2020-12-09T03:46:56Z

@Borda can I try out some other issue or is there more I should do here?

Borda · 2020-12-09T06:38:33Z

@Borda can I try out some other issue or is there more I should do here?

Sure, ping me on slack, we can find good one for you...

* update DALIClassificationLoader to not use deprecated arguments * fix line length * dali version check added and changed args accordingly * versions * checking version using disutils.version.LooseVersion now * . * ver * import Co-authored-by: Jirka Borovec <jirka.borovec@seznam.cz> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

update DALIClassificationLoader to not use deprecated arguments

1426476

fix line length

34a2364

Borda added 3rd party Related to a 3rd-party example bug Something isn't working labels Dec 1, 2020

Borda added this to the 1.1 milestone Dec 1, 2020

Borda reviewed Dec 1, 2020

View reviewed changes

Borda added the priority: 1 Medium priority task label Dec 1, 2020

gan3sh500 added 2 commits December 2, 2020 09:57

dali version check added and changed args accordingly

ba05185

Merge branch 'master' into dali_fix

1edc825

Borda reviewed Dec 2, 2020

View reviewed changes

pl_examples/basic_examples/dali_image_classifier.py Outdated Show resolved Hide resolved

Borda and others added 4 commits December 2, 2020 11:15

versions

fca9e47

checking version using disutils.version.LooseVersion now

82da61b

Merge branch 'dali_fix' of github.com:gan3sh500/pytorch-lightning int…

d21a7f4

…o dali_fix

Merge branch 'master' into dali_fix

0383b2f

.

06e1f81

Borda approved these changes Dec 3, 2020

View reviewed changes

Borda marked this pull request as ready for review December 3, 2020 09:08

Borda requested review from ananyahjha93, awaelchli, justusschock, nateraw, SeanNaren, tchaton, teddykoker and williamFalcon as code owners December 3, 2020 09:08

edenlightning modified the milestones: 1.1, 1.1.x Dec 8, 2020

Borda added ready PRs ready to be merged and removed ready PRs ready to be merged labels Dec 8, 2020

Merge branch 'master' into dali_fix

4724994

Borda self-assigned this Dec 8, 2020

Borda added 3 commits December 8, 2020 19:54

ver

0e8d545

Merge branch 'dali_fix' of https://github.com/gan3sh500/pytorch-light…

476ba60

…ning into dali_fix

import

7f91b56

Merge branch 'master' into dali_fix

889cc15

mergify bot requested a review from a team December 12, 2020 14:57

Merge branch 'master' into dali_fix

1de4990

Borda added the ready PRs ready to be merged label Dec 16, 2020

s-rog approved these changes Dec 18, 2020

View reviewed changes

SkafteNicki approved these changes Dec 18, 2020

View reviewed changes

Merge branch 'master' into dali_fix

d8c66e2

Borda merged commit 16e819e into Lightning-AI:master Dec 18, 2020

rsaite mentioned this pull request Jan 12, 2021

Iterator returns wrong length when last batch policy is drop NVIDIA/DALI#2607

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update DALIClassificationLoader to not use deprecated arguments #4925

update DALIClassificationLoader to not use deprecated arguments #4925

gan3sh500 commented Dec 1, 2020 •

edited

Loading

pep8speaks commented Dec 1, 2020 •

edited

Loading

Borda Dec 1, 2020

gan3sh500 Dec 1, 2020

gan3sh500 Dec 2, 2020

codecov bot commented Dec 2, 2020 •

edited

Loading

tchaton commented Dec 3, 2020

gan3sh500 commented Dec 3, 2020

Borda commented Dec 3, 2020

gan3sh500 commented Dec 9, 2020

Borda commented Dec 9, 2020

update DALIClassificationLoader to not use deprecated arguments #4925

update DALIClassificationLoader to not use deprecated arguments #4925

Conversation

gan3sh500 commented Dec 1, 2020 • edited Loading

What does this PR do?

Before submitting

PR review

Did you have fun?

pep8speaks commented Dec 1, 2020 • edited Loading

Comment last updated at 2020-12-18 13:31:30 UTC

Borda Dec 1, 2020

Choose a reason for hiding this comment

gan3sh500 Dec 1, 2020

Choose a reason for hiding this comment

gan3sh500 Dec 2, 2020

Choose a reason for hiding this comment

codecov bot commented Dec 2, 2020 • edited Loading

Codecov Report

tchaton commented Dec 3, 2020

gan3sh500 commented Dec 3, 2020

Borda commented Dec 3, 2020

gan3sh500 commented Dec 9, 2020

Borda commented Dec 9, 2020

gan3sh500 commented Dec 1, 2020 •

edited

Loading

pep8speaks commented Dec 1, 2020 •

edited

Loading

codecov bot commented Dec 2, 2020 •

edited

Loading