[GitHub] [incubator-mxnet] nuslq opened a new pull request #15424: fixed config.mk and Makefile bugs for installing mkl

[GitBox <gi...@apache.org>]

nuslq opened a new pull request #15424: fixed config.mk and Makefile bugs for installing mkl
URL: https://github.com/apache/incubator-mxnet/pull/15424
 
 
   ## Description ##
   (Brief description on what this PR is about)
   I have found two bugs in the Makefile file and make/config.mk file in apache/incubator-mxnet package. These bugs might lead to incorrect installation of Mxnet with MKL library. 
   
   Before the changes, I could not find "BLAS_MKL" in mxnet.runtime.Features() outputs. After rebuilt my mxnet with the following changes, I found "BLAS_MKL" in mxnet.runtime.Features() outputs and the model inference speed I was benchmarking increased by around 35%.
   
   In the make/config.mk file, USE_BLAS will be first set to "atlas" for linux or "apple" for osx by default as follows (lines 119 - 124)
   
   ```
   UNAME_S := $(shell uname -s)
   ifeq ($(UNAME_S), Darwin)
   USE_BLAS = apple
   else
   USE_BLAS = atlas
   endif
   ```
   
   Later, the USE_STATIC_MKL would be set "NONE" if users did not set USE_BLAS to "mkl" before the following lines (lines 137 - 142). This would not be corrected even if users set "USE_BLAS=mkl" at the end of the config.mk file or in the command line.
   ```
   # If use MKL only for BLAS, choose static link automatically to allow python wrapper
   ifeq ($(USE_BLAS), mkl)
   USE_STATIC_MKL = 1
   else
   USE_STATIC_MKL = NONE
   endif
   ```
   
   So, I correct this block to the follows,
   
   ```
   # If use MKL only for BLAS, choose static link automatically to allow python wrapper
   # Please Note: You have to set USE_BLAS = mkl here if you want to build mxnet with mkl. Otherwise USE_STATIC_MKL will be set to NONE. 
   # USE_BLAS = mkl 
   ifeq ($(USE_BLAS), mkl)
   USE_STATIC_MKL = 1
   else
   USE_STATIC_MKL = NONE
   endif
   ```
   
   In the Makefile, lines 247 - 255 as follows, I corrected the "use_blas" to capital form. 
   
   ```
   ifeq ($(use_blas), open)
   	CFLAGS += -DMXNET_USE_BLAS_OPEN=1
   else ifeq ($(use_blas), atlas)
   	CFLAGS += -DMXNET_USE_BLAS_ATLAS=1
   else ifeq ($(use_blas), mkl)
   	CFLAGS += -DMXNET_USE_BLAS_MKL=1
   else ifeq ($(use_blas), apple)
   	CFLAGS += -DMXNET_USE_BLAS_APPLE=1
   endif
   ```
   
   ## 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](https://issues.apache.org/jira/projects/MXNET/issues) created (except PRs with tiny changes)
   - [x] 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
   - [x] To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change
   
   ### Changes ###
   - [x] In the make/config.mk file, add # USE_BLAS = mkl  and comments for setting correct USE_STATIC_MKL as described in Description section
   - [x] In the Makefile, lines 247 - 255 as follows, I corrected the "use_blas" to capital form
   
   ## Comments ##
   - This change should be backward compatible.
   - Don't see any edge cases
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services