Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GCC 5 Builds #2939

Merged
merged 9 commits into from
Dec 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions setup.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import glob
import os
import sys
from collections import defaultdict
from distutils.ccompiler import get_default_compiler
from distutils.version import LooseVersion

Expand Down Expand Up @@ -36,20 +37,24 @@
with open("README.md") as file:
long_description = file.read()

if check_for_openmp():
omp_args = ["-fopenmp"]
else:
omp_args = []
CPP14_CONFIG = defaultdict(
lambda: ["-std=c++14"], {"unix": ["-std=c++14"], "msvc": ["/std:c++14"]}
)
CPP03_CONFIG = defaultdict(
lambda: ["-std=c++03"], {"unix": ["-std=c++03"], "msvc": ["/std:c++03"]}
)
Comment on lines +43 to +45
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's used anywhere. At least git grep doesn't show anything. If I'm not missing anything, let's not add things that might be used, but are not.


_COMPILER = get_default_compiler()

omp_args, _ = check_for_openmp()
Copy link
Member

Choose a reason for hiding this comment

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

If we are distinguishing between compile and link flags inside check_for_openmp(), shouldn't we do the same thing during cythonization, i.e. use two separate aliases? Otherwise, why return two variables?


if os.name == "nt":
std_libs = []
else:
std_libs = ["m"]

if get_default_compiler() == "msvc":
CPP14_FLAG = ["/std:c++14"]
else:
CPP14_FLAG = ["--std=c++14"]
CPP14_FLAG = CPP14_CONFIG[_COMPILER]
CPP03_FLAG = CPP03_CONFIG[_COMPILER]

cythonize_aliases = {
"LIB_DIR": "yt/utilities/lib/",
Expand All @@ -65,6 +70,7 @@
"FIXED_INTERP": "yt/utilities/lib/fixed_interpolator.cpp",
"ARTIO_SOURCE": glob.glob("yt/frontends/artio/artio_headers/*.c"),
"CPP14_FLAG": CPP14_FLAG,
"CPP03_FLAG": CPP03_FLAG,
ax3l marked this conversation as resolved.
Show resolved Hide resolved
}

lib_exts = [
Expand Down
22 changes: 14 additions & 8 deletions setupext.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ def stdchannel_redirected(stdchannel, dest_filename):


def check_for_openmp():
"""Returns True if local setup supports OpenMP, False otherwise
"""Returns OpenMP compiler and linker flags if local setup supports
OpenMP or [], [] otherwise

Code adapted from astropy_helpers, originally written by Tom
Robitaille and Curtis McCully.
Expand All @@ -72,15 +73,17 @@ def check_for_openmp():
tmp_dir = tempfile.mkdtemp()
start_dir = os.path.abspath(".")

# TODO: test more known compilers:
# MinGW, AppleClang with libomp, MSVC, ICC, XL, PGI, ...
if os.name == "nt":
# TODO: make this work with mingw
# AFAICS there's no easy way to get the compiler distutils
# will be using until compilation actually happens
compile_flag = "-openmp"
link_flag = ""
compile_flags = ["-openmp"]
link_flags = [""]
else:
compile_flag = "-fopenmp"
link_flag = "-fopenmp"
compile_flags = ["-fopenmp"]
link_flags = ["-fopenmp"]

try:
os.chdir(tmp_dir)
Expand All @@ -93,12 +96,12 @@ def check_for_openmp():
# Compile, link, and run test program
with stdchannel_redirected(sys.stderr, os.devnull):
ccompiler.compile(
["test_openmp.c"], output_dir="objects", extra_postargs=[compile_flag]
["test_openmp.c"], output_dir="objects", extra_postargs=compile_flags
)
ccompiler.link_executable(
glob.glob(os.path.join("objects", "*")),
"test_openmp",
extra_postargs=[link_flag],
extra_postargs=link_flags,
)
output = (
subprocess.check_output("./test_openmp")
Expand Down Expand Up @@ -136,7 +139,10 @@ def check_for_openmp():
"extensions will be compiled without parallel support"
)

return using_openmp
if using_openmp:
return compile_flags, link_flags
else:
return [], []


def check_for_pyembree(std_libs):
Expand Down
1 change: 1 addition & 0 deletions yt/geometry/particle_oct_container.pyx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# distutils: include_dirs = LIB_DIR_EWAH
# distutils: language = c++
# distutils: extra_compile_args = CPP14_FLAG
# distutils: libraries = STD_LIBS
"""
Oct container tuned for Particles
Expand Down
2 changes: 1 addition & 1 deletion yt/utilities/lib/cykdtree/kdtree.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# distutils: sources = yt/utilities/lib/cykdtree/c_utils.cpp
# distutils: depends = yt/utilities/lib/cykdtree/c_kdtree.hpp, yt/utilities/lib/cykdtree/c_utils.hpp
# distutils: language = c++
# distutils: extra_compile_args = -std=c++03
# distutils: extra_compile_args = CPP03_FLAG
ax3l marked this conversation as resolved.
Show resolved Hide resolved
import cython
import numpy as np

Expand Down
2 changes: 1 addition & 1 deletion yt/utilities/lib/cykdtree/utils.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# distutils: sources = yt/utilities/lib/cykdtree/c_utils.cpp
# distutils: depends = yt/utilities/lib/cykdtree/c_utils.hpp
# distutils: language = c++
# distutils: extra_compile_args = -std=c++03
# distutils: extra_compile_args = CPP03_FLAG
ax3l marked this conversation as resolved.
Show resolved Hide resolved
import numpy as np

cimport cython
Expand Down
4 changes: 2 additions & 2 deletions yt/utilities/lib/cyoctree.pyx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# distutils: libraries = STD_LIBS
# distutils: extra_link_args = OMP_ARGS
# distutils: extra_compile_args = OMP_ARGS
# distutils: extra_link_args = CPP14_FLAG OMP_ARGS
# distutils: extra_compile_args = CPP14_FLAG OMP_ARGS
# distutils: include_dirs = LIB_DIR
# distutils: language = c++
"""
Expand Down
14 changes: 12 additions & 2 deletions yt/utilities/lib/field_interpolation_tables.pxd
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# distutils: language = c++
# distutils: extra_compile_args = CPP14_FLAG
# distutils: extra_link_args = CPP14_FLAG
"""
Field Interpolation Tables

Expand All @@ -8,12 +11,19 @@ Field Interpolation Tables

cimport cython
cimport numpy as np
from libc.math cimport isnormal
from libc.stdlib cimport malloc

from yt.utilities.lib.fp_utils cimport fabs, fclip, fmax, fmin, iclip, imax, imin


cdef extern from "<cmath>" namespace "std":
bint isnormal(double x) nogil
Comment on lines +19 to +20
Copy link
Member

@cphyc cphyc Oct 15, 2020

Choose a reason for hiding this comment

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

Note: for some reason, Visual Studio doesn't happy about this. However, if we replace by

Suggested change
cdef extern from "<cmath>" namespace "std":
bint isnormal(double x) nogil
from libc.math cimport isnormal

or

Suggested change
cdef extern from "<cmath>" namespace "std":
bint isnormal(double x) nogil
cdef extern from "<math.h>":
bint isnormal(double) nogil

it compiles.



cdef extern from "platform_dep_math.hpp":
bint __isnormal(double) nogil


cdef struct FieldInterpolationTable:
# Note that we make an assumption about retaining a reference to values
# externally.
Expand Down Expand Up @@ -58,7 +68,7 @@ cdef inline np.float64_t FIT_get_value(const FieldInterpolationTable *fit,
cdef np.float64_t dd, dout
cdef int bin_id
if dvs[fit.field_id] >= fit.bounds[1] or dvs[fit.field_id] <= fit.bounds[0]: return 0.0
if not isnormal(dvs[fit.field_id]): return 0.0
if not __isnormal(dvs[fit.field_id]): return 0.0
bin_id = <int> ((dvs[fit.field_id] - fit.bounds[0]) * fit.idbin)
bin_id = iclip(bin_id, 0, fit.nbins-2)

Expand Down
5 changes: 3 additions & 2 deletions yt/utilities/lib/geometry_utils.pyx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# distutils: libraries = STD_LIBS
# distutils: extra_compile_args = OMP_ARGS
# distutils: extra_link_args = OMP_ARGS
# distutils: language = c++
# distutils: extra_compile_args = CPP14_FLAG OMP_ARGS
ax3l marked this conversation as resolved.
Show resolved Hide resolved
# distutils: extra_link_args = CPP14_FLAG OMP_ARGS
"""
Simple integrators for the radiative transfer equation

Expand Down
9 changes: 3 additions & 6 deletions yt/utilities/lib/grid_traversal.pyx
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# distutils: include_dirs = LIB_DIR
# distutils: libraries = STD_LIBS
# distutils: sources = FIXED_INTERP
# distutils: language = c++
# distutils: extra_compile_args = CPP14_FLAG
# distutils: extra_link_args = CPP14_FLAG
"""
Simple integrators for the radiative transfer equation

Expand All @@ -13,12 +16,6 @@ import numpy as np

cimport cython
cimport numpy as np
from field_interpolation_tables cimport (
FieldInterpolationTable,
FIT_eval_transfer,
FIT_eval_transfer_with_light,
FIT_initialize_table,
)
from fixed_interpolator cimport *
from libc.math cimport (
M_PI,
Expand Down
4 changes: 2 additions & 2 deletions yt/utilities/lib/image_samplers.pyx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# distutils: include_dirs = LIB_DIR
# distutils: extra_compile_args = OMP_ARGS
# distutils: extra_link_args = OMP_ARGS
# distutils: extra_compile_args = CPP14_FLAG OMP_ARGS
# distutils: extra_link_args = CPP14_FLAG OMP_ARGS
# distutils: libraries = STD_LIBS
# distutils: sources = FIXED_INTERP
# distutils: language = c++
Expand Down
5 changes: 3 additions & 2 deletions yt/utilities/lib/misc_utilities.pyx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# distutils: libraries = STD_LIBS
# distutils: extra_compile_args = OMP_ARGS
# distutils: extra_link_args = OMP_ARGS
# distutils: language = c++
# distutils: extra_compile_args = CPP14_FLAG OMP_ARGS
# distutils: extra_link_args = CPP14_FLAG OMP_ARGS
"""
Simple utilities that don't fit anywhere else

Expand Down
2 changes: 2 additions & 0 deletions yt/utilities/lib/partitioned_grid.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
# distutils: include_dirs = LIB_DIR
# distutils: libraries = STD_LIBS
# distutils: language = c++
# distutils: extra_compile_args = CPP14_FLAG
# distutils: extra_link_args = CPP14_FLAG
"""
Image sampler definitions

Expand Down
4 changes: 2 additions & 2 deletions yt/utilities/lib/pixelization_routines.pyx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# distutils: include_dirs = LIB_DIR
# distutils: extra_compile_args = OMP_ARGS
# distutils: extra_link_args = OMP_ARGS
# distutils: extra_compile_args = CPP14_FLAG OMP_ARGS
# distutils: extra_link_args = CPP14_FLAG OMP_ARGS
# distutils: language = c++
# distutils: libraries = STD_LIBS
# distutils: sources = yt/utilities/lib/pixelization_constants.cpp
Expand Down
4 changes: 1 addition & 3 deletions yt/utilities/lib/platform_dep.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,4 @@ double erf(double x)
#include <stdint.h>
#include "alloca.h"
#include <math.h>
#endif


#endif
18 changes: 18 additions & 0 deletions yt/utilities/lib/platform_dep_math.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
This file provides a compatibility layout between MSVC, and different version of GCC.

MSVC does not define isnormal in the std:: namespace, so we cannot import it from <cmath>, but from <math.h> instead.
However with GCC-5, there is a clash between the definition of isnormal in <math.h> and using C++14, so we need to import from cmath instead.
*/

#if _MSC_VER
#include <math.h>
inline bool __isnormal(double x) {
return isnormal(x);
}
#else
#include <cmath>
inline bool __isnormal(double x) {
return std::isnormal(x);
}
#endif