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

Load NDArray only to GPU if GPU is present#16432

Merged
szha merged 2 commits intoapache:masterfrom
leezu:fix16399
Oct 17, 2019
Merged

Load NDArray only to GPU if GPU is present#16432
szha merged 2 commits intoapache:masterfrom
leezu:fix16399

Conversation

@leezu
Copy link
Contributor

@leezu leezu commented Oct 11, 2019

Description

Fix #16399

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • 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 my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Fix crash when attempting to load a NDArray that was saved with GPU context on a MXNet runtime built with Cuda but when no GPUs are present

Comments

  • I'm not sure if the CI actually tests the case of "GPU build of MXNET but no GPU present". So the test added here may always pass on CI.

@ZhennanQin
Copy link
Contributor

ZhennanQin commented Oct 11, 2019

For a machine with GPU, if user want to run model on CPU only, will this change works for him?

@leezu
Copy link
Contributor Author

leezu commented Oct 11, 2019

This change only affects the case where a MXNet cuda-enabled build is used on a CPU-only machine. It avoids a crash that currently happens. So the use-case you describe is not affected.

@ZhennanQin
Copy link
Contributor

ZhennanQin commented Oct 11, 2019

So if user want to use MXNet cuda-enabled build on a GPU equipped machine, but want to run model on its CPU, is this case supported?

@leezu
Copy link
Contributor Author

leezu commented Oct 11, 2019 •
edited
Loading

Yes. It is already supported currently. Nothing is changed with respect to that.

The problem addressed here, is that a serialized ndarray encodes the context it lived on before serialization. When loading it, and no GPU is present, MXNet crashes as it attempts to load the array to GPU. With this PR, we fallback to loading to CPU. This is already done for CPU-only builds of MXNet.

@leezu leezu mentioned this pull request Oct 14, 2019
Closed
@szha szha merged commit a4ea4a8 into apache:master Oct 17, 2019
@leezu leezu deleted the fix16399 branch October 17, 2019 22:54
apeforest pushed a commit that referenced this pull request Nov 14, 2019
@leezu @apeforest
Load NDArray only to GPU if GPU is present (#16432)
70b65cb 
* Load NDArray only to GPU if GPU is present

* Add test
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@szha szha szha approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ndarray.load crashes MXNet GPU builds on CPU machines

3 participants

@leezu @ZhennanQin @szha