Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Fixes infinite loop using imagedetiter #13550

Merged
merged 1 commit into from
Dec 8, 2018
Merged

Fixes infinite loop using imagedetiter #13550

merged 1 commit into from
Dec 8, 2018

Conversation

stu1130
Copy link
Contributor

@stu1130 stu1130 commented Dec 5, 2018 •
edited
Loading

Description

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  1. add last_batch_handle support for imagedetiter
  2. add unit tests

Comments

@zhreshold

@stu1130 stu1130 requested a review from szha as a code owner December 5, 2018 17:55
@stu1130
Copy link
Contributor Author

stu1130 commented Dec 5, 2018

@mxnet-label-bot add [pr-awaiting-testing]

@marcoabreu marcoabreu added the pr-awaiting-testing PR is reviewed and waiting CI build and test label Dec 5, 2018
@stu1130 stu1130 changed the title [WIP] Fix infinite loop imageiter Fix infinite loop imageiter Dec 5, 2018
@stu1130 stu1130 changed the title Fix infinite loop imageiter Fixes infinite loop imageiter Dec 5, 2018
@stu1130 stu1130 changed the title Fixes infinite loop imageiter Fixes infinite loop using imagedetiter Dec 5, 2018
@zhreshold
Copy link
Member

LGTM, suggest to cherry-pick into v1.4.x as well.

roywei
roywei approved these changes Dec 6, 2018
View reviewed changes
Copy link
Member

@roywei roywei left a comment

Choose a reason for hiding this comment

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

LGTM! thanks for the fix

tests/python/unittest/test_image.py Outdated
@@ -149,88 +151,90 @@ def test_color_normalize(self):
mx_result = mx.image.color_normalize(mx.nd.array(src),
mx.nd.array(mean), mx.nd.array(std))
assert_almost_equal(mx_result.asnumpy(), (src - mean) / std, atol=1e-3)
@nottest
Copy link
Member

Choose a reason for hiding this comment

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

new line

tests/python/unittest/test_image.py Outdated
@@ -149,88 +151,90 @@ def test_color_normalize(self):
mx_result = mx.image.color_normalize(mx.nd.array(src),
mx.nd.array(mean), mx.nd.array(std))
assert_almost_equal(mx_result.asnumpy(), (src - mean) / std, atol=1e-3)
@nottest
def _test_imageiter_last_batch(self, imageiter_list, assert_data_shape):
Copy link
Member

Choose a reason for hiding this comment

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

rename helper function so you don't need to add `@nottest'?

@stu1130
fix the situation where idx didn't align with rec
9c61ac2 
minor fix the image.py

add last_batch_handle for imagedeiter

remove the label type

refactor the imageiter unit test

fix the trailing whitespace

fix coding style

add new line

move helper function to the top of the file
@zhreshold
Copy link
Member

@stu1130 Is it finished? Can we merge now?

@stu1130
Copy link
Contributor Author

stu1130 commented Dec 8, 2018

@zhreshold It's finished. Yes you can

@zhreshold zhreshold merged commit 186a746 into apache:master Dec 8, 2018
zhreshold pushed a commit that referenced this pull request Dec 8, 2018
@stu1130 @zhreshold
fix the situation where idx didn't align with rec (#13550)
b7f0122 
minor fix the image.py

add last_batch_handle for imagedeiter

remove the label type

refactor the imageiter unit test

fix the trailing whitespace

fix coding style

add new line

move helper function to the top of the file
@zhreshold
Copy link
Member

cherry picked into v1.4.x

@stu1130 stu1130 deleted the fix_infinite_loop_imageiter branch December 11, 2018 00:55
zhaoyao73 pushed a commit to zhaoyao73/incubator-mxnet that referenced this pull request Dec 13, 2018
@stu1130 @zhaoyao73
fix the situation where idx didn't align with rec (apache#13550)
a2769a2 
minor fix the image.py

add last_batch_handle for imagedeiter

remove the label type

refactor the imageiter unit test

fix the trailing whitespace

fix coding style

add new line

move helper function to the top of the file
zhaoyao73 added a commit to zhaoyao73/incubator-mxnet that referenced this pull request Dec 13, 2018
@zhaoyao73
Merge remote-tracking branch 'upstream/master'
79de25d 
* upstream/master: (54 commits)
  Add notes about debug with libstdc++ symbols (apache#13533)
  add cpp example inception to nightly test (apache#13534)
  Fix exception handling api doc (apache#13519)
  fix link for gluon model zoo (apache#13583)
  ONNX import/export: Size (apache#13112)
  Update MXNetTutorialTemplate.ipynb (apache#13568)
  fix the situation where idx didn't align with rec (apache#13550)
  Fix use-before-assignment in convert_dot (apache#13511)
  License update  (apache#13565)
  Update version to v1.5.0 including clojure package (apache#13566)
  Fix flaky test test_random:test_randint_generator (apache#13498)
  Add workspace cleaning after job finished (apache#13490)
  Adding test for softmaxoutput (apache#13116)
  apache#13441 [Clojure] Add Spec Validations for the Random namespace (apache#13523)
  Revert "Bumped minor version from 1.4.0 to 1.5.0 on master, updated License file" (apache#13558)
  Chi_square_check for discrete distribution fix (apache#13543)
  Updated docs for randint operator (apache#13541)
  Simplifications and some fun stuff for the MNIST Gluon tutorial (apache#13094)
  Fix apache#13521 (apache#13537)
  Add a retry to qemu_provision (apache#13551)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@roywei roywei roywei approved these changes

@zhreshold zhreshold zhreshold approved these changes

@szha szha Awaiting requested review from szha szha is a code owner

Assignees
No one assigned
Labels
pr-awaiting-testing PR is reviewed and waiting CI build and test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ImageDetIter looping forever in MXNet-1.3.0
4 participants
@stu1130 @zhreshold @roywei @marcoabreu