Use static constexpr for VS compilation

[HunterTracer]

static constexpr should be used in macro in cuda/reducer.cuh, otherwise VS compilation will fail with error C2975: 'REDUCE': invalid template argument for 'Reducer', expected compile-time constant expression

[codecov-commenter]

Codecov Report

Merging #342 (2d297cd) into master (8e6635b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #342   +/-   ##
=======================================
  Coverage   96.85%   96.85%           
=======================================
  Files           9        9           
  Lines         191      191           
=======================================
  Hits          185      185           
  Misses          6        6           

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

[rusty1s]

thanks!

[rusty1s]

It looks like this breaks our current Windows build in the CI. Do you know why?

[HunterTracer]

It seems a bug in pytorch release version. There is a conflict between include/ATen/Parallel.h inline implementation of void lazy_init_num_threads and libs in pytorch release, a simple solution is to remove the inline TORCH_API void lazy_init_num_threads() implementation and only keeps TORCH_API void lazy_init_num_threads();. A similar answer can be found in https://stackoverflow.com/questions/74366357/updating-to-visual-studio-17-4-0-yields-linker-errors-related-to-tls

[HunterTracer]

Another way is to rebuild libtorch by ourselves and not use the official release version

[rusty1s]

Thanks! I did something similar here: https://github.com/pyg-team/pyg-lib/blob/master/.github/actions/setup/action.yml#L42. Can we add the patching of Parallel.h as part of our CI?

[HunterTracer]

I think it's ok. The only thing need to do is to replace

inline TORCH_API void lazy_init_num_threads() {
  thread_local bool init = false;
  if (C10_UNLIKELY(!init)) {
    at::init_num_threads();
    init = true;
  }
}

to
TORCH_API void lazy_init_num_threads();
in torch/include/ATen/Parallel.h.

[rusty1s]

Okay, do you wanna take care of this?

[rusty1s]

We would just need to add this here: https://github.com/rusty1s/pytorch_scatter/blob/master/.github/workflows/testing.yml#L30

[HunterTracer]

Can I use python code in testing.yml to replace the context? If so, maybe I can try, since I'm not quite familiar with the bash language.

[rusty1s]

I think you can, but with sed this should be a one-liner, see here:

sed -i '3,5c\
TORCH_API void lazy_init_num_threads();' Parallel.h

where 3 and 5 indicate the start and end line you want to replace.

[HunterTracer]

This can work if Parallel.h in all versions of pytorch remains the same. If the source code change the position of lazy_init_num_threads, such method may fail.

[rusty1s]

Yes, that is correct. We would need to adjust line numbers both for PyTorch 1.12 and 1.13

[HunterTracer]

Fortunately, for pytorch 1.12 and 1.13, the positions are the same, both of them are 32-38

[HunterTracer]

I just add the text replacement

sed -i '32,38c\
TORCH_API void lazy_init_num_threads();' "$(python -c "import os, torch; print(os.path.join(os.path.dirname(torch.__file__), 'include/ATen/Parallel.h'))")"

[HunterTracer]

Sorry for that, I made a mistake, for pytorch 1.12, the lines should be 31-37.

[rusty1s]

Yeah, I think I have fixed this.

[HunterTracer]

OK, thanks!