Skip to content

Commit 264d6e5

Browse files
mdouzefacebook-github-bot
mdouze
authored and
facebook-github-bot
committed
Add check for struct sizes
Summary: C++ Faiss is compiled with (at least) 3 compilers: - the one compiling the main CPU Faiss - the one called by nvcc to compile host code on GPU Faiss - the one compiling the SWIG generated file Sometimes the compilers are misconfigured and have different ideas of data alignment, size and packing. This is a hard to catch bug. This test attemtps to check if one of these differences occur. The test did catch the bug in one configuration in the Github version, see #4136 https://github.com/facebookresearch/faiss/actions/runs/13177278514/job/36779465009?pr=4136 Differential Revision: D69243348
1 parent d720155 commit 264d6e5

File tree

9 files changed

+218
-0
lines changed

9 files changed

+218
-0
lines changed

‎faiss/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ set(FAISS_HEADERS
214214
utils/simdlib_emulated.h
215215
utils/simdlib_neon.h
216216
utils/simdlib_ppc64.h
217+
utils/struct_packing_test.h
217218
utils/utils.h
218219
utils/distances_fused/avx512.h
219220
utils/distances_fused/distances_fused.h

‎faiss/gpu/GpuIndex.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,5 +194,7 @@ bool isGpuIndex(faiss::Index* index);
194194
/// Does the given CPU index instance have a corresponding GPU implementation?
195195
bool isGpuIndexImplemented(faiss::Index* index);
196196

197+
int struct_packing_test_cuda(int q);
198+
197199
} // namespace gpu
198200
} // namespace faiss

‎faiss/gpu/impl/IndexUtils.cu‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <faiss/gpu/impl/IndexUtils.h>
99
#include <faiss/impl/FaissAssert.h>
10+
#include <faiss/utils/struct_packing_test.h>
1011
#include <faiss/gpu/utils/DeviceDefs.cuh>
1112
#include <limits>
1213

@@ -38,5 +39,7 @@ void validateNProbe(size_t nprobe) {
3839
nprobe);
3940
}
4041

42+
int struct_packing_test_cuda(int q) STRUCT_PACKING_FUNCTION_BODY
43+
4144
} // namespace gpu
4245
} // namespace faiss

‎faiss/gpu/test/test_gpu_basics.py‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,3 +469,26 @@ class TestGpuFlags(unittest.TestCase):
469469

470470
def test_gpu_flag(self):
471471
assert "GPU" in faiss.get_compile_options().split()
472+
473+
474+
class TestStructPacking(unittest.TestCase):
475+
""" Verify if the size structures as seen from Cuda and C++ are the same """
476+
477+
def test_swig(self):
478+
" This test is redundant with the CPU tests, but let's check run it just in case"
479+
sizes = np.array([
480+
(faiss.struct_packing_test_cpp(q),
481+
faiss.struct_packing_test_swig(q))
482+
for q in range(20)
483+
])
484+
print(sizes)
485+
np.testing.assert_array_equal(sizes[:, 0], sizes[:, 1])
486+
487+
def test_cuda(self):
488+
sizes = np.array([
489+
(faiss.struct_packing_test_cpp(q),
490+
faiss.struct_packing_test_cuda(q))
491+
for q in range(20)
492+
])
493+
print(sizes)
494+
np.testing.assert_array_equal(sizes[:, 0], sizes[:, 1])

‎faiss/python/swigfaiss.swig‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,7 @@ struct faiss::simd16uint16 {};
679679

680680
%include <faiss/gpu/GpuIndicesOptions.h>
681681
%include <faiss/gpu/GpuClonerOptions.h>
682+
682683
%include <faiss/gpu/GpuIndex.h>
683684
#ifdef FAISS_ENABLE_CUVS
684685
%include <faiss/gpu/GpuIndexCagra.h>
@@ -1245,6 +1246,20 @@ REV_SWIG_PTR(uint64_t, NPY_UINT64);
12451246

12461247
#endif
12471248

1249+
namespace faiss {
1250+
int struct_packing_test_swig (int q);
1251+
}
1252+
1253+
%{
1254+
1255+
#include <faiss/utils/struct_packing_test.h>
1256+
1257+
namespace faiss {
1258+
int struct_packing_test_swig (int q) STRUCT_PACKING_FUNCTION_BODY
1259+
}
1260+
1261+
%}
1262+
12481263

12491264

12501265
/*******************************************************************

‎faiss/utils/struct_packing_test.h‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
/* This file and macro is there to catch discrepancies on the different
9+
* compilers used in Faiss align and pack structures. This is a difficult to
10+
* catch bug. It works by declaring a structure and measuring its size in
11+
* diffetent compilers from a function. The function can be called from a test.
12+
* If the size as seen from different compilers changes, it's a guarantee for
13+
* fireworks at runtime. */
14+
15+
#pragma once
16+
#include <vector>
17+
18+
namespace faiss {
19+
namespace struct_packing_test {
20+
21+
/*******************************************************
22+
* Fake structure to detect structure packing/alignment
23+
* issues.
24+
*******************************************************/
25+
26+
struct StructPackingTestD {
27+
int a;
28+
bool b, c, d;
29+
long e;
30+
bool f, g, h, i, j;
31+
};
32+
33+
struct StructPackingTestC : StructPackingTestD {
34+
bool k;
35+
int l;
36+
bool m;
37+
};
38+
39+
struct StructPackingTestA {
40+
virtual void* operator()(int) {
41+
return nullptr;
42+
}
43+
44+
virtual ~StructPackingTestA() {}
45+
};
46+
47+
struct StructPackingTestB : StructPackingTestA {
48+
StructPackingTestC options;
49+
std::vector<void*> vres;
50+
std::vector<int> devices;
51+
int ncall;
52+
53+
void* operator()(int) override {
54+
return nullptr;
55+
}
56+
virtual ~StructPackingTestB() {}
57+
};
58+
59+
// This is the object hierachy of GpuProgressiveDimIndexFactory that initially
60+
// triggered this error (see PR #4135 and #4136)
61+
62+
enum IndicesOptionsB {
63+
INDICES_CPU_B = 0,
64+
INDICES_IVF_B = 1,
65+
INDICES_32_BIT_B = 2,
66+
INDICES_64_BIT_B = 3,
67+
};
68+
69+
struct GpuClonerOptionsB {
70+
IndicesOptionsB indicesOptions = INDICES_64_BIT_B;
71+
72+
bool useFloat16CoarseQuantizer = false;
73+
74+
bool useFloat16 = false;
75+
76+
bool usePrecomputed = false;
77+
78+
long reserveVecs = 0;
79+
80+
bool storeTransposed = false;
81+
82+
bool verbose = false;
83+
84+
bool use_cuvs = false;
85+
bool allowCpuCoarseQuantizer = false;
86+
};
87+
88+
struct GpuMultipleClonerOptionsB : public GpuClonerOptionsB {
89+
bool shard = false;
90+
int shard_type = 1;
91+
bool common_ivf_quantizer = false;
92+
};
93+
94+
struct ProgressiveDimIndexFactoryB {
95+
96+
virtual void* operator()(int dim) {
97+
return nullptr;
98+
}
99+
100+
virtual ~ProgressiveDimIndexFactoryB() {}
101+
};
102+
103+
struct GpuProgressiveDimIndexFactoryB : ProgressiveDimIndexFactoryB {
104+
GpuMultipleClonerOptionsB options;
105+
std::vector<void*> vres;
106+
std::vector<int> devices;
107+
int ncall;
108+
109+
explicit GpuProgressiveDimIndexFactoryB(int ngpu) {}
110+
111+
void* operator()(int dim) override {
112+
return nullptr;
113+
}
114+
115+
virtual ~GpuProgressiveDimIndexFactoryB() override {}
116+
};
117+
118+
} // namespace struct_packing_test
119+
120+
} // namespace faiss
121+
122+
// body of function should be
123+
// int function_name (int q) STRUCT_PACKING_FUNCTION_BODY
124+
125+
#define STRUCT_PACKING_FUNCTION_BODY \
126+
{ \
127+
struct_packing_test::StructPackingTestB sb; \
128+
switch (q) { \
129+
case 0: \
130+
return sizeof(struct_packing_test::StructPackingTestB); \
131+
case 1: \
132+
return (char*)&sb.ncall - (char*)&sb; \
133+
case 2: \
134+
return sizeof(struct_packing_test::StructPackingTestD); \
135+
case 3: \
136+
return sizeof(struct_packing_test::StructPackingTestC); \
137+
case 4: \
138+
return sizeof(struct_packing_test::StructPackingTestB); \
139+
case 5: \
140+
return sizeof(struct_packing_test::StructPackingTestA); \
141+
case 6: \
142+
return sizeof(struct_packing_test::IndicesOptionsB); \
143+
case 7: \
144+
return sizeof(struct_packing_test::GpuMultipleClonerOptionsB); \
145+
case 8: \
146+
return sizeof(struct_packing_test::GpuClonerOptionsB); \
147+
case 9: \
148+
return sizeof( \
149+
struct_packing_test::ProgressiveDimIndexFactoryB); \
150+
case 10: \
151+
return sizeof( \
152+
struct_packing_test::GpuProgressiveDimIndexFactoryB); \
153+
default: \
154+
return -1; \
155+
} \
156+
}

‎faiss/utils/utils.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// -*- c++ -*-
99

1010
#include <faiss/Index.h>
11+
#include <faiss/utils/struct_packing_test.h>
1112
#include <faiss/utils/utils.h>
1213

1314
#include <cassert>
@@ -641,4 +642,6 @@ void CodeSet::insert(size_t n, const uint8_t* codes, bool* inserted) {
641642
}
642643
}
643644

645+
int struct_packing_test_cpp(int q) STRUCT_PACKING_FUNCTION_BODY;
646+
644647
} // namespace faiss

‎faiss/utils/utils.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,8 @@ struct CodeSet {
204204
void insert(size_t n, const uint8_t* codes, bool* inserted);
205205
};
206206

207+
int struct_packing_test_cpp(int q);
208+
207209
} // namespace faiss
208210

209211
#endif /* FAISS_utils_h */

‎tests/test_build_blocks.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,3 +562,16 @@ def test_small(self):
562562
def xx_test_large(self):
563563
# don't run by default because it's slow
564564
self.do_test(2 ** 21, 10 ** 6)
565+
566+
567+
class TestStructPacking(unittest.TestCase):
568+
""" Verify if the size structures as seen from SWIG and C++ are the same """
569+
570+
def test_swig(self):
571+
sizes = np.array([
572+
(faiss.struct_packing_test_cpp(q),
573+
faiss.struct_packing_test_swig(q))
574+
for q in range(20)
575+
])
576+
print(sizes)
577+
np.testing.assert_array_equal(sizes[:, 0], sizes[:, 1])

0 commit comments

Comments
 (0)