Fix MXNet R package build #13952

jitMatrix · 2019-01-22T06:42:38Z

Description

As mentioned in #13859 & #13936 , below error will occur when building R-Package on my ubuntu:

make[1]: Leaving directory '/root/incubator-mxnet/R-package/src'
installing to /usr/local/lib/R/site-library/mxnet/libs
** R
** demo
** inst
** preparing package for lazy loading
** help
No man pages found in package  'mxnet'
*** installing help indices
** building package indices
** installing vignettes
** testing if installed package can be loaded
[1] "Loading local: inst/libs/libmxnet.so"
Error: package or namespace load failed for 'mxnet':
 .onLoad failed in loadNamespace() for 'mxnet', details:
  call: dyn.load("R-package/inst/libs/libmxnet.so", local = FALSE)
  error: unable to load shared object '/root/incubator-mxnet/R-package/R-package/inst/libs/libmxnet.so':
  /root/incubator-mxnet/R-package/R-package/inst/libs/libmxnet.so: cannot open shared object file: No such file or directory
Error: loading failed
Execution halted
ERROR: loading failed
* removing '/usr/local/lib/R/site-library/mxnet'
Makefile:599: recipe for target 'rpkg' failed
make: *** [rpkg] Error 1

It seems that some dependent libraries of mxnet should also be copied to R directory.

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

pengzhao-intel · 2019-01-23T04:04:38Z

@xinyu-intel could you help take a review? Does this PR conflict with yours #13867 ?

xinyu-intel · 2019-01-23T04:21:03Z

@rajeshii Hi, thanks for your contribution! Can you also help add R unittest with MKL-DNN to the CI(./ci/jenkins/Jenkins_steps.groovy)?

jitMatrix · 2019-01-23T07:26:58Z

@xinyu-intel @pengzhao-intel Added and passed the CI:)

pengzhao-intel · 2019-01-23T13:58:33Z

@hetong007 could you help take a review?

hetong007 · 2019-01-23T19:50:42Z

LGTM. @marcoabreu do you have any comments?

marcoabreu · 2019-01-23T22:06:45Z

ci/jenkins/Jenkinsfile_unix_gpu

@@ -53,6 +53,7 @@ core_logic: {
    custom_steps.test_unix_python3_tensorrt_gpu(),
    custom_steps.test_unix_perl_gpu(),
    custom_steps.test_unix_r_gpu(),
+    custom_steps.test_unix_r_mkldnn_gpu(),


I'm not sure whether we need GPU+MKLDNN for R since we already test it in python. Could you elaborate what concerns you are specifically having?

marcoabreu · 2019-01-23T22:09:16Z

Ideally, we could do some smoketests for different compilation options by running a simple test to ensure everything is working. Considering we test different backends in Python already, I don't think there's much benefit of now testing the full spectrum across the different frontend languages. As far as I understand, the goal is to ensure everything gets compiled and linked properly, right?

TaoLv · 2019-01-24T09:43:04Z

Agree that we only need run minimal tests to make sure libraries can be found and properly linked during runtime.

pengzhao-intel · 2019-01-25T03:14:13Z

@marcoabreu @TaoLv could you help to take a review again?

pengzhao-intel

LGTM

marcoabreu · 2019-01-25T03:25:26Z

My feedback regarding the amount of testing is still unchanged. Am Fr., 25. Jan. 2019, 04:15 hat PatricZhao <notifications@github.com> geschrieben:

…

***@***.**** approved this pull request. LGTM — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13952 (review)>, or mute the thread </~https://github.com/notifications/unsubscribe-auth/ARxB6_mAvdZCdFElOe915hWGp3nAVQYIks5vGnbMgaJpZM4aMBrU> .

pengzhao-intel

@marcoabreu sorry, I mixed up the different PR.

@rajeshii could you update the PR based on the change request?

jitMatrix · 2019-01-27T13:04:46Z

@marcoabreu Sorry for the late response. I'm confused that whether I should skip the rpkg test for mkldnn backend and leave only the rpkg build in CI or I need to add a lite test for it. For the later one, I don't know how to design the test. What's your opinion @hetong007 ?

marcoabreu · 2019-01-28T05:44:20Z

Think about a small task that just creates a 5x5 matrix of ones. Just to make sure everything is properly linked at runtime. Or pick a random existing test and run that one. Am So., 27. Jan. 2019, 14:05 hat rajeshii <notifications@github.com> geschrieben:

…

@marcoabreu </~https://github.com/marcoabreu> Sorry for the late response. I'm confused that whether I should skip the rpkg test for mkldnn backend and leave only the rpkg build in CI or I need to add a lite test for it. For the later one, I don't know how to design the test. What's your opinion @hetong007 </~https://github.com/hetong007> ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13952 (comment)>, or mute the thread </~https://github.com/notifications/unsubscribe-auth/ARxB6xF4isZUoIAIGayi2OhWgjGkKRUpks5vHaQGgaJpZM4aMBrU> .

jitMatrix · 2019-01-29T11:12:45Z

@marcoabreu addressed your comments. It seems that all tests have passed but the CI is pending...

jitMatrix · 2019-01-30T07:20:34Z

Rebase three times with different kinds of CI failures:(

jitMatrix · 2019-01-30T12:08:56Z

@pengzhao-intel @xinyu-intel @marcoabreu All tests pass now:)

pengzhao-intel · 2019-01-30T15:20:57Z

ci/docker/runtime_functions.sh

+        R_LIBS=/tmp/r-site-library
+
+    R CMD INSTALL --library=/tmp/r-site-library R-package
+    R_LIBS=/tmp/r-site-library Rscript -e "library(mxnet); as.array(mx.nd.ones(c(2,3)))"


Could we add a more meaningful test, like convolution, FC or relu which will hit the real MKLDNN calculation?
We need to guarantee R can execute at least in the CI.

@pengzhao-intel Has changed to use mlp.

* fix mxnet r package build * add ci * remove mkldnn-gpu test for R * add minimal test for MKLDNN-R * pick mlp as minimal R test

fix mxnet r package build

6db3f6b

jitMatrix requested a review from szha as a code owner January 22, 2019 06:42

add ci

7f82402

marcoabreu suggested changes Jan 23, 2019

View reviewed changes

Rajeshii added 3 commits January 24, 2019 01:32

remove mkldnn-gpu test for R

c9c7bdd

Merge remote-tracking branch 'origin' into r_build

dc0b42e

Merge remote-tracking branch 'origin' into r_build

3c33417

pengzhao-intel approved these changes Jan 25, 2019

View reviewed changes

pengzhao-intel suggested changes Jan 25, 2019

View reviewed changes

sandeep-krishnamurthy added R CI pr-awaiting-response PR is reviewed and waiting for contributor to respond labels Jan 25, 2019

wittyalias mentioned this pull request Jan 28, 2019

unable to load mxnet library in R #14004

Open

TaoLv mentioned this pull request Jan 29, 2019

[Hot Fix]Temporary Disable MKL-DNN for building R-Package #13867

Closed

7 tasks

Rajeshii added 2 commits January 29, 2019 06:54

add minimal test for MKLDNN-R

9e0ec3a

Merge remote-tracking branch 'origin' into r_build

cd36fb1

Rajeshii added 2 commits January 30, 2019 01:17

Merge remote-tracking branch 'origin' into r_build

8ae9a57

Merge remote-tracking branch 'origin' into r_build

e55d00a

Merge remote-tracking branch 'origin' into r_build

1a0c2e6

pengzhao-intel reviewed Jan 30, 2019

View reviewed changes

Rajeshii added 2 commits January 31, 2019 00:31

pick mlp as minimal R test

4f65e2c

Merge remote-tracking branch 'origin' into r_build

f941133

marcoabreu approved these changes Jan 31, 2019

View reviewed changes

hetong007 merged commit 47277b0 into apache:master Jan 31, 2019

stephenrawls pushed a commit to stephenrawls/incubator-mxnet that referenced this pull request Feb 16, 2019

Fix MXNet R package build (apache#13952)

f78d776

* fix mxnet r package build * add ci * remove mkldnn-gpu test for R * add minimal test for MKLDNN-R * pick mlp as minimal R test

jwmueller mentioned this pull request Feb 19, 2019

Newly updated Makefile generate path errors #13859

Open

vdantu pushed a commit to vdantu/incubator-mxnet that referenced this pull request Mar 31, 2019

Fix MXNet R package build (apache#13952)

2e0de8d

* fix mxnet r package build * add ci * remove mkldnn-gpu test for R * add minimal test for MKLDNN-R * pick mlp as minimal R test

haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019

Fix MXNet R package build (apache#13952)

90300b5

* fix mxnet r package build * add ci * remove mkldnn-gpu test for R * add minimal test for MKLDNN-R * pick mlp as minimal R test

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix MXNet R package build #13952

Fix MXNet R package build #13952

jitMatrix commented Jan 22, 2019 •

edited

Loading

pengzhao-intel commented Jan 23, 2019

xinyu-intel commented Jan 23, 2019

jitMatrix commented Jan 23, 2019

pengzhao-intel commented Jan 23, 2019

hetong007 commented Jan 23, 2019

marcoabreu Jan 23, 2019

marcoabreu commented Jan 23, 2019

TaoLv commented Jan 24, 2019

pengzhao-intel commented Jan 25, 2019

pengzhao-intel left a comment

marcoabreu commented Jan 25, 2019 via email

pengzhao-intel left a comment

jitMatrix commented Jan 27, 2019

marcoabreu commented Jan 28, 2019 via email

jitMatrix commented Jan 29, 2019

jitMatrix commented Jan 30, 2019

jitMatrix commented Jan 30, 2019

pengzhao-intel Jan 30, 2019

jitMatrix Jan 31, 2019

Fix MXNet R package build #13952

Fix MXNet R package build #13952

Conversation

jitMatrix commented Jan 22, 2019 • edited Loading

Description

Checklist

Essentials

Changes

Comments

pengzhao-intel commented Jan 23, 2019

xinyu-intel commented Jan 23, 2019

jitMatrix commented Jan 23, 2019

pengzhao-intel commented Jan 23, 2019

hetong007 commented Jan 23, 2019

marcoabreu Jan 23, 2019

Choose a reason for hiding this comment

marcoabreu commented Jan 23, 2019

TaoLv commented Jan 24, 2019

pengzhao-intel commented Jan 25, 2019

pengzhao-intel left a comment

Choose a reason for hiding this comment

marcoabreu commented Jan 25, 2019 via email

pengzhao-intel left a comment

Choose a reason for hiding this comment

jitMatrix commented Jan 27, 2019

marcoabreu commented Jan 28, 2019 via email

jitMatrix commented Jan 29, 2019

jitMatrix commented Jan 30, 2019

jitMatrix commented Jan 30, 2019

pengzhao-intel Jan 30, 2019

Choose a reason for hiding this comment

jitMatrix Jan 31, 2019

Choose a reason for hiding this comment

jitMatrix commented Jan 22, 2019 •

edited

Loading