[MXNET-115] USE_LAPACK is forced on all platforms with OpenBLAS and c…

[larroy]

…make

Description

(Brief description on what this PR is about)

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

[lebeg]

For this change (assuming we want to keep the lapack functionality) you have to remove the hacks for arm cross compilations ie -DUSE_LAPACK=ON. With the old version is was faulty but forced on all platforms, with this change the option becomes effective.

[lebeg]

There is no separate liblapack for openblas as far as I understood. Is there for other libraries other than OpenBLAS? As far as I can tell CI checks only for OpenBLAS.

[larroy]

I don't get your comment, I tried to fix the logic as I understood the previous logic was working around adding these flags in the windows case and breaking other platforms. Can you elaborate?

[lebeg]

list(APPEND mxnet_LINKER_LIBS lapack) line adds liblapack to required linker libraries. But there is no such think as liblapack for OpenBLAS, at least not for our builds. The CI passed in this case because with this change -DUSE_LAPACK=OFF becomes effective.

[lebeg]

And there is a chance that liblapack exists for other blas libraries (atlas?) but we are not testing them in CI currently

[lebeg]

With your change you need to set -DUSE_LAPACK=ON in arm builds not to loose lapack functionality.

[szha]

Just create symlinks such as libcblas and liblapack to the compiled libopenblas. There’s no need to hack around.

[lebeg]

Ok, this might be an option

[larroy]

Thanks for the clarification, I got now what was the concern. We need better ARM / ANDROID architecture detection when cross compiling in CMake. This might be something we can definitely refine later. For the moment I'm fine with either solution proposed by @lebeg or @szha I think we will need to refine and improve this logic on future PRS.

[chinakook]

Mkl also has lapack95, but it’s not supported by mxnet.

[larroy]

ping