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

Compiler error Werror=float-equal during build #4375

Open
geiseri opened this issue Sep 20, 2023 · 3 comments
Open

Compiler error Werror=float-equal during build #4375

geiseri opened this issue Sep 20, 2023 · 3 comments
Assignees

Comments

@geiseri
Copy link

geiseri commented Sep 20, 2023

Greetings I am on debian 12

Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/12/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 12.2.0-14' --with-bugurl=file:///usr/share/doc/gcc-12/README.Bugs --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr --with-gcc-major-version-only --program-suffix=-12 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib --enable-libphobos-checking=release --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch --disable-werror --enable-cet --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none=/build/gcc-12-bTRWOB/gcc-12-12.2.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-12-bTRWOB/gcc-12-12.2.0/debian/tmp-gcn/usr --enable-offload-defaulted --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.2.0 (Debian 12.2.0-14) 

I am including TileDB in my project using FetchContent in cmake. I set TILEDB_WERROR to OFF but I think that flag comes in from my main project.

I see the following error during the build:

[{
	"resource": "build/dev/_deps/tiledb_super-src/tiledb/sm/array_schema/dimension.cc",
	"owner": "cmake-build-diags",
	"severity": 8,
	"message": "comparing floating-point with ‘==’ or ‘!=’ is unsafe [-Werror=float-equal]",
	"source": "GCC",
	"startLineNumber": 292,
	"startColumn": 21,
	"endLineNumber": 292,
	"endColumn": 1000,
	"relatedInformation": [
		{
			"startLineNumber": 292,
			"startColumn": 21,
			"endLineNumber": 292,
			"endColumn": 1000,
			"message": "In instantiation of ‘static bool tiledb::sm::Dimension::coincides_with_tiles(const tiledb::sm::Dimension*, const tiledb::type::Range&) [with T = float]’:",
			"resource": "build/dev/_deps/tiledb_super-src/tiledb/sm/array_schema/dimension.cc"
		},
		{
			"startLineNumber": 2018,
			"startColumn": 37,
			"endLineNumber": 2018,
			"endColumn": 1000,
			"message": "   required from here",
			"resource": "build/dev/_deps/tiledb_super-src/tiledb/sm/array_schema/dimension.cc"
		}
	]
}]

They seem to be in array_schema

dimention.cc:292
dimension.cc:610
dimension.cc:678
dimension.cc:703
dimension.cc:705
dimension.cc:1621
domain.cc:668
range_subset.h:120
range_subset.h:168

I am not sure if you can cheat with abs(a-b) < epsilon for these cases. Since these are boundries they could get nailed on small numbers.

@KiterLuc
Copy link
Contributor

Thanks for bringing this to our attention @geiseri! There are some of those comparisons that we can change but a few that we need. The right solution will be for us to disable the warning on all compilers, which we will get done in the next couple weeks and let you know. In the meantime, I hope you can disable the warnings on your build.

@geiseri
Copy link
Author

geiseri commented Sep 28, 2023

Cool! Yeah, the float comparison thing seems to be one of those things that doesn't matter until it does. You would know better than me, but cursory look most of them are a zero or not zero check. The ones related to tile range overlaps concern me, just because there can be legitimate reasons to have very close floating point numbers. I would expect you to have better insight as to what the conditions are.

As to disabling warnings, I tend to use #pragma push/pop pattern around specific code. It is a bit cumbersome to make them portable, but it makes sure real warnings are not lost in the noise. =)

@eric-hughes-tiledb
Copy link
Contributor

eric-hughes-tiledb commented Oct 18, 2023

So as not to confuse the practical advice Luc gave, you'll want to compile TileDB at present with -Wno-float-equal overriding whatever you might have set by default.

The larger issue is rather more subtle. A number of the comparisons that are bringing up warnings are comparisons to exact values of 0.0 and 1.0. These do need to be exact in order to function properly. In some cases, for example, we are doing exact checks in order to deal with the difference between a closed interval and an open one. There's no way to make this kind of check correct with an approximate check.

not sure if you can cheat with abs(a-b) < epsilon for these cases
In this case, not at all.

For a large class of numerical algorithms, such as Newton's method, you really don't want to be testing with exact equality because you can end up with infinite loops because of a closed cycle in a small neighborhood of zero that never actually touches zero. This is the kind of code where turning on the compiler warning is useful. Some of our code, however, is not of this kind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants