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

Add -DMXNET_USE_OPENMP to Makefiles so runtime info gets updated accordingly #15498

Merged
merged 1 commit into from
Jul 30, 2019

Conversation

larroy
Copy link
Contributor

@larroy larroy commented Jul 9, 2019

Description

Fixes OPENMP info in runtime features.

https://github.com/apache/incubator-mxnet/blob/master/python/mxnet/runtime.py

This would be also needed to fix BLAS flags.
#15424

@larroy larroy requested a review from szha as a code owner July 9, 2019 21:56
@karan6181
Copy link
Contributor

@mxnet-label-bot add [Installation, Makefile, pr-awaiting-review]

Copy link
Member

@szha szha left a comment

Choose a reason for hiding this comment

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

LGTM. let's also add a test to a build that has openmp enabled and check whether the runtime info is available.

@larroy
Copy link
Contributor Author

larroy commented Jul 22, 2019

This is a bit of a departure of how we are testing since we are not testing depending on specific build flags (other than GPU and CPU which we could remove now thanks to libinfo). In that case we should also do all the permutation builds and check the build flags and also for Make and Cmake. This means adding quite a number of recompilations to test this. I'm not sure is worth the increased cost and trouble. Is it justified for you?

@larroy
Copy link
Contributor Author

larroy commented Jul 22, 2019

@mxnet-label-bot add Bug

@larroy
Copy link
Contributor Author

larroy commented Jul 22, 2019

@mxnet-label-bot add [Bug]

@marcoabreu marcoabreu added the Bug label Jul 22, 2019
@szha szha merged commit 0f28f5b into apache:master Jul 30, 2019
anirudhacharya pushed a commit to anirudhacharya/mxnet that referenced this pull request Aug 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants