[MXNET-43] Fix Jetson compilation #13532

larroy · 2018-12-04T16:24:23Z

Description

Fix NVidia Jetson builds and add to CI

Was disabled in PR:
#13402

Fixes #13440

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

larroy · 2018-12-04T18:43:31Z

@KellenSunderland

marcoabreu · 2018-12-04T19:56:17Z

ci/docker/Dockerfile.build.jetson

@@ -77,6 +77,7 @@ RUN JETPACK_DOWNLOAD_PREFIX=https://developer.download.nvidia.com/devzone/devcen
    dpkg -i --force-architecture  $ARM_NVINFER_INSTALLER_PACKAGE && \
    dpkg -i --force-architecture  $ARM_NVINFER_DEV_INSTALLER_PACKAGE && \
    apt update -y || true && apt install -y cuda-libraries-dev-9-0 libcudnn7-dev libnvinfer-dev
+RUN cd /usr/include/aarch64-linux-gnu/ && ln -s cudnn_v7.h cudnn.h


Can we create the symlink without changing the current work directory?

Because that adds additional side effects to this command. If there's no reason to introduce side effects, you generally would want to avoid it. For that symlink, I don't see much reason.
Otherwise, follow up commands might now write to that new path since they assume an unchanged path.

I asked "why not?" meaning, sure why not. I agreed with you, sorry for the misunderstanding, my bad.

lebeg · 2018-12-05T14:27:04Z

make/crosscompile.jetson.mk

@@ -57,10 +57,10 @@ DEBUG = 0
 USE_SIGNAL_HANDLER = 1

 # the additional link flags you want to add
-ADD_LDFLAGS = -L${CROSS_ROOT}/lib
+ADD_LDFLAGS = -L${CROSS_ROOT}/lib -L/usr/lib/aarch64-linux-gnu/


This is strange that this is needed, ${CROSS_ROOT} should be enough... What's the value for ${CROSS_ROOT}?

It's different, I don't remember. If it wasn't needed I woulnd't have put it there. I checked what you said before I had the same thought.

There can be 2 options: host libraries and cross compile libraries. By mixing both we might get in trouble. I suggest to find out why ${CROSS_ROOT}/lib is not working (maybe try without /lib?)

Yeah I like that thought. I believe you Pedro that you checked that, but I think it's still a good idea to have your investigation documented because I also struggled at finding the right headers. Describing your thoughts helps others to understand how and what you did and might also offer the possibility to get maybe improve the solution :)

@lebeg As I said, CROSS_ROOT != /usr/lib/aarch64-linux-gnu/ I checked before, why do you want me to investigate again? /usr/lib/aarch64-linux-gnu/ is where nvidia packages go, there's no need to investigate further in my opinion.

/usr/lib/aarch64-linux-gnu/ is where nvidia packages go, there's no need to investigate further in my opinion.

I think that nvidia packages (if they are not in ${CROSS_ROOT}/lib) should be installed or symlinked there instead.

/usr/lib/aarch64-linux-gnu/ is too generic (based on the name alone) to contain only CUDA libraries. Therefore it's confusing and is complicating the build in my opinion, which can lead to host/target library mixtures.

That's a good point. I would suggest you send a PR a posteriori improving this. I think the Jetson & dockcross build is a bit hacky anyway. My PR fixes cross compilation, further refactor and improvements are out scope of this fix.

@marcoabreu what should I document? tomorrow the nvidia packages will put the library in some other folder and this will break again and the comment will be outdated. You go into the container and look for the library there's not much to it.

Maybe let's get @KellenSunderland input as he is more knowledgeable of Jetson and dockcross.

KellenSunderland

LGTM, can you rebase it once more to make sure it's up-to-date with CI changes. I'll keep an eye on it and merge when it passes.

larroy · 2018-12-13T22:32:07Z

Done. Can we merge?

larroy · 2018-12-16T23:31:02Z

@marcoabreu can we merge? @KellenSunderland

This reverts commit 48e25c4.

* Revert "remove omp which can cause ssd accuracy variance (#13622)" This reverts commit 655f1c6. * Revert "Fix Jetson compilation (#13532)" This reverts commit 48e25c4.

* Revert "remove omp which can cause ssd accuracy variance (apache#13622)" This reverts commit 655f1c6. * Revert "Fix Jetson compilation (apache#13532)" This reverts commit 48e25c4.

larroy force-pushed the jetson_fix branch from 1bf2b9d to 4455ead Compare December 4, 2018 17:21

larroy changed the title ~~Fix Jetson compilation~~ [MXNET-43] Fix Jetson compilation Dec 4, 2018

marcoabreu reviewed Dec 4, 2018

View reviewed changes

larroy force-pushed the jetson_fix branch from 4455ead to d3f3e9f Compare December 5, 2018 13:51

larroy mentioned this pull request Dec 5, 2018

[MXNET-43] Fix Jetson compilation (v1.4.x branch) #13547

Merged

5 tasks

lebeg reviewed Dec 5, 2018

View reviewed changes

nswamy added the pr-awaiting-review PR is waiting for code review label Dec 7, 2018

jlcontreras mentioned this pull request Dec 7, 2018

Updated dockerfiles to get the dockcross images from mxnetcipinned #13562

Merged

KellenSunderland approved these changes Dec 11, 2018

View reviewed changes

Fix Jetson compilation

a8c2c34

larroy force-pushed the jetson_fix branch from d3f3e9f to a8c2c34 Compare December 11, 2018 16:48

KellenSunderland merged commit 48e25c4 into apache:master Dec 17, 2018

KellenSunderland added a commit that referenced this pull request Dec 17, 2018

Revert "Fix Jetson compilation (#13532)"

58e9d01

This reverts commit 48e25c4.

KellenSunderland mentioned this pull request Dec 17, 2018

Revert "[MXNET-43] Fix Jetson compilation" #13665

Merged

mseth10 pushed a commit to mseth10/incubator-mxnet that referenced this pull request Dec 18, 2018

Fix Jetson compilation (apache#13532)

e2fe081

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MXNET-43] Fix Jetson compilation #13532

[MXNET-43] Fix Jetson compilation #13532

larroy commented Dec 4, 2018 •

edited

Loading

larroy commented Dec 4, 2018

marcoabreu Dec 4, 2018

larroy Dec 4, 2018

marcoabreu Dec 5, 2018

larroy Dec 5, 2018 •

edited

Loading

lebeg Dec 5, 2018

larroy Dec 5, 2018

lebeg Dec 5, 2018

marcoabreu Dec 5, 2018

larroy Dec 5, 2018

lebeg Dec 5, 2018

lebeg Dec 5, 2018

larroy Dec 6, 2018

larroy Dec 7, 2018

larroy Dec 7, 2018

KellenSunderland left a comment

larroy commented Dec 13, 2018

larroy commented Dec 16, 2018

[MXNET-43] Fix Jetson compilation #13532

[MXNET-43] Fix Jetson compilation #13532

Conversation

larroy commented Dec 4, 2018 • edited Loading

Description

Checklist

Essentials

larroy commented Dec 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larroy Dec 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KellenSunderland left a comment

Choose a reason for hiding this comment

larroy commented Dec 13, 2018

larroy commented Dec 16, 2018

larroy commented Dec 4, 2018 •

edited

Loading

larroy Dec 5, 2018 •

edited

Loading