Support for windows + cuda? #1586
Comments
|
I don't think the GPU side depends on anything C++14, should just be C++11. I didn't go over the errors with a fine toothed comb, but it seems to be some integer size (int/long/size_t/etc) issues, something with Probably only the integer size issues are more of an issue to solve. I know that @beauby did a pass a while back to fix related issues in the Windows CPU side of things. I likely need to do something similar here. @h-vetinari @beauby Is there some easy way for me to grab some Windows VM somewhere and try to reproduce these compilation issues? I will be on vacation off and on over the next two weeks though, so likely can't make progress on it until after that though. |
|
Hey @wickedfoo, thanks for the quick response!
The comment about C++ was not critical for building, just to illustrate the different states of the compilers.
The conda-forge CI is non-interactive, so I don't think you can grab a VM there. If you have access to a windows-box some other way, it would however be possible to very closely (or even perfectly) mimic the conda-forge setup, clone the PR and run
No worries, same here. |
|
The long 32 / 64 bit issue is also there in OSX but since we don't support cuda on OSX by lack of hardware, so is shows up only now in the code. Fixing is just a matter of not using long at all and always use int64/uint64 explicitly. |
Summary: Upstreaming some work towards #1586 from conda-forge/faiss-split-feedstock#19; complements #1610 Pull Request resolved: #1614 Reviewed By: mdouze Differential Revision: D25864994 Pulled By: wickedfoo fbshipit-source-id: 4abaaebf1c4102c24c3a9dca544744318e780581
|
Update: conda-forge/faiss-split-feedstock#19 got merged after the CI went green (based on Since we don't have GPUs in the CI currently I also tested the gpu build locally, and unfortunately found that there are a bunch of errors and segfaults. I immediately removed those packages again: conda-forge/admin-requests#235 I tested as follows: This leads to a segfault of the following kind: ... and incrementally adding skips revealed that everything else segfaults as well (or so I thought...). As it turns out, the MemoryError from skipdiff --git a/faiss/gpu/test/test_gpu_index.py b/faiss/gpu/test/test_gpu_index.py
index d96bd13a..8e16931f 100755
--- a/faiss/gpu/test/test_gpu_index.py
+++ b/faiss/gpu/test/test_gpu_index.py
@@ -199,6 +199,7 @@ class ReferencedObject(unittest.TestCase):
assert ref == new
+ @unittest.skipIf(True, "MemoryError!")
def test_stress(self):
# a mixture of the above, from issue #631
target = np.random.rand(50, 16).astype('float32')Log of test runFailure:
|
|
OK, I debugged one of the problems ( diffdiff --git a/faiss/gpu/test/test_gpu_index.py b/faiss/gpu/test/test_gpu_index.py
index d96bd13a..d5af7a46 100755
--- a/faiss/gpu/test/test_gpu_index.py
+++ b/faiss/gpu/test/test_gpu_index.py
@@ -61,8 +61,11 @@ class EvalIVFPQAccuracy(unittest.TestCase):
index.train(xt)
ts.append(time.time())
- # adding some ids because there was a bug in this case
- index.add_with_ids(xb, np.arange(nb) * 3 + 12345)
+ # adding some ids because there was a bug in this case;
+ # those need to be cast to idx_t(= int64_t), because
+ # on windows the numpy int default is int32
+ ids = (np.arange(nb) * 3 + 12345).astype('int64')
+ index.add_with_ids(xb, ids)
ts.append(time.time())
index.nprobe = 4This lead to different failures logAnother failure is that I'm probably gonna need a bit of help on this one... @beauby @wickedfoo |
|
@h-vetinari Thanks for this thorough investigation! My first guess would be line 303 of swigfaiss.swig. Those |
|
It would not hurt to add the same three template declarations with unsigned long long for windows. |
|
Thanks for the inputs. Before adding more declarations, I wanted to try the followingdiff --git a/faiss/python/swigfaiss.swig b/faiss/python/swigfaiss.swig
index 3113a136..d455ec1a 100644
--- a/faiss/python/swigfaiss.swig
+++ b/faiss/python/swigfaiss.swig
@@ -204,9 +204,7 @@ namespace std {
%template(DoubleVector) std::vector<double>;
%template(ByteVector) std::vector<uint8_t>;
%template(CharVector) std::vector<char>;
-// NOTE(hoss): Using unsigned long instead of uint64_t because OSX defines
-// uint64_t as unsigned long long, which SWIG is not aware of.
-%template(Uint64Vector) std::vector<unsigned long>;
+%template(Uint64Vector) std::vector<uint64_t>;
%template(LongVector) std::vector<long>;
%template(LongLongVector) std::vector<long long>;
%template(IntVector) std::vector<int>;
@@ -300,9 +298,9 @@ void gpu_sync_all_devices()
%}
-%template() std::pair<int, unsigned long>;
-%template() std::map<std::string, std::pair<int, unsigned long> >;
-%template() std::map<int, std::map<std::string, std::pair<int, unsigned long> > >;
+%template() std::pair<int, uint64_t>;
+%template() std::map<std::string, std::pair<int, uint64_t> >;
+%template() std::map<int, std::map<std::string, std::pair<int, uint64_t> > >;
// causes weird wrapper bug
%ignore *::allocMemoryHandle;which is more in line with what has been done for the long-on-win issue so far, cf. e.g. #1614.
Morally speaking, I'd prefer the patch above, but would also try with |
Summary: Part of the work towards #1586; I already mentioned this diff as necessary for a passing GPU test suite on windows [here](#1586 (comment)) Pull Request resolved: #1739 Reviewed By: wickedfoo Differential Revision: D26855633 Pulled By: mdouze fbshipit-source-id: 96d3e627034ccbbe3a32cc4f2310a721e8ea0a69
|
Woohoo, a passing GPU test suite on windows! 🥳
You can check out conda-forge/faiss-split-feedstock#32 (where there's also artefacts available for download; here and alternatively here), which is based on |
|
I'll close then... |
|
After #1742 was merged and incorporated into the feedstock (and I solved some CI timeouts), conda-forge now has win+ cuda builds 🥳 Can be installed like |
Summary
conda-forge recently added windows support for faiss, and - based on the infrastructure there - that makes it an automatic candidate for also building for windows + cuda, which is currently being worked on in conda-forge/faiss-split-feedstock#19.
After fixing some problems with cmake etc., we're now at the point where things start compiling, but this is running into a whole host of compilation errors. It seems that MSVC + nvcc aren't nearly as far as on linux (e.g. no C++14 support, whereas on linux C++11 is deprecated), and so the question is if there is interest in reaching compatibility with windows + cuda for faiss, and how these ~250 errors should be tackled, resp. such work should be structured.
PS. We can use that PR for debugging, but obviously, I'd much prefer to not carry patches if things can be done directly upstream.
Platform
OS: Windows
Faiss version: 1.6.5
Building in conda-forge/faiss-split-feedstock#19
Faiss compilation options:
Running on:
Interface:
Reproduction instructions
The text was updated successfully, but these errors were encountered: