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

[MXNET-1352] Allow dynamic shape in while_loop and if conditionals #14393

Merged
merged 18 commits into from
May 12, 2019
Merged

[MXNET-1352] Allow dynamic shape in while_loop and if conditionals #14393

merged 18 commits into from
May 12, 2019

Conversation

junrushao
Copy link
Member

@junrushao junrushao commented Mar 11, 2019 •
edited
Loading

Description

This PR introduces dynamic shape in contrib.while_loop and contrib.cond to accept dynamically shaped states.

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:https://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

  • Remove shape inference for while and cond.

Comments

@junrushao junrushao changed the title Allow dynamic shape in while_loop and if conditionals [MXNET-1352] Allow dynamic shape in while_loop and if conditionals Mar 11, 2019
@marcoabreu marcoabreu added Operator pr-awaiting-review PR is waiting for code review pr-work-in-progress PR is still work in progress labels Mar 12, 2019
@karan6181
Copy link
Contributor

@mxnet-label-bot add [Operator, pr-work-in-progress]

@junrushao junrushao marked this pull request as ready for review April 4, 2019 22:04
@junrushao
Copy link
Member Author

This work is more complicated than expected: to let CI pass, I have to modify the graph executor...

@junrushao junrushao requested a review from szha as a code owner April 12, 2019 21:54
@junrushao
Copy link
Member Author

junrushao commented Apr 12, 2019 •
edited
Loading

Symbol executor seems to be willing to work with dynamic shape now...At least I could pass unittest for control flow operators locally...

@junrushao
Copy link
Member Author

junrushao commented Apr 12, 2019 •
edited
Loading

This is kinda weird that I failed ASAN test, because my changes are not in the code path in MNIST training. Could anyone help to look at it? Thanks!

Updated: fixed, never mind

@junrushao junrushao force-pushed the dynamic-cached-op-pr-4 branch 5 times, most recently from 18aa427 to acfbb5a Compare April 13, 2019 05:02
@junrushao
Copy link
Member Author

@szha Finally it is done. Could you help view? Thanks!

@junrushao junrushao force-pushed the dynamic-cached-op-pr-4 branch 4 times, most recently from 6333a47 to f28f4b9 Compare April 22, 2019 06:09
@junrushao
Copy link
Member Author

Finally it should work...

@szha szha requested a review from zheng-da May 7, 2019 23:22
zheng-da
zheng-da reviewed May 8, 2019
View reviewed changes
include/mxnet/ndarray.h Show resolved Hide resolved
src/executor/graph_executor.cc Show resolved Hide resolved
src/executor/graph_executor.cc Outdated Show resolved Hide resolved
@junrushao
Copy link
Member Author

Review comments addressed. Are we good to merge?

@zheng-da zheng-da merged commit d577b6f into apache:master May 12, 2019
haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019
@junrushao
[MXNET-1352] Allow dynamic shape in while_loop and if conditionals (a…
fce7baf 
…pache#14393)

* Initial commit

* Rebase

* WIP for fixing rebase issues

* WIP for fixing rebase issues

* fix wip

* wip fix

* wip fix

* wip fix

* wip fix

* wip fix

* wip fix

* should be good to go

* wip remove debug info

* wip remove debug info

* linter

* linter

* Retrigger

* Address comments from Da
@vandanavk vandanavk mentioned this pull request Jan 10, 2020
Merged
5 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@zheng-da zheng-da zheng-da approved these changes

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

@anirudh2290 anirudh2290 Awaiting requested review from anirudh2290

@eric-haibin-lin eric-haibin-lin Awaiting requested review from eric-haibin-lin eric-haibin-lin is a code owner

Assignees
No one assigned
Labels
Operator pr-awaiting-review PR is waiting for code review pr-work-in-progress PR is still work in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@junrushao @karan6181 @zheng-da @marcoabreu