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

[MXNET-627] Fix Installation instructions for R bindings on Linux systems. #11590

Merged
merged 14 commits into from
Jul 12, 2018

Conversation

anirudhacharya
Copy link
Member

@anirudhacharya anirudhacharya commented Jul 6, 2018

Anirudh Acharya added 3 commits July 6, 2018 13:13
@anirudhacharya anirudhacharya requested a review from szha as a code owner July 6, 2018 20:24
@@ -987,10 +987,23 @@ $ make -j $(nproc) USE_OPENCV=1 USE_BLAS=openblas

**Build and install the MXNet R binding**

You will need to first install R. MXNet requires R version >3.3 and we can fetch that from CRAN package repository.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add similar instructions for windows and os x users.

Copy link
Member Author

Choose a reason for hiding this comment

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

for macOS and windows we do not build from source, this will not be there. There is only the issue of ensuring that the pre-built binary in the S3 works fine with both R 3.4 and 3.5.
And that is not part of this PR.

@anirudhacharya anirudhacharya changed the title Fix Installation instructions for R bindings for Linux CPU environment. [MXNET-627] Fix Installation instructions for R bindings for Linux CPU environment. Jul 6, 2018
@aaronmarkham
Copy link
Contributor

Follow these directions to setup a test server and build the docs. Test the selector to make sure content shows up when you click on the different install options.

https://github.com/apache/incubator-mxnet/blob/master/docs/build_version_doc/README.md#setting-up-a-docs-dev-server

@anirudhacharya
Copy link
Member Author

anirudhacharya commented Jul 9, 2018

@anirudhacharya anirudhacharya changed the title [MXNET-627] Fix Installation instructions for R bindings for Linux CPU environment. [MXNET-627] Fix Installation instructions for R bindings on Linux systems. Jul 9, 2018
@aaronmarkham
Copy link
Contributor

So it looks like we have duplicated information on the install.md page and each of the $OS_setup.md pages. Also, when you use the install page's selector, you get instructions, but no link to the $OS_setup page.

Wouldn't it be better to provide the minimum info needed on the install page, and leave the detail for the OS specific pages?

@anirudhacharya
Copy link
Member Author

@aaronmarkham you have been added as a collaborator to my repo. Please make the required changes and push to this branch. Thanks!

@aaronmarkham
Copy link
Contributor

aaronmarkham commented Jul 10, 2018

I went ahead and resolved an issue with using the CI scripts as part of the installation instructions (@marcoabreu) for Ubuntu and made a new script for core and python deps. The "pre-pip" dependency install still uses the CI script though, since it doesn't do any source steps. (I suppose I can make another one for this or modify the install_mxnet_ubuntu_python.sh to have pip and source options.)

I also made an R installation script (ported and updated the dmlc one to work with our install flow).

The instructions are generally simplified now by having access to the two new shell scripts:

install_mxnet_ubuntu_python.sh

  • It checks for nvidia-smi and sets the make flags accordingly, so now we don't have doubled up instructions. It will build GPU or CPU depending, which I think is pretty handy. No need for complicated instructions.
  • I moved the pip installs to a central requirements.txt so that's not duplicated anymore. Note: these could be adjusted.
  • It uses sudo in the pip upgrade step - I didn't find a way around this.

install_mxnet_ubuntu_r.sh

  • The R deps are now in alignment - I updated CI for R to match.

@aaronmarkham
Copy link
Contributor

@hetong007 I updated the install page for OSX and Windows to point to the detailed instructions pages. OSX should be good to go now. Windows is being worked on in a separate PR by @ankkhedia. Please revisit your change request since further updates on Windows instructions is out of scope for this PR.


```bash
$ cd incubator-mxnet/R-package
Copy link
Member Author

Choose a reason for hiding this comment

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

you run make rpkg from incubator-mxnet folder not incubator-mxnet/R-package

@szha szha merged commit ea24c1c into apache:master Jul 12, 2018
@@ -20,17 +20,31 @@
# build and install are separated so changes to build don't invalidate
# the whole docker cache for the image

# Important Maintenance Instructions:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably unintended, but the change to this file causes CI to occasionally fail. The keyserver for this gpg key fails approx 1/10 requests. Is there a good reason we reverted @larroy change which manually installs the key? It seemed like a reasonable fix for the flaky gpg server issue. Any objection to me restoring the manual key add?

@anirudhacharya anirudhacharya deleted the fixCpuR branch August 13, 2018 19:59
XinYao1994 pushed a commit to XinYao1994/incubator-mxnet that referenced this pull request Aug 29, 2018
…tems. (apache#11590)

* fix doc

* fix

* fix

* fix

* gpu

* nit

* new ubuntu and r install scripts and docs; minor osx update

* script now uses requirements.txt

* removed unneeded commands

* fixed path for make rpkg
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants