headmyshoulder / odeint-v2

odeint - solving ordinary differential equations in c++ v2
http://headmyshoulder.github.com/odeint-v2/
Other
340 stars 101 forks source link

odeint + thrust placeholders conflict #134

Closed slayoo closed 10 years ago

slayoo commented 10 years ago

Hi,

The following code:

#include <boost/numeric/odeint/external/thrust/thrust_algebra.hpp>
#include <boost/numeric/odeint/stepper/euler.hpp>
int main()
{
  using namespace thrust::placeholders;
  thrust::host_vector<int> a;
  thrust::transform(a.begin(), a.end(), a.begin(), 2 * _1);
}

fails to compile with:

bug.cpp: In function ‘int main()’:
bug.cpp:7:56: error: reference to ‘_1’ is ambiguous
   thrust::transform(a.begin(), a.end(), a.begin(), 2 * _1);
                                                        ^
In file included from /usr/lib/nvidia-cuda-toolkit/include/thrust/system/detail/error_category.inl:22:0,
                 from /usr/lib/nvidia-cuda-toolkit/include/thrust/system/error_code.h:518,
                 from /usr/lib/nvidia-cuda-toolkit/include/thrust/system/system_error.h:29,
                 from /usr/lib/nvidia-cuda-toolkit/include/thrust/system/cuda/detail/malloc_and_free.h:23,
                 from /usr/lib/nvidia-cuda-toolkit/include/thrust/system/detail/adl/malloc_and_free.h:30,
                 from /usr/lib/nvidia-cuda-toolkit/include/thrust/system/detail/generic/memory.inl:21,
                 from /usr/lib/nvidia-cuda-toolkit/include/thrust/system/detail/generic/memory.h:66,
                 from /usr/lib/nvidia-cuda-toolkit/include/thrust/detail/reference.inl:22,
                 from /usr/lib/nvidia-cuda-toolkit/include/thrust/detail/reference.h:166,
                 from /usr/lib/nvidia-cuda-toolkit/include/thrust/memory.h:25,
                 from /usr/lib/nvidia-cuda-toolkit/include/thrust/device_ptr.h:25,
                 from /usr/lib/nvidia-cuda-toolkit/include/thrust/device_malloc_allocator.h:25,
                 from /usr/lib/nvidia-cuda-toolkit/include/thrust/device_vector.h:25,
                 from /usr/include/boost/numeric/odeint/external/thrust/thrust_algebra.hpp:22,
                 from bug.cpp:1:
/usr/lib/nvidia-cuda-toolkit/include/thrust/functional.h:1020:63: note: candidates are: const type thrust::placeholders::_1
static const thrust::detail::functional::placeholder<0>::type _1;
                                                               ^
In file included from /usr/include/boost/bind/bind.hpp:1742:0,
                 from /usr/include/boost/bind.hpp:22,
                 from /usr/include/boost/numeric/odeint/util/bind.hpp:26,
                 from /usr/include/boost/numeric/odeint/stepper/base/explicit_stepper_base.hpp:25,
                 from /usr/include/boost/numeric/odeint/stepper/euler.hpp:23,
                 from bug.cpp:2:
/usr/include/boost/bind/placeholders.hpp:55:15: note:                 boost::arg<1> {anonymous}::_1
 boost::arg<1> _1;
               ^

HTH, Sylwester

mariomulansky commented 10 years ago

Hi Sylwester,

thanks for the report. I'm not yet sure if this is really an odeint issue. The clash happens between boost.bind and thrust placeholder names. odeint does include boost.bind which pulls the placeholders into global namespace, hence a using namespace thrust::placeholders leads to the clash between boost and thrust. My suggestion would be exactly what has been done in the commit referenced above: use namespace arg = thrust::placeholders instead. We could try to add some workaround in odeint but I think the core problem here is boost.bind that doesnt allow you to pull your own placeholders into the global namespace.

Does the solution above work for you or would you prefer to being able to use global placeholders for some reason?

headmyshoulder commented 10 years ago

Hi Sylvester,

goot to hear from you and that you are still using odeint.

Does this error also occurs in C++11 mode?

Best regards,

Karsten

On 04.07.2014 15:43, Mario Mulansky wrote:

Hi Sylwester,

thanks for the report. I'm not yet sure if this is really an odeint issue. The clash happens between boost.bind and thrust placeholder names. odeint does include boost.bind which pulls the placeholders into global namespace, hence a |using namespace thrust::placeholders| leads to the clash between boost and thrust. My suggestion would be exactly what has been done in the commit referenced above: use |namespace arg = thrust::placeholders| instead. We could try to add some workaround in odeint but I think the core problem here is boost.bind that doesnt allow you to pull your own placeholders into the global namespace.

— Reply to this email directly or view it on GitHub https://github.com/headmyshoulder/odeint-v2/issues/134#issuecomment-48045273.

ddemidov commented 10 years ago

Cuda in general and Thrust in particular do not support compilation in c++11 mode. On Jul 4, 2014 5:54 PM, "headmyshoulder" notifications@github.com wrote:

Hi Sylvester,

goot to hear from you and that you are still using odeint.

Does this error also occurs in C++11 mode?

Best regards,

Karsten

On 04.07.2014 15:43, Mario Mulansky wrote:

Hi Sylwester,

thanks for the report. I'm not yet sure if this is really an odeint issue. The clash happens between boost.bind and thrust placeholder names. odeint does include boost.bind which pulls the placeholders into global namespace, hence a |using namespace thrust::placeholders| leads to the clash between boost and thrust. My suggestion would be exactly what has been done in the commit referenced above: use |namespace arg = thrust::placeholders| instead. We could try to add some workaround in odeint but I think the core problem here is boost.bind that doesnt allow you to pull your own placeholders into the global namespace.

— Reply to this email directly or view it on GitHub < https://github.com/headmyshoulder/odeint-v2/issues/134#issuecomment-48045273 .

— Reply to this email directly or view it on GitHub https://github.com/headmyshoulder/odeint-v2/issues/134#issuecomment-48046504 .

slayoo commented 10 years ago

Hi,

Thanks for quick replies.

When compiling with g++, the same error appears regardless if -std=c++11 was set or not. As ddemidov mentioned, nvcc does not support c++11 at all.

Re Thrust/C++11, I have not seen a single issue with it, and moreover, employing C++11 constructs makes use of Thrust much more efficient!

Sylwester

ddemidov commented 10 years ago

Indeed, I forgot that it is possible to use thrust without cuda. On Jul 4, 2014 6:33 PM, "Sylwester Arabas" notifications@github.com wrote:

Hi,

Thanks for quick replies.

When compiling with g++, the same error appears regardless if -std=c++11 was set or not. As ddemidov mentioned, nvcc does not support c++11 at all.

Re Thrust/C++11, I have not seen a single issue with it, and moreover, employing C++11 constructs makes use of Thrust much more efficient!

Sylwester

— Reply to this email directly or view it on GitHub https://github.com/headmyshoulder/odeint-v2/issues/134#issuecomment-48050664 .

headmyshoulder commented 10 years ago

Re Thrust/C++11, I have not seen a single issue with it, and moreover, employing C++11 constructs makes use of Thrust much more efficient!

Yes, this is right.

I thought that odeint uses std::bind and its placeholders if is is compiled in C++11 mode. Seems like bug, that this does not work.

Nevertheless, Mario is right. The issue here in C++03 mode occurs due to the placeholders of boost bind which are in the global namespace. We can not do so much here.

slayoo commented 10 years ago

For the record, answering Mario's question: yes, the "namespace arg = ..." workaround of course fixes the issue.

Should it be then reported to boost::bind developers? (if anyone here anyhow receiving boost developers' mailing list would volunteer, I would ask him to report it there - this list is much too high traffic for me :))

Thanks, Sylwester

mariomulansky commented 10 years ago

Well I'm quite sure that this issue is known to the boost bind people. But I think they wouldnt change that due to backwards compatibility - this would mean a huge breaking change.

headmyshoulder commented 10 years ago

On 07/04/2014 10:30 PM, Sylwester Arabas wrote:

For the record, answering Mario's question: yes, the "namespace arg = ..." workaround of course fixes the issue.

Should it be then reported to boost::bind developers? (if anyone here anyhow receiving boost developers' mailing list would volunteer, I would ask him to report it there - this list is much too high traffic for me :))

Yes, this issue is already known, see for example:

http://stackoverflow.com/questions/13596697/c11-placeholders-with-boost

But this post also offers a solution. We could use

define BOOST_BIND_NO_PLACEHOLDERS

and then define our own placeholders, for example:

boost::arg<1> _odeint_1;

headmyshoulder commented 10 years ago

I commited a fix into the master branch. Can you check if your code now compiles?

Thanks,

Karsten

On 07/04/2014 10:30 PM, Sylwester Arabas wrote:

For the record, answering Mario's question: yes, the "namespace arg = ..." workaround of course fixes the issue.

Should it be then reported to boost::bind developers? (if anyone here anyhow receiving boost developers' mailing list would volunteer, I would ask him to report it there - this list is much too high traffic for me :))

Thanks, Sylwester

— Reply to this email directly or view it on GitHub https://github.com/headmyshoulder/odeint-v2/issues/134#issuecomment-48068919.

slayoo commented 10 years ago

Yes, it compiles fine now (checked with clang++ and g++). Thanks! Sylwester