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

MXNET-1447 [Perl] Runtime features and large tensor support. #17610

Merged
merged 7 commits into from
Feb 19, 2020
Merged

MXNET-1447 [Perl] Runtime features and large tensor support. #17610

merged 7 commits into from
Feb 19, 2020

Conversation

sergeykolychev
Copy link
Contributor

@sergeykolychev sergeykolychev commented Feb 17, 2020 •
edited
Loading

Description

[Perl] MXNET-1447 Runtime features and large tensor support.

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 https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  1. Added runtime features
  2. Added support for large tensors

Comments

@sergeykolychev
Copy link
Contributor Author

sergeykolychev commented Feb 17, 2020 •
edited
Loading

@tlby
Robert, could you please review. This is first from several PRs I plan to do within next few months.
It adds support for large tensors (int64) and for displaying runtime features (compile flags).

@sergeykolychev sergeykolychev removed the request for review from marcoabreu February 17, 2020 06:41
tlby
tlby reviewed Feb 17, 2020
View reviewed changes
perl-package/AI-MXNet/lib/AI/MXNet/NDArray.pm
@@ -384,8 +385,11 @@ method _slice (
)
{
confess("start $start > stop $stop") if $start > $stop;
my $sub = AI::MXNet::RunTime->Features()->is_enabled('INT64_TENSOR_SIZE')
? \&AI::MXNetCAPI::NDArraySlice64
: \&AI::MXNetCAPI::NDArraySlice;
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems a little awkward to put the caller in charge of checking the INT64_TENSOR_SIZE compile setting. Would it make sense to make AI::MXNetCAPI::NDArraySlice() and friends responsible for checking the flag and deferring to the NDArray*64() variants when appropriate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tlby On python side the switch is made in the caller space. The c api has two functions defined for each task, one with (u)?int32_t input/output and another with (u)?int64_t
I don't know how to combine these two functions via the swig space.
Looks like a task for you to complete later the line.

Copy link
Contributor

Choose a reason for hiding this comment

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

If that's how things are done in Python, then no need to diverge from this pattern. Thanks!

tlby
tlby approved these changes Feb 17, 2020
View reviewed changes
Copy link
Contributor

@tlby tlby left a comment

Choose a reason for hiding this comment

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

Is this a full switch away from the mx_ typedefs in reaction to #15899?

@sergeykolychev
Copy link
Contributor Author

@tlby The mx_ typedefs change is a sync to current state of the mxnet/c_api.h in master branch.

@sergeykolychev sergeykolychev merged commit c215b33 into apache:master Feb 19, 2020
anirudh2290 pushed a commit to anirudh2290/mxnet that referenced this pull request May 29, 2020
@sergeykolychev @anirudh2290
MXNET-1447 [Perl] Runtime features and large tensor support. (apache#…
5edeeeb 
…17610)

* [Perl] MXNET-1447 Runtime features and large tensor support.

* MXNET-1447 fixed test, corrected module version.

* miscellaneous cleanups.

* added missing file.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tlby tlby tlby approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sergeykolychev @tlby