peterwittek / somoclu

Massively parallel self-organizing maps: accelerate training on multicore CPUs, GPUs, and clusters
https://peterwittek.github.io/somoclu/
MIT License
268 stars 70 forks source link

sparseCpuKernels.cpp(81): error C3005: 'collapse': unexpected token encountered on OpenMP 'parallel for' directive with MSVC #111

Closed xgdgsc closed 6 years ago

xgdgsc commented 6 years ago

https://github.com/peterwittek/somoclu/blob/master/src/sparseCpuKernels.cpp#L81 causes error with microsoft visual c++ compiler as it only supports openmp 2.0 (https://stackoverflow.com/questions/43318137/using-openmp-3-4-in-visual-studio-2017).

peterwittek commented 6 years ago

This is horrible. We removed the defences from around every OpenMP pragma, since a half-decent compiler should just ignore them. It was commit f46fe62e8cf83b5aaeca313807021c3313412f7c. If you have a few minutes, could you add them back?

xgdgsc commented 6 years ago

That' s not the problem. It' s just the collapse introduced in https://github.com/peterwittek/somoclu/commit/fe40bf4f02f3747a9e9dec940476dc4b704e3e6d#diff-f46df6d66593d5760985227f021a1affR87 isn' t supported.

peterwittek commented 6 years ago

Would it solve the problem if we surrounded that one pragma with macro guards?

xgdgsc commented 6 years ago
#ifdef _WIN32
    #pragma omp parallel for
#else
    #pragma omp parallel for collapse(2)
#endif
    for (omp_iter_t som_y = 0; som_y < map.nSomY; som_y++) {
        for (omp_iter_t som_x = 0; som_x < map.nSomX; som_x++) {
            size_t idx = som_y * map.nSomX + som_x;
            float acc = 0.;
            for ( unsigned int d = 0; d < map.nDimensions; d++ ) {
                acc += map.codebook[idx * map.nDimensions + d] * map.codebook[idx * map.nDimensions + d];
            }
            W2[idx] = acc;
        }
    }

Would this be enough? Does @yoch have any comment?

peterwittek commented 6 years ago

I am cool with it. Thanks.

yoch commented 6 years ago

Should be OK