Closed mbargull closed 6 years ago
Hi! This is the friendly automated conda-forge-linting service.
I just wanted to let you know that I linted all conda-recipes in your PR (recipe
) and found it was in an excellent condition.
Oops, didn't look at gh-13 before in which equal/similar changes had been made...
Well, let's just wait for the CI and see if the amount of test failures changed from the 0.39.0
PR.
@mbargull Thanks for helping! Looks like there are no tests failures so far. I will wait for final CI tests to pass before merging the PR.
Neat! I didn't expect this to go so smoothly :).
@stuartarchibald: Two questions:
mandel.py
instead of run_test.py
) and not referenced anywhere AFAICT.conda-forge::tbb
is available, should we add tbb
to the build (meaning host
) and run
requirements? Any reason not to?Other than that, no unexpected failures here, but some warnings:
I believe mandel.py
can be removed, since we run tests from numba.runtests
for checking the recipe.
Right, I'd be surprised if mandel.py
adds anything significant not covered by the test suite :laughing:.
@souravsingh, do you have any insights in regards to whether there is any argument against building with TBB?
If the package is not too big , I would support using TBB. I would reserve final decision upto the numba devs, since they might now if there are any problems.
Alright, thanks. I just opened gh-16 so we can let the CI take a look at it :wink:
@mbargull thanks for taking a look at this. To your questions...
- Is the Mandelbrot test that is part of the recipe still supposed to be run or can it be removed? Currently it's just being copied into the test environment but not run (being named mandel.py instead of run_test.py) and not referenced anywhere AFAICT.
I don't think it does anything and its existence is an artefact of development. Git grep doesn't show anything enlightening.
Since conda-forge::tbb is available, should we add tbb to the build (meaning host) and run requirements? Any reason not to?
Numba 0.40.0 contains a rewrite of the threading backend (used by all parallel targets on the CPU). As part of that rewrite there are now three implementations of the threadpool library used by the backend, one using TBB, one using OpenMP and one is a simple threaded workqueue. The following describes the build options provided by various packages, the Anaconda distribution builds all the threading libraries by virtue of correctly supplying all the dependencies at build time. The reality is that the Numba build system is pretty tolerant of missing dependencies if e.g. TBB is not present at build time, then that threadpool will simply not be built.
If at build time the TBB development headers and libraries are available (Anaconda distro has them in the tbb-devel
package), then Numba will build the TBB backed threadpool. There is no runtime requirement for TBB, if a user tries to use the backend in a way that needs TBB and TBB cannot be found, they will be told to go and install it.
If at build time a working OpenMP implementation if found, then Numba will build the OpenMP backed threadpool.
llvm-openmp
and intel-openmp
packages respectively, in the Anaconda distribution). This is because at compile time the threading library needs OpenMP headers which can be provided by llvm-openmp
, and at link time intel-openmp
libraries are used as these are binary compatible with llvm-openmp
libraries but both implementations may not safely exist in the same address space. As Intel OpenMP is a dependency of MKL (in the Anaconda distro, Numba uses NumPy which uses MKL which uses Intel OpenMP) this library juggling is required so as to get a dependency on Intel OpenMP. As conda-forge uses OpenBLAS and a build which doesn't look like it's OpenMP aware, along with only the presence of llvm-openmp https://github.com/conda-forge/openmp-feedstock as a potential source of OpenMP then I think setup.py
will need a patch to handle simply using llvm-openmp
and the meta.yaml
test dependencies will need llvm-openmp
for OSX. Having written this I realise that this got quite complicated, please ping me to assist if needed!intel-openmp
implementation cannot be found, they will be told to go and install it.The workqueue backend just works without dependency, it's not materially different to what has shipped in numerous previous releases.
To the warnings, thanks for raising these... replies are inlined.
everywhere:
lib/python3.6/runpy.py:125: RuntimeWarning: 'numba.runtests' found in sys.modules after import of package 'numba', but prior to execution of 'numba.runtests'; this may result in unpredictable behaviour warn(RuntimeWarning(msg))``` This is more or less just a nuisance since numba.runtests uses if `__name__ == '__main__'` for the non-declarative part. If you wanted to get rid of those warnings, you could put the declarations into a separate module, e.g., `numba._runtests`, and import them from that module in `numba.runtests` and `numba.__init__` so to remove the `numba.runtests import in numba.__init__`.
Yes, this has been around a while, it's harmless but a nuisance as noted. Issue to fix is now here https://github.com/numba/numba/issues/3360, thanks for the suggestion.
build_linux_python2.7 and build_linux_python3.5: cc1plus: warning: command line option ‘-Wstrict-prototypes’ is valid for C/ObjC but not for C++ > [enabled by default]
Think that's harmless, bit strange it's not for all Python's as the toolchain ought to be the same. Think it's probably distutils related as it's coming from
pycc
, the AOT compiler setup tries to compile some source with a.cxx
extension using standardcc
when checking the toolchain works. Issue to fix: https://github.com/numba/numba/issues/3361
build_linux_python2.7: Successfully built numba Installing collected packages: numba Compiling /tmp/pip-install-OTYJOa/numba/numba/tests/annotation_usecases.py ... File "/tmp/pip-install-OTYJOa/numba/numba/tests/annotation_usecases.py", line 12 def __init__(self, v: int): ^ SyntaxError: invalid syntax Packaging numba INFO:conda_build.build:Packaging numba Packaging numba-0.40.0-py27hf8a1672_0 INFO:conda_build.build:Packaging numba-0.40.0-py27hf8a1672_0 compiling .pyc files... File "lib/python2.7/site-packages/numba/tests/annotation_usecases.py", line 12 def __init__(self, v: int): ^ SyntaxError: invalid syntax```
This file should be removed during the build, Numba has this in build.sh https://github.com/numba/numba/blob/master/buildscripts/condarecipe.local/build.sh#L3 which calls the script https://github.com/numba/numba/blob/master/buildscripts/remove_unwanted_files.py
cc1plus: warning: command line option ???-Wstrict-prototypes??? is valid for C/ObjC but not for C++ [enabled by default]```
Addressed above.
Exception TypeError: "'NoneType' object is not callable" in <bound method ModuleRef.__del__ of <llvmlite.binding.module.ModuleRef object at 0x7fe70a573bd0>> ignored Exception TypeError: "'NoneType' object is not callable" in <bound method ModulePassManager.__del__ of <llvmlite.binding.passmanagers.ModulePassManager object at 0x7fe70c5e80d0>> ignored Exception TypeError: "'NoneType' object is not callable" in <bound method _ObjectCacheRef.__del__ of <llvmlite.binding.executionengine._ObjectCacheRef object at 0x7fe70c5e8190>> ignored Exception TypeError: "'NoneType' object is not callable" in <bound method ExecutionEngine.__del__ of <llvmlite.binding.executionengine.ExecutionEngine object at 0x7fe70c5e81d0>> ignored ..Exception TypeError: "'NoneType' object is not callable" in <bound method ExecutionEngine.__del__ of <llvmlite.binding.executionengine.ExecutionEngine object at 0x7f69bdd811d0>> ignored ....Exception TypeError: "'NoneType' object is not callable" in <bound method ModulePassManager.__del__ of <llvmlite.binding.passmanagers.ModulePassManager object at 0x7f6bc25260d0>> ignored Exception TypeError: "'NoneType' object is not callable" in <bound method _ObjectCacheRef.__del__ of <llvmlite.binding.executionengine._ObjectCacheRef object at 0x7f6bc2526190>> ignored Exception TypeError: "'NoneType' object is not callable" in <bound method ExecutionEngine.__del__ of <llvmlite.binding.executionengine.ExecutionEngine object at 0x7f6bc25261d0>> ignored Exception TypeError: "'NoneType' object is not callable" in <bound method ModuleRef.__del__ of <llvmlite.binding.module.ModuleRef object at 0x7f6bc04b1750>> ignored
This is a known and harmless llvmlite issue to do with deletion order at shut down https://github.com/numba/llvmlite/issues/350.
build_osx_python2.7: <snip> python(2859,0x7fffe77423c0) malloc: *** mach_vm_map(size=2305843009213698048) failed (error code=3) *** error: can't allocate region *** set a breakpoint in malloc_error_break to debug``` This is fine, it comes from here: https://github.com/numba/numba/blob/3d08d9534bea0496008898e260f50522ee3cbf2e/numba/tests/test_lists.py#L685-L696 , OSX malloc is just noisy about it. <snip> rest are duplicates of the above.
@stuartarchibald, thanks for the detailed answers!
Re: TBB:
If tbb
is completely optional at runtime even if it has been used for the build, then we could
tbb >=2018.0.5
from requirements/run
in https://github.com/conda-forge/numba-feedstock/pull/16/commits/163ae446e8fcf1ae59b74dbcc9a52ebb6832e19c#diff-e178b687b10a71a3348107ae3154e44cR38.tbb >=2018.0.5
to requirements/run_constrained
.tbb
to build/ignore_run_exports
to avoid https://github.com/conda-forge/tbb-feedstock/blob/115fcdbc55f97c8d80a424013f6f06928004b466/recipe/meta.yaml#L62.Right?
Re: OpenMP: I saw people struggle with getting OpenMP to work with the old/system compilers on conda-forge. I would assume it makes sense to just wait a bit until we migrate this to use the newer Anaconda compiler packages, to save us the trouble.
Re: remove_unwanted_files.py
:
"This file should be removed during the build", correct, but remove_unwanted_files.py
isn't part of the PyPI sdist. So options are:
AnacondaRecipes
.setup.py
to remove the file (whilst taking care this is only done for the PY2 bdists but not for the sdist.)tbb
to test/requires
if we remove it from requirements/run
.@mbargull no problem.
Further, this numba/numba#3202 (comment) also bit whilst writing the new backends, which is why Numba has: https://github.com/numba/numba/blob/fbd3d11e4cd1f68a978db1df21a929fb961cf4bb/buildscripts/condarecipe.local/meta.yaml#L45 as that version is fixed.
Thanks for the pointer regarding why that version has been chosen! Plus, before I overlooked that buildscripts/condarecipe.local/meta.yaml
already uses run_constrained
for it :).
It's OSX that may require the complicated stuff.
Oh, yes, I meant to write "struggle with getting OpenMP to work on OSX", but apparently left out the important part..
I also don't think there should be a reliance on Numba moving the testing code yet! ;-)
For sure -- maybe something for (post) 1.0
;).
I updated gh-16 to make tbb
an optional run requirement and switched to using the git archive to be able to just run remove_unwanted_files.py
.
Not sure that this would actually impact conda-forge though. Ping @anton-malakhov, do you recall if conda-forge TBB builds were safe?
Even if earlier builds on conda-forge
are fine, it is always possible that defaults::tbb
gets installed for whatever reason. Hence the safest thing is to just copy the lower version boundary, IMO.
Agree, let's use this version of the tbb package even though I'm sure that that particular issue with GCC 7.3 was Anaconda-specific as it used another approach to compile tbb until they converged to the version derived pretty directly from conda-forge's recipe.
Great, thanks! I'm going ahead an close this PR in favor of gh-16 then.
0
(if the version changed)conda-smithy
Built on top of gh-14, this
llvmlite
requirement.numba
andpycc
.conda-build 3
features.