neperfepx / neper

Polycrystal generation and meshing
http://neper.info
GNU General Public License v3.0
198 stars 53 forks source link

Build failures on 32 bit systems #832

Closed kimauth closed 3 months ago

kimauth commented 3 months ago

Describe the bug

We're having problems building the latest version of Neper (4.8.1) on all 32-bit systems over at https://github.com/JuliaPackaging/Yggdrasil/pull/8351. In the PR there are build logs available for different platforms, see e.g. https://buildkite.com/julialang/yggdrasil/builds/9154#018e7621-2483-4812-8011-1145bee7ed15.

I don't have much experience with C++ myself, but other people have looked at it and suggested it is an integer width issue:

[15:02:46] /workspace/srcdir/neper/src/neut/neut_oset/neut_oset1.cpp: In function ‘void neut_oset_clustering(OL_SET, OL_SET, char*, OL_SET*)’:
[15:02:46] /workspace/srcdir/neper/src/neut/neut_oset/neut_oset1.cpp:38:85: error: invalid conversion from ‘__gnu_cxx::__alloc_traits<std::allocator<long unsigned int> >::value_type* {aka long unsigned int*}’ to ‘unsigned int*’ [-fpermissive]
[15:02:46]        nano_index->knnSearch (OSet.q[i], num_results, &ret_index[0], &out_dist_sqr[0]);
[15:02:46]                                                                                      ^
[15:02:46] In file included from /workspace/srcdir/neper/src/neut/neut_structs/neut_qcloud_struct.hpp:6:0,
[15:02:46]                  from /workspace/srcdir/neper/src/neut/neut_oset/neut_oset.hpp:5,
[15:02:46]                  from /workspace/srcdir/neper/src/neut/neut_oset/neut_oset_.hpp:8,
[15:02:46]                  from /workspace/srcdir/neper/src/neut/neut_oset/neut_oset1.cpp:5:
[15:02:46] /workspace/srcdir/neper/src/contrib/nanoflann/nanoflann.hpp:1223:10: note:   initializing argument 3 of ‘size_t nanoflann::KDTreeSingleIndexAdaptor<Distance, DatasetAdaptor, DIM, IndexType>::knnSearch(const ElementType*, size_t, IndexType*, nanoflann::KDTreeSingleIndexAdaptor<Distance, DatasetAdaptor, DIM, IndexType>::DistanceType*, int) const [with Distance = nanoflann::L2_Simple_Adaptor<double, QCLOUD>; DatasetAdaptor = QCLOUD; int DIM = 4; IndexType = unsigned int; size_t = unsigned int; nanoflann::KDTreeSingleIndexAdaptor<Distance, DatasetAdaptor, DIM, IndexType>::ElementType = double; nanoflann::KDTreeSingleIndexAdaptor<Distance, DatasetAdaptor, DIM, IndexType>::DistanceType = double]’
[15:02:46]    size_t knnSearch(const ElementType *query_point, const size_t num_closest, IndexType *out_indices, DistanceType *out_distances_sq, const int /* nChecks_IGNORED */ = 10) const

Looks like an integer width issue - they are mixing long and int types, which match on 64-bit platforms but differ on 32-bit platforms.

Originally posted by @imciner2 in https://github.com/JuliaPackaging/Yggdrasil/issues/8351#issuecomment-2020052437

Possible fix

I think changing the long unsigned int into size_t will fix it here:

https://github.com/neperfepx/neper/blob/ed518c1329c057f1c4d3b2971f56c17066a8e5b6/src/neut/neut_oset/neut_oset1.cpp#L33C17-L33C34

This is because the nano_index used on the line that errors has a default index type of size_t, whic is probably not the same as long unsigned int on 32-bit platforms.

Originally posted by @barche in https://github.com/JuliaPackaging/Yggdrasil/issues/8351#issuecomment-2021043155

rquey commented 3 months ago

Thanks @kimauth for investigating, and for the fix. I've made the change in version 4.8.2 (just released).

kimauth commented 3 months ago

Thank you for the immediate fix! Now the builds are all successful again. 🎉