You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/09/14 13:01:56 UTC

[GitHub] [incubator-mxnet] akarbown opened a new pull request #19140: [BUGFIX] Fix for MKL symbol name clashing issue (#18855)

akarbown opened a new pull request #19140:
URL: https://github.com/apache/incubator-mxnet/pull/19140


   ## Description ##
   This change enables symbol exclusion of statically
   linked MKL libraries to avoid the name clashing issue #18855.
   
   ## Checklist ##
   ### Essentials ###
   - [ ] PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
   - [ ] Changes are complete (i.e. I finished coding on this PR)
   - [ ] Code is well-documented
   


----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] akarbown commented on pull request #19140: [BUGFIX] Fix for MKL symbol name clashing issue (#18855)

Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #19140:
URL: https://github.com/apache/incubator-mxnet/pull/19140#issuecomment-696006289


   > This option applies globally and affects all libraries statically linked into libmxnet.so. Thus the behavior should not depend on if MKL is disabled or enabled.
   > 
   > Also note that the option is not supported on Mac, so we may need to use a explicit symbol whitelist for consistency. #19057 has a similar change affecting only the distribution (think pypi) scripts.
   
   Hi @leezu 
   Thanks for the review!
   I see that you've done all the changes needed to avoid symbol clashing issue but for mxnet distribution version. I'd like to ask you whether it should be done for all the build types not only for distribution? So that all the developers would have it by default.
   


----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] akarbown commented on pull request #19140: [BUGFIX] Fix for MKL symbol name clashing issue (#18855)

Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #19140:
URL: https://github.com/apache/incubator-mxnet/pull/19140#issuecomment-696006289


   > This option applies globally and affects all libraries statically linked into libmxnet.so. Thus the behavior should not depend on if MKL is disabled or enabled.
   > 
   > Also note that the option is not supported on Mac, so we may need to use a explicit symbol whitelist for consistency. #19057 has a similar change affecting only the distribution (think pypi) scripts.
   
   Hi @leezu 
   Thanks for the review!
   I see that you've done all the changes needed to avoid symbol clashing issue but for mxnet distribution version. I'd like to ask you whether it should be done for all the build types not only for distribution? So that all the developers would have it by default.
   


----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] leezu commented on pull request #19140: [BUGFIX] Fix for MKL symbol name clashing issue (#18855)

Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #19140:
URL: https://github.com/apache/incubator-mxnet/pull/19140#issuecomment-699077950


   For example, the current approach won't work on macOS.
   
   To support macOS, you could switch the following lines to always apply (not only in the distribution build):
   
   https://github.com/apache/incubator-mxnet/blob/b225fa5d3c9b6d9ba10533fcbf20a20c6aa96aff/CMakeLists.txt#L673-L677


----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19140: [BUGFIX] Fix for MKL symbol name clashing issue (#18855)

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19140:
URL: https://github.com/apache/incubator-mxnet/pull/19140#issuecomment-692036515


   Hey @akarbown , Thanks for submitting the PR 
   All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands: 
   - To trigger all jobs: @mxnet-bot run ci [all] 
   - To trigger specific jobs: @mxnet-bot run ci [job1, job2] 
   *** 
   **CI supported jobs**: [centos-gpu, sanity, windows-cpu, unix-gpu, unix-cpu, website, centos-cpu, edge, windows-gpu, miscellaneous, clang]
   *** 
   _Note_: 
    Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin. 
   All CI tests must pass before the PR can be merged. 
   


----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] leezu commented on pull request #19140: [BUGFIX] Fix for MKL symbol name clashing issue (#18855)

Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #19140:
URL: https://github.com/apache/incubator-mxnet/pull/19140#issuecomment-697487742


   @akarbown yes, it's fine to enable by default. The main challenge is that you need to ensure not to break several special cases, such as compilation on different operating systems and different compilers.


----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] akarbown closed pull request #19140: [BUGFIX] Fix for MKL symbol name clashing issue (#18855)

Posted by GitBox <gi...@apache.org>.
akarbown closed pull request #19140:
URL: https://github.com/apache/incubator-mxnet/pull/19140


   


----------------------------------------------------------------
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