Fix compilation error

[hckr]

Had an error with installation on Linux (see below), my addition of -Wno-narrowing compilation flag fixes it.

$ pip install -e git+https://github.com/xoolive/geodesy.git#egg=geodesy Obtaining geodesy from git+https://github.com/xoolive/geodesy.git#egg=geodesy Updating ./venv/src/geodesy clone Installing collected packages: geodesy Running setup.py develop for geodesy Complete output from command /home/luke/Programowanie/Python/LocalizeFriends/backend/venv/bin/python3 -c "import setuptools, tokenize;file='/home/luke/Programowanie/Python/LocalizeFriends/backend/venv/src/geodesy/setup.py';f=getattr(tokenize, 'open', open)(file);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, file, 'exec'))" develop --no-deps: running develop running egg_info writing geodesy.egg-info/PKG-INFO writing dependency_links to geodesy.egg-info/dependency_links.txt writing top-level names to geodesy.egg-info/top_level.txt reading manifest file 'geodesy.egg-info/SOURCES.txt' writing manifest file 'geodesy.egg-info/SOURCES.txt' running build_ext building 'geodesy.sphere' extension creating build creating build/temp.linux-x86_64-3.6 creating build/temp.linux-x86_64-3.6/geodesy gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fPIC -I/home/luke/Programowanie/Python/LocalizeFriends/backend/venv/lib/python3.6/site-packages/numpy/core/include -I/usr/include/python3.6m -c geodesy/sphere.cpp -o build/temp.linux-x86_64-3.6/geodesy/sphere.o -Wno-unused-function cc1plus: warning: command line option ‘-Wstrict-prototypes’ is valid for C/ObjC but not for C++

In file included from geodesy/sphere.cpp:472:0:
geodesy/../src/spherical_geodesy.h:54:44: warning: ignoring attributes on template argument ‘__m128 {aka __vector(4) float}’ [-Wignored-attributes]
   template<> __m128 SphericalGeodesy<__m128>::EarthRadius =

geodesy/../src/sse2_math.h:114:68: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
 static const __m128d _mm_cst_pd_sincof_p4 = _mm_set1_pd(*(double*) sincof4);
                                                                    ^~~~~~~

geodesy/../src/sse2_math.h:144:68: error: narrowing conversion of ‘51019’ from ‘int’ to ‘short int’ inside { } [-Wnarrowing]
 static const short asincof_p2[] = { 0x2dd9, 0x178a, 0xc74b, 0x4015 };
                                                                    ^
geodesy/../src/sse2_math.h:169:68: error: narrowing conversion of ‘40712’ from ‘int’ to ‘short int’ inside { } [-Wnarrowing]
 static const short asincof_r0[] = { 0x9f08, 0x988e, 0x4fc3, 0x3f68 };
                                                                    ^
geodesy/../src/spherical_geodesy.hpp:404:32: warning: ignoring attributes on template argument ‘__m128 {aka __vector(4) float}’ [-Wignored-attributes]
         SphericalGeodesy<__m128>::destination(
                                ^
error: command 'gcc' failed with exit status 1

[shortened]

[xoolive]

Thank you for your PR. I am pretty sure your commit does not break anything and can't see any reason to reject it if it works for you but I don't understand how removing a warning would fix anything. Could you be more specific about issues you meet? On what architecture (32bit/64bit) do you work? There may be a real issue to work on for a real patch and it would be sad not to take this opportunity to address it properly.

[hckr]

I use gcc 6.3.1 on 64-bit. In this version warning -Wnarrowing seems to be treated as error and causes compilation to fail.

As I took a closer look it seems there might be a real issue which is just being hidden by proposed suppresion.

short (which is a shortcut for signed short int) has the size of 16 bits minimum (max. value being 32768) but in the code much larger values are used (40712, 39054, ...):

static const short asincof_r0[] = { 0x9f08, 0x988e, 0x4fc3, 0x3f68 };

But on the other hand those values are in hex, so they might have been hand-picked and are later are e. g. treated as unsigned (in which case desclarations should be changed to unsigned) or something else. I don't know, but author should know his intensions. :)

It was surprised that library code doesn't compile and assumed that code works correctly and compiler just started to be more picky in this area.

[xoolive]

Thank you for digging in that far. The coefficients you are referring to come from here: https://github.com/jeremybarnes/cephes/blob/master/cmath/asin.c#L222

Could you maybe try with your compiler and see if you get any warnings there? I see a #if MIEEE (it seems to basically mean big endian) there, maybe I am assuming too much...

[hckr]

I digged a bit in the code and found out those values are treated as something else – doubles (sse2_math.h#L175):

static const __m128d _mm_cst_pd_asincof_r0 = _mm_set1_pd(*(double*) asincof_r0);
static const __m128d _mm_cst_pd_asincof_r1 = _mm_set1_pd(*(double*) asincof_r1);
static const __m128d _mm_cst_pd_asincof_r2 = _mm_set1_pd(*(double*) asincof_r2);
static const __m128d _mm_cst_pd_asincof_r3 = _mm_set1_pd(*(double*) asincof_r3);
static const __m128d _mm_cst_pd_asincof_r4 = _mm_set1_pd(*(double*) asincof_r4);

It looks like some high level magic which I don't understand (why wouldn't someone just define it as simple double literals?, why array of shorts wasn't defined as array of unsigned shorts?), but narrowing isn't an issue, because those arrays are interpreted as doubles anyway.

I think that changing library code isn't the wisest thing to do and personally solved the issue with a warning/error suppression.

[xoolive]

It is just a way to represent a specific double precision float by its binary representation. But as you can see, I did not invent those numbers, just copy pasted them from a different code.

The thing here is just that I try to take advantage of SSE2 registers (if available ; still have to do AVX) to make the code faster. SSE2 instruction do not provide trigonometric functions so I had to code this again... But this part can also be commented out if results seem incorrect after your warning is raised.

For now, I will just merge the PR (your line doesn't hurt!), but I am still curious about the error raised. I don't want to take it the easy way and blame it on the compiler, but still, there must be some better way to secure that part...