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

An empty NDArray should have size 0 #14717

Closed
wants to merge 4 commits into from
Closed

An empty NDArray should have size 0 #14717

wants to merge 4 commits into from

Conversation

ziyuang
Copy link

@ziyuang ziyuang commented Apr 17, 2019

Description

(Brief description on what this PR is about)

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

  • Feature1, tests, (and when applicable, API doc)
  • Feature2, tests, (and when applicable, API doc)

Comments

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

@ziyuang ziyuang requested a review from nswamy as a code owner April 17, 2019 13:04
@Roshrini Roshrini added the pr-awaiting-review PR is waiting for code review label Apr 17, 2019
@Roshrini
Copy link
Member

@ziyuang Can you add unittest for this change?

@reminisce
Copy link
Contributor

Can you confirm that an empty shape represents an empty ndarray in cpp-package? In Python and C++ backend, an empty shape actually represents a scalar tensor.

@ziyuang
Copy link
Author

ziyuang commented Apr 25, 2019

Can you confirm that an empty shape represents an empty ndarray in cpp-package? In Python and C++ backend, an empty shape actually represents a scalar tensor.

The expected behavior is to return 0 when the array is empty; now I do this check directly without touching the shape vector.

@ziyuang
Copy link
Author

ziyuang commented Apr 26, 2019

@ziyuang Can you add unittest for this change?

Will add a standalone one under tests/cpp/misc/ if that's good for you

@vandanavk
Copy link
Contributor

@ziyuang please fill in the PR description template with relevant details about the changes

@pinaraws
Copy link

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

@marcoabreu marcoabreu added the pr-awaiting-response PR is reviewed and waiting for contributor to respond label May 20, 2019
@pinaraws
Copy link

@mxnet-label-bot remove[pr-awaiting-review]

@marcoabreu marcoabreu removed the pr-awaiting-review PR is waiting for code review label May 20, 2019
@abhinavs95
Copy link
Contributor

@ziyuang any update on the PR? Also please fill the PR description with relevant details.

@piyushghai
Copy link
Contributor

@apeforest For review.

@Roshrini
Copy link
Member

@ziyuang any update on the PR?

@ziyuang
Copy link
Author

ziyuang commented Jun 28, 2019

Hi, has this zero-size NDArray issue been handled well by #14661 already?

@karan6181
Copy link
Contributor

@ziyuang Could you please see if #14661 fix your issue or not by running small reproducible code and installing the latest pre-release version of MXNet.

@piyushghai
Copy link
Contributor

@ziyuang Ping for an update.

@roywei
Copy link
Member

roywei commented Aug 19, 2019

@ziyuang please trigger CI and update this PR?

@mseth10 mseth10 added pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test and removed pr-awaiting-response PR is reviewed and waiting for contributor to respond pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Mar 14, 2022
@mseth10 mseth10 added pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Mar 30, 2022
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Apr 16, 2022
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Apr 27, 2022
@ziyuang ziyuang closed this by deleting the head repository Sep 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr-work-in-progress PR is still work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.