storpipfugl / pykdtree

Fast kd-tree implementation in Python
GNU Lesser General Public License v3.0
215 stars 47 forks source link

Make Visual C++ happy: signed loop variables in OpenMP for statement #24

Closed jmozmoz closed 6 years ago

jmozmoz commented 6 years ago

avoid error: error C3016: 'XXX': index variable in OpenMP 'for' statement must have signed integral type

(sorry for the trailing white space removal, the editor did this, but I think it should be done anyway sometimes)

jmozmoz commented 6 years ago

Just for explanation: This is the error, if I compile it using Python 3.6.1 using Windows 10, with C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\x86_amd64\cl.exe

>pip install pykdtree
Collecting pykdtree
  Using cached pykdtree-1.2.2.tar.gz
  In the tar file C:\Users\joachim\AppData\Local\Temp\pip-y4sr1cxi-unpack\pykdtree-1.2.2.tar.gz the member pykdtree-1.2.2/README is invalid: unable to resolve link inside archive
Requirement already satisfied: numpy in c:\users\joachim\testvenv\lib\site-packages (from pykdtree)
Installing collected packages: pykdtree
  Running setup.py install for pykdtree ... error
    Complete output from command c:\users\joachim\testvenv\scripts\python.exe -u -c "import setuptools, tokenize;__file__='C:\\Users\\joachim\\AppData\\Local\\Temp\\pip-build-12fezzk8\\pykdtree\\setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record C:\Users\joachim\AppData\Local\Temp\pip-ri5mqc4h-record\install-record.txt --single-version-externally-managed --compile --install-headers c:\users\joachim\testvenv\include\site\python3.6\pykdtree:
    running install
    running build
    running build_py
    creating build
    creating build\lib.win-amd64-3.6
    creating build\lib.win-amd64-3.6\pykdtree
    copying pykdtree\test_tree.py -> build\lib.win-amd64-3.6\pykdtree
    copying pykdtree\__init__.py -> build\lib.win-amd64-3.6\pykdtree
    running build_ext
    building 'pykdtree.kdtree' extension
    creating build\temp.win-amd64-3.6
    creating build\temp.win-amd64-3.6\Release
    creating build\temp.win-amd64-3.6\Release\pykdtree
    C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\x86_amd64\cl.exe /c /nologo /Ox /W3 /GL /DNDEBUG /MD -Ic:\users\joachim\testvenv\include -IC:\opt\python36\include -IC:\opt\python36\include -Ic:\users\joachim\testvenv\lib\site-packages\numpy\core\include "-IC:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE" "-IC:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\ATLMFC\INCLUDE" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.10240.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\include\um" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\shared" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\um" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\winrt" /Tcpykdtree/kdtree.c /Fobuild\temp.win-amd64-3.6\Release\pykdtree/kdtree.obj /Ox /openmp
    kdtree.c
    c:\users\joachim\testvenv\lib\site-packages\numpy\core\include\numpy\npy_1_7_deprecated_api.h(12) : Warning Msg: Using deprecated NumPy API, disable it by #defining NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
    pykdtree/kdtree.c(2327): warning C4244: '=': conversion from 'npy_intp' to 'long', possible loss of data
    pykdtree/kdtree.c(2414): warning C4244: '=': conversion from 'npy_intp' to 'uint32_t', possible loss of data
    pykdtree/kdtree.c(3038): warning C4244: 'function': conversion from 'double' to 'float', possible loss of data
    C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\x86_amd64\cl.exe /c /nologo /Ox /W3 /GL /DNDEBUG /MD -Ic:\users\joachim\testvenv\include -IC:\opt\python36\include -IC:\opt\python36\include -Ic:\users\joachim\testvenv\lib\site-packages\numpy\core\include "-IC:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE" "-IC:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\ATLMFC\INCLUDE" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.10240.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\include\um" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\shared" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\um" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\winrt" /Tcpykdtree/_kdtree_core.c /Fobuild\temp.win-amd64-3.6\Release\pykdtree/_kdtree_core.obj /Ox /openmp
    _kdtree_core.c
    pykdtree/_kdtree_core.c(708): warning C4056: overflow in floating-point constant arithmetic
    pykdtree/_kdtree_core.c(702): error C3016: 'i': index variable in OpenMP 'for' statement must have signed integral type
    pykdtree/_kdtree_core.c(702): fatal error C1903: unable to recover from previous error(s); stopping compilation
    Internal Compiler Error in C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\x86_amd64\cl.exe.  You will be prompted to send an error report to Microsoft later.
    error: command 'C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\BIN\\x86_amd64\\cl.exe' failed with exit status 1

    ----------------------------------------

see also: https://stackoverflow.com/questions/2820621/why-arent-unsigned-openmp-index-variables-allowed

storpipfugl commented 6 years ago

Thanks for the pull request. The principle looks sound but please note the _kdtree_core.c file is generated from _kdtree_core.c.mako using render_template.py (simple datatype templating) so please apply the changes to _kdtree_core.c.mako and regenerate _kdtree_core.c

Also please base the pull request against pre-master instead of master (tracks latest release)

True about the white spaces needing fixing

jmozmoz commented 6 years ago

I will create two new pull requests: one for the spaces, the other for the fix.

If the file _kdtree_core.c is generated by the rendert?tmplate.py script, shouldn't it be removed from the repository and generated each time by the build process?