Implement neighbor sampler #76

kgajdamo · 2022-08-23T13:40:24Z

The purpose of this PR is to implement neighbor sampler in pyg-lib.

What has been done:

Registered neighbor_sampler as a torch custom operator.
Moved neighbor_sampler implementation from pytorch_sparse to pyg-lib.
pyg-lib neighbor_sampler supports CSR matrix format.
Added simple test (test passed).

In progress:

Replace to_local_node map with Mapper.
Add support for scalar types.
Add support for isolated and return_edge_id arguments.

for more information, see https://pre-commit.ci

rusty1s

Thanks for getting us started! This looks pretty good. Would prefer moving basic tests to C++, and using rand_engine for any random number generator logic.

pyg_lib/sampler/__init__.py

rusty1s · 2022-08-24T06:12:49Z

test/csrc/sampler/test_neighbor_sample.py

@@ -0,0 +1,49 @@
+import torch


It would be great if we can just test in C++. WDYT?

Sure, but unfortunately, when I want to compile pyg-lib with unit tests I am receiving undefined reference error. I wonder if this is only happening on my side?

Can you show me the log?

/home/kgajdamo/miniconda3/envs/pyg/bin/../lib/gcc/x86_64-conda-linux-gnu/9.3.0/../../../../x86_64-conda-linux-gnu/bin/ld: CMakeFiles/test_biased_random.dir/test/csrc/random/test_biased_random.cpp.o: in function `BiasedSamplingCDFConversionTest_BasicAssertions_Test::TestBody()': test_biased_random.cpp:(.text._ZN52BiasedSamplingCDFConversionTest_BasicAssertions_Test8TestBodyEv+0x344): undefined reference to `testing::internal::GetBoolAssertionFailureMessage[abi:cxx11](testing::AssertionResult const&, char const*, char const*, char const*)' /home/kgajdamo/miniconda3/envs/pyg/bin/../lib/gcc/x86_64-conda-linux-gnu/9.3.0/../../../../x86_64-conda-linux-gnu/bin/ld: test_biased_random.cpp:(.text._ZN52BiasedSamplingCDFConversionTest_BasicAssertions_Test8TestBodyEv+0x479): undefined reference to `testing::internal::GetBoolAssertionFailureMessage[abi:cxx11](testing::AssertionResult const&, char const*, char const*, char const*)' /home/kgajdamo/miniconda3/envs/pyg/bin/../lib/gcc/x86_64-conda-linux-gnu/9.3.0/../../../../x86_64-conda-linux-gnu/bin/ld: CMakeFiles/test_biased_random.dir/test/csrc/random/test_biased_random.cpp.o: in function `BiasedSamplingAliasConversionTest_BasicAssertions_Test::TestBody()': test_biased_random.cpp:(.text._ZN54BiasedSamplingAliasConversionTest_BasicAssertions_Test8TestBodyEv+0x90b): undefined reference to `testing::internal::GetBoolAssertionFailureMessage[abi:cxx11](testing::AssertionResult const&, char const*, char const*, char const*)' /home/kgajdamo/miniconda3/envs/pyg/bin/../lib/gcc/x86_64-conda-linux-gnu/9.3.0/../../../../x86_64-conda-linux-gnu/bin/ld: test_biased_random.cpp:(.text._ZN54BiasedSamplingAliasConversionTest_BasicAssertions_Test8TestBodyEv+0x9e3): undefined reference to `testing::internal::GetBoolAssertionFailureMessage[abi:cxx11](testing::AssertionResult const&, char const*, char const*, char const*)' /home/kgajdamo/miniconda3/envs/pyg/bin/../lib/gcc/x86_64-conda-linux-gnu/9.3.0/../../../../x86_64-conda-linux-gnu/bin/ld: test_biased_random.cpp:(.text._ZN54BiasedSamplingAliasConversionTest_BasicAssertions_Test8TestBodyEv+0xabd): undefined reference to `testing::internal::GetBoolAssertionFailureMessage[abi:cxx11](testing::AssertionResult const&, char const*, char const*, char const*)' /home/kgajdamo/miniconda3/envs/pyg/bin/../lib/gcc/x86_64-conda-linux-gnu/9.3.0/../../../../x86_64-conda-linux-gnu/bin/ld: CMakeFiles/test_biased_random.dir/test/csrc/random/test_biased_random.cpp.o:test_biased_random.cpp:(.text._ZN54BiasedSamplingAliasConversionTest_BasicAssertions_Test8TestBodyEv+0xb9d): more undefined references to `testing::internal::GetBoolAssertionFailureMessage[abi:cxx11](testing::AssertionResult const&, char const*, char const*, char const*)' follow /home/kgajdamo/miniconda3/envs/pyg/bin/../lib/gcc/x86_64-conda-linux-gnu/9.3.0/../../../../x86_64-conda-linux-gnu/bin/ld: CMakeFiles/test_biased_random.dir/test/csrc/random/test_biased_random.cpp.o: in function `testing::AssertionResult testing::internal::CmpHelperOpFailure<int, double>(char const*, char const*, int const&, double const&, char const*)': test_biased_random.cpp:(.text._ZN7testing8internal18CmpHelperOpFailureIidEENS_15AssertionResultEPKcS4_RKT_RKT0_S4_[_ZN7testing8internal18CmpHelperOpFailureIidEENS_15AssertionResultEPKcS4_RKT_RKT0_S4_]+0xa4): undefined reference to `testing::Message::GetString[abi:cxx11]() const' /home/kgajdamo/miniconda3/envs/pyg/bin/../lib/gcc/x86_64-conda-linux-gnu/9.3.0/../../../../x86_64-conda-linux-gnu/bin/ld: test_biased_random.cpp:(.text._ZN7testing8internal18CmpHelperOpFailureIidEENS_15AssertionResultEPKcS4_RKT_RKT0_S4_[_ZN7testing8internal18CmpHelperOpFailureIidEENS_15AssertionResultEPKcS4_RKT_RKT0_S4_]+0x15d): undefined reference to `testing::Message::GetString[abi:cxx11]() const' /home/kgajdamo/miniconda3/envs/pyg/bin/../lib/gcc/x86_64-conda-linux-gnu/9.3.0/../../../../x86_64-conda-linux-gnu/bin/ld: test_biased_random.cpp:(.text._ZN7testing8internal18CmpHelperOpFailureIidEENS_15AssertionResultEPKcS4_RKT_RKT0_S4_[_ZN7testing8internal18CmpHelperOpFailureIidEENS_15AssertionResultEPKcS4_RKT_RKT0_S4_]+0x1ff): undefined reference to `testing::Message::GetString[abi:cxx11]() const' /home/kgajdamo/miniconda3/envs/pyg/bin/../lib/gcc/x86_64-conda-linux-gnu/9.3.0/../../../../x86_64-conda-linux-gnu/bin/ld: test_biased_random.cpp:(.text._ZN7testing8internal18CmpHelperOpFailureIidEENS_15AssertionResultEPKcS4_RKT_RKT0_S4_[_ZN7testing8internal18CmpHelperOpFailureIidEENS_15AssertionResultEPKcS4_RKT_RKT0_S4_]+0x2b0): undefined reference to `testing::Message::GetString[abi:cxx11]() const' /home/kgajdamo/miniconda3/envs/pyg/bin/../lib/gcc/x86_64-conda-linux-gnu/9.3.0/../../../../x86_64-conda-linux-gnu/bin/ld: test_biased_random.cpp:(.text._ZN7testing8internal18CmpHelperOpFailureIidEENS_15AssertionResultEPKcS4_RKT_RKT0_S4_[_ZN7testing8internal18CmpHelperOpFailureIidEENS_15AssertionResultEPKcS4_RKT_RKT0_S4_]+0x352): undefined reference to `testing::Message::GetString[abi:cxx11]() const' /home/kgajdamo/miniconda3/envs/pyg/bin/../lib/gcc/x86_64-conda-linux-gnu/9.3.0/../../../../x86_64-conda-linux-gnu/bin/ld: CMakeFiles/test_biased_random.dir/test/csrc/random/test_biased_random.cpp.o:test_biased_random.cpp:(.text._ZN7testing8internal18CmpHelperOpFailureIidEENS_15AssertionResultEPKcS4_RKT_RKT0_S4_[_ZN7testing8internal18CmpHelperOpFailureIidEENS_15AssertionResultEPKcS4_RKT_RKT0_S4_]+0x40b): more undefined references to `testing::Message::GetString[abi:cxx11]() const' follow collect2: error: ld returned 1 exit status make[2]: *** [CMakeFiles/test_biased_random.dir/build.make:102: test_biased_random] Error 1 make[1]: *** [CMakeFiles/Makefile2:204: CMakeFiles/test_biased_random.dir/all] Error 2 make: *** [Makefile:146: all] Error 2

@ZenoTan Did you see this before? If not, let's ignore C++ tests for now :)

Didn't meet this but it looks like the linkage to gtest failed. It's fine to ignore though.

Do you also get this error during compilation or is it just me?

I did not get exactly the same, but similar issues when I tried to build other libraries.

@kgajdamo @rusty1s I noticed this error occurs when we have a mix of CF/anaconda c++ packages in our toolchain. Compiling cpp tests in simple venv works well.

pyg_lib/csrc/sampler/cpu/neighbor_kernel.cpp

pyg_lib/csrc/sampler/cpu/utils.h

codecov-commenter · 2022-08-25T09:23:55Z

Codecov Report

Merging #76 (38a3460) into master (e73cd09) will decrease coverage by 8.22%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master      #76      +/-   ##
==========================================
- Coverage   86.82%   78.59%   -8.23%     
==========================================
  Files          14       14              
  Lines         258      285      +27     
==========================================
  Hits          224      224              
- Misses         34       61      +27

Impacted Files	Coverage Δ
pyg_lib/csrc/sampler/cpu/mapper.h	`64.51% <0.00%> (-15.49%)`	⬇️
pyg_lib/csrc/sampler/cpu/neighbor_kernel.cpp	`15.00% <0.00%> (-45.00%)`	⬇️
pyg_lib/csrc/sampler/neighbor.cpp	`21.42% <0.00%> (-16.08%)`	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

rusty1s · 2022-08-26T18:34:30Z

I think this is a great start. Thank you so much for putting this together. I also added scalar_t support.

kgajdamo and others added 2 commits August 23, 2022 14:50

add neighbor sampler to pyg-lib

076f8ba

[pre-commit.ci] auto fixes from pre-commit.com hooks

8706561

for more information, see https://pre-commit.ci

rusty1s reviewed Aug 24, 2022

View reviewed changes

kgajdamo added 2 commits August 25, 2022 11:33

change isolated to disjoint, ad descriptions and asserts

c387d54

modify test

bb092a9

kgajdamo force-pushed the add-neighbor-sampler branch from 6168329 to d628574 Compare August 25, 2022 09:56

kgajdamo added 2 commits August 25, 2022 12:06

delete utils file, add mapper to neighbor kernel, replace rand function

d628574

replace list type with typing.List

38a3460

kgajdamo force-pushed the add-neighbor-sampler branch from 61fd42a to 561a668 Compare August 25, 2022 10:36

kgajdamo changed the title ~~[WIP] Implement neighbor sampler in pyg-lib~~ Implement neighbor sampler in pyg-lib Aug 25, 2022

kgajdamo changed the title ~~Implement neighbor sampler in pyg-lib~~ Implement neighbor sampler Aug 25, 2022

kgajdamo and others added 2 commits August 25, 2022 12:46

update CHANGELOG.md

561a668

update

c91010b

rusty1s approved these changes Aug 26, 2022

View reviewed changes

rusty1s enabled auto-merge (squash) August 26, 2022 18:34

rusty1s assigned kgajdamo Aug 26, 2022

rusty1s added 0 - Priority P0 feature sampler labels Aug 26, 2022

rusty1s disabled auto-merge August 26, 2022 18:39

update

6c8caeb

rusty1s enabled auto-merge (squash) August 26, 2022 18:41

rusty1s merged commit 84e4b23 into pyg-team:master Aug 26, 2022

kgajdamo deleted the add-neighbor-sampler branch August 30, 2022 06:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement neighbor sampler #76

Implement neighbor sampler #76

kgajdamo commented Aug 23, 2022 •

edited by rusty1s

Loading

rusty1s left a comment

rusty1s Aug 24, 2022

kgajdamo Aug 24, 2022

rusty1s Aug 24, 2022

kgajdamo Aug 24, 2022 •

edited by rusty1s

Loading

rusty1s Aug 24, 2022 •

edited

Loading

ZenoTan Aug 25, 2022

kgajdamo Aug 25, 2022

ZenoTan Aug 25, 2022 •

edited

Loading

DamianSzwichtenberg Aug 26, 2022 •

edited

Loading

codecov-commenter commented Aug 25, 2022 •

edited

Loading

rusty1s commented Aug 26, 2022

Implement neighbor sampler #76

Implement neighbor sampler #76

Conversation

kgajdamo commented Aug 23, 2022 • edited by rusty1s Loading

rusty1s left a comment

Choose a reason for hiding this comment

rusty1s Aug 24, 2022

Choose a reason for hiding this comment

kgajdamo Aug 24, 2022

Choose a reason for hiding this comment

rusty1s Aug 24, 2022

Choose a reason for hiding this comment

kgajdamo Aug 24, 2022 • edited by rusty1s Loading

Choose a reason for hiding this comment

rusty1s Aug 24, 2022 • edited Loading

Choose a reason for hiding this comment

ZenoTan Aug 25, 2022

Choose a reason for hiding this comment

kgajdamo Aug 25, 2022

Choose a reason for hiding this comment

ZenoTan Aug 25, 2022 • edited Loading

Choose a reason for hiding this comment

DamianSzwichtenberg Aug 26, 2022 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Aug 25, 2022 • edited Loading

Codecov Report

rusty1s commented Aug 26, 2022

kgajdamo commented Aug 23, 2022 •

edited by rusty1s

Loading

kgajdamo Aug 24, 2022 •

edited by rusty1s

Loading

rusty1s Aug 24, 2022 •

edited

Loading

ZenoTan Aug 25, 2022 •

edited

Loading

DamianSzwichtenberg Aug 26, 2022 •

edited

Loading

codecov-commenter commented Aug 25, 2022 •

edited

Loading