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.

Sign up for GitHub

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

Jump to bottom

Correct isnan invocation #964

Closed
wants to merge 1 commit into from
Closed

Correct isnan invocation #964

wants to merge 1 commit into from

Conversation

sanjayankur31
Copy link

No description provided.

@Shuangistan
Copy link

Had the same problem 5 mins. ago ;)

@halfflat
Copy link
Contributor

Hi! Thanks for the PRs! We have a fix in the queue: PR #925 hasn't been merged yet only because we currently don't have access to our ARM test machine.

Would you be able to try applying that PR, and confirm that it builds and that the unit tests pass (with vectorization enabled, naturally)? If it does pass, I'll merge that PR.

@sanjayankur31
Copy link
Author

Hrm, this PR isn't ARM related. It merely ensures that isnan is completely defined using the std:: namespace. It was causing the build to fail on x86_64 also.

We already tested the build with #925 and reported the results in #920. Works without MPI, but not with MPI enabled.

@halfflat
Copy link
Contributor

Hrm, this PR isn't ARM related. It merely ensures that isnan is completely defined using the std:: namespace. It was causing the build to fail on x86_64 also.

It's odd that it was causing x86_64 to fail: apart from the fact that that's our default build environment, the neon.hpp header is only included if __ARM_NEON__ or __aarch64__ are defined (see arbor/include/arbor/simd/native.hpp lines 68-74).

The reason why I'd like to get #925 in in preference, is that it avoids using std::isnan altogether.

We already tested the build with #925 and reported the results in #920. Works without MPI, but not with MPI enabled.

Ah, thanks for that! I am sorry that I missed the update. The error quoted there on the mpich failure has since been resolved.

@bcumming
Copy link
Member

Can you test the latest version of master, and tell us if the problem still persists? It should have been addressed.

@sanjayankur31
Copy link
Author

Can you test the latest version of master, and tell us if the problem still persists? It should have been addressed.

Here's the test build with a35b819

(it'll only stay around for a day or two, since it's a scratch build):
https://koji.fedoraproject.org/koji/taskinfo?taskID=41887869

armv7hl still fails:

/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/communication/mpi.cpp
cc1plus: warning: switch '-mcpu=arm10e' conflicts with '-march=armv7-a' switch
In file included from /builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/communication/mpi.cpp:3:
/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/communication/mpi.hpp:61:63: error: static assertion failed: size_t is not the same as unsigned long or unsigned long long
   61 | static_assert(std::is_same<std::size_t, unsigned long>::value ||
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
   62 |               std::is_same<std::size_t, unsigned long long>::value,
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [arbor/CMakeFiles/arbor.dir/build.make:798: arbor/CMakeFiles/arbor.dir/communication/mpi.cpp.o] Error 1
make[2]: Leaving directory '/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/build-mpich'
make[2]: *** Waiting for unfinished jobs....
make[2]: Entering directory '/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/build-mpich'
[ 67%] Building CXX object arbor/CMakeFiles/arbor.dir/communication/mpi_context.cpp.o
cd /builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/build-mpich/arbor && /usr/lib/mpich/bin/mpicxx  -DARB_HAVE_MPI -DMPICH_SKIP_MPICXX=1 -DOMPI_SKIP_MPICXX=1 -I/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor -I/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/include -I/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/build-mpich/arbor/include  -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -march=armv7-a -mfpu=vfpv3-d16 -mtune=generic-armv7-a -mabi=aapcs-linux -mfloat-abi=hard -DNDEBUG -fPIC   -g -Wall -Wno-maybe-uninitialized -mcpu=native -std=gnu++14 -o CMakeFiles/arbor.dir/communication/mpi_context.cpp.o -c /builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/communication/mpi_context.cpp
make[2]: Leaving directory '/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/build-mpich'
cc1plus: warning: switch '-mcpu=arm10e' conflicts with '-march=armv7-a' switch
In file included from /builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/communication/mpi_context.cpp:15:
/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/communication/mpi.hpp:61:63: error: static assertion failed: size_t is not the same as unsigned long or unsigned long long
   61 | static_assert(std::is_same<std::size_t, unsigned long>::value ||
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
   62 |               std::is_same<std::size_t, unsigned long long>::value,
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [arbor/CMakeFiles/arbor.dir/build.make:824: arbor/CMakeFiles/arbor.dir/communication/mpi_context.cpp.o] Error 1
make[2]: Entering directory '/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/build-mpich'

i686 also still fails with the same error:

CMakeFiles/arbor.dir/communication/mpi.cpp.o -c /builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/communication/mpi.cpp
make[2]: Leaving directory '/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/build-mpich'
In file included from /builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/communication/mpi.cpp:3:
/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/communication/mpi.hpp:61:63: error: static assertion failed: size_t is not the same as unsigned long or unsigned long long
   61 | static_assert(std::is_same<std::size_t, unsigned long>::value ||
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
   62 |               std::is_same<std::size_t, unsigned long long>::value,
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [arbor/CMakeFiles/arbor.dir/build.make:798: arbor/CMakeFiles/arbor.dir/communication/mpi.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: Entering directory '/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/build-mpich'
[ 67%] Building CXX object arbor/CMakeFiles/arbor.dir/communication/mpi_context.cpp.o
cd /builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/build-mpich/arbor && /usr/lib/mpich/bin/mpicxx  -DARB_HAVE_MPI -DMPICH_SKIP_MPICXX=1 -DOMPI_SKIP_MPICXX=1 -I/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor -I/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/include -I/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/build-mpich/arbor/include  -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m32 -march=i686 -mtune=generic -msse2 -mfpmath=sse -mstackrealign -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -DNDEBUG -fPIC   -g -Wall -Wno-maybe-uninitialized -march=native -std=gnu++14 -o CMakeFiles/arbor.dir/communication/mpi_context.cpp.o -c /builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/communication/mpi_context.cpp
make[2]: Leaving directory '/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/build-mpich'
In file included from /builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/communication/mpi_context.cpp:15:
/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/arbor/communication/mpi.hpp:61:63: error: static assertion failed: size_t is not the same as unsigned long or unsigned long long
   61 | static_assert(std::is_same<std::size_t, unsigned long>::value ||
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
   62 |               std::is_same<std::size_t, unsigned long long>::value,
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [arbor/CMakeFiles/arbor.dir/build.make:824: arbor/CMakeFiles/arbor.dir/communication/mpi_context.cpp.o] Error 1
make[2]: Entering directory '/builddir/build/BUILD/arbor-a35b8198328b1e30fa32bd32c950607e3e9c731f/build-mpich'

Both logs:
arbor-a35b819-armv7hl-build-fail.txt
arbor-a35b819-i686-build-fail.txt

Ah, thanks for that! I am sorry that I missed the update. The error quoted there on the mpich failure has since been resolved.

No, doesn't look like it (unless I'm doing something wrong here) :(

@sanjayankur31
Copy link
Author

Oh, so, the isnan bit has been fixed. Closing this. Continuing the conversation in #920

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@sanjayankur31 @Shuangistan @halfflat @bcumming