Difficulty converting a for loop to parallel for, possible lack of understanding

[Carey]

I'm having a very confusing time trying to get a loop parallelised.

Here is the original loop:

[cpp]// find F initially for all allowed pairwise interactions, store the vector and move the particle
for (i=0;iparticleSetLength;i++) {
        this->particleSet.interactWithAll(this->particleSet,this->particleSetLength,i);
        this->particleSet.accountForInertia();
        this->particleSet.addCurrentVectorToMemory();
        this->particleSet.moveParticle(distance/2);
}[/cpp]

And here is the TBB version, which I've got in its own class file

[cpp]#include "../particle/Particle.h"
#include "tbb/tbb.h"
#include "tbb/parallel_for.h"
#include "tbb/task_scheduler_init.h"
#include "tbb/blocked_range.h"

using namespace tbb;
   class FindInitialFandMove {
       Particle *const my_p;
       int numP;
       int dist;
   public:
    void operator()( const blocked_range& r ) const {
        Particle *localParticles = my_p;
        int numParticles = numP;
        int distance = dist;
        for( size_t i=r.begin(); i!=r.end(); ++i )  {
                localParticles.interactWithAll(localParticles,numP,(int)i);
                localParticles.accountForInertia();
                localParticles.addCurrentVectorToMemory();
                localParticles.moveParticle(dist/2);
            }
    }
    FindInitialFandMove( Particle localParticles[], int numParticles, int distance) :
        my_p(localParticles), numP(numParticles),dist(distance)
    {}
  };[/cpp]

All this compiles, until I get to the method call to invoke it, which I call from another class that makes use of this (where the original for loop came from)

[cpp]void Capsule::FindInitialFandMove(Particle localParticles[], int numParticles, int distance ) {
    parallel_for(blocked_range(0,(size_t)this->particleSetLength), FindInitialFandMove(this->particleSet,this->particleSetLength,this->stepSize));
}
[/cpp]

When I compile it, I get the following errors:

[cpp]1>        I:threadBuildingBlocksincludetbb/parallel_for.h(211) : see declaration of 'tbb::strict_ppl::parallel_for'
1>.capsuleCapsule.cpp(681) : error C2780: 'Function tbb::strict_ppl::parallel_for(Index,Index,Index,Function)' : expects 4 arguments - 2 provided
1>        I:threadBuildingBlocksincludetbb/parallel_for.h(206) : see declaration of 'tbb::strict_ppl::parallel_for'
1>.capsuleCapsule.cpp(681) : error C2780: 'void tbb::parallel_for(const Range &,const Body &,tbb::affinity_partitioner &,tbb::task_group_context &)' : expects 4 arguments - 2 provided
1>        I:threadBuildingBlocksincludetbb/parallel_for.h(175) : see declaration of 'tbb::parallel_for'
1>.capsuleCapsule.cpp(681) : error C2780: 'void tbb::parallel_for(const Range &,const Body &,const tbb::auto_partitioner &,tbb::task_group_context &)' : expects 4 arguments - 2 provided
1>        I:threadBuildingBlocksincludetbb/parallel_for.h(168) : see declaration of 'tbb::parallel_for'
1>.capsuleCapsule.cpp(681) : error C2780: 'void tbb::parallel_for(const Range &,const Body &,const tbb::simple_partitioner &,tbb::task_group_context &)' : expects 4 arguments - 2 provided
1>        I:threadBuildingBlocksincludetbb/parallel_for.h(161) : see declaration of 'tbb::parallel_for'
1>.capsuleCapsule.cpp(681) : error C2780: 'void tbb::parallel_for(const Range &,const Body &,tbb::affinity_partitioner &)' : expects 3 arguments - 2 provided
1>        I:threadBuildingBlocksincludetbb/parallel_for.h(153) : see declaration of 'tbb::parallel_for'
1>.capsuleCapsule.cpp(681) : error C2780: 'void tbb::parallel_for(const Range &,const Body &,const tbb::auto_partitioner &)' : expects 3 arguments - 2 provided
1>        I:threadBuildingBlocksincludetbb/parallel_for.h(146) : see declaration of 'tbb::parallel_for'
1>.capsuleCapsule.cpp(681) : error C2780: 'void tbb::parallel_for(const Range &,const Body &,const tbb::simple_partitioner &)' : expects 3 arguments - 2 provided
1>        I:threadBuildingBlocksincludetbb/parallel_for.h(139) : see declaration of 'tbb::parallel_for'
1>.capsuleCapsule.cpp(681) : error C2784: 'void tbb::parallel_for(const Range &,const Body &)' : could not deduce template argument for 'const Body &' from 'void'
1>        I:threadBuildingBlocksincludetbb/parallel_for.h(132) : see declaration of 'tbb::parallel_for'
[/cpp]

I'm pretty confused by this, because I can't see where the apparently missing parameters go.

Can anyone point out where I'm going wrong?

[RafSchietekat]

Namespace using-directive's are evil.

[Carey]

Quoting - Raf Schietekat

Namespace using directives are evil.

I removed that, no change to the error. That is what it's doing in the above code in fact, I put it back in case that was the problem, but it isn't.

[RafSchietekat]

Quoting - carey.pridgeongmail.com

I removed that, no change to the error. That is what it's doing in the above code in fact, I put it back in case that was the problem, but it isn't.

(Cowardly removed bogus advice.)

[RafSchietekat]

(Cowardly removed addition to bogus advice.)

Note to self: read everything, even if you're doing other things at the same time.

[vu64]

I think I have met this problem somewhere. Try creating the function object outside parallel_for then have parallel_for call it.

[RafSchietekat]

I'm still confused because the list of errors starts and ends with "see declaration of" (has something been omitted?), and I would have expected the error with declaration parallel_for.h:139 to instead appear with parallel_for.h:132 (the only overload with the matching number of parameters, in tbb22_20090809oss anyway). At least then I might have some basis to suspect the compiler (now it's just very strange all around). What is the TBB version being used here?

[Carey]

Quoting - Raf Schietekat

I'm still confused because the list of errors starts and ends with "see declaration of" (has something been omitted?), and I would have expected the error with declaration parallel_for.h:139 to instead appear with parallel_for.h:132.

I might have ommitted something, I'm not sure. I *thought* I selected it all. The last line reads

see declaration of 'tbb::parallel_for'

to me.
The previous suggestion didn't help when I tried it, so I've taken the code apart, and I'm starting again. Therefore I don't actually have the means to reproduce that trace. I do however know that I've done this before, charged ahead and not taken care to make sure that what I'm writing will actually work with the things I want it to.

I'm starting to think the issue might be that my code isn't actually parallel ready, as far as TBB needs. It works with OpenMP, but I want to support both aproaches if possible. I'll start again with something simpler, a toy project, and learn from that.

Thanks for the help.

[RafSchietekat]

"The previous suggestion didn't help"
Do you mean vu64's, about providing a named instance instead of a temporary object? I have no idea why C++ would require that, but at least it would be more comforting to perhaps be able to blame a specific compiler.

[Carey]

Quoting - Raf Schietekat

"The previous suggestion didn't help"
Do you mean vu64's, about providing a named instance instead of a temporary object? I have no idea why C++ would require that, but at least it would be more comforting to perhaps be able to blame a specific compiler.

The resulting application needs to be cross platform, Windows, Linux, Unix and MacOS, so I have to have it working without compiler/platform specific workarounds.

Not that it mattered in this case.

The program I'm applying this to has thousands of lines of code. I'm creating a smaller app using some of the things I need that will be a more suitable testbed for exploring this problem.

I do want to get this working, OpenMP is ok, but I would prefer to use TBB.

[AJ13]

Quoting - Raf Schietekat

"The previous suggestion didn't help"
Do you mean vu64's, about providing a named instance instead of a temporary object? I have no idea why C++ would require that, but at least it would be more comforting to perhaps be able to blame a specific compiler.

When I get really confused by one compiler's errors, I try a different compiler. Try gcc and see what gives.

AJ

[Carey]

Quoting - AJ

When I get really confused by one compiler's errors, I try a different compiler. Try gcc and see what gives.

AJ

That's my plan for tomorrow. I need to produce a linux build for 16 core testing.

[Alexey_K_Intel3]

You have named the loop body class for parallel_for the same as the method where you call parallel_for from:

[cpp]void Capsule::FindInitialFandMove(Particle localParticles[], int numParticles, int distance ) { 
    parallel_for(blocked_range(0,(size_t)this->particleSetLength), FindInitialFandMove(this->particleSet,this->particleSetLength,this->stepSize)); 
} 
[/cpp]

So the compiler resolves the second parameter to parallel_for as a call to Capsule::FindInitialFandMove, and not as the constructor of a body instance as you expect. The function returns void, and so parallel_for is attempted to instantiate with "void" as the type for one of its template parameters, which understandably fails.
The error list shows you why every overload of parallel_for in TBB has failed to match the call, with the very last message giving the clue.

[Carey]

Quoting - Alexey Kukanov (Intel)

You have named the loop body class for parallel_for the same as the method where you call parallel_for from:

[cpp]void Capsule::FindInitialFandMove(Particle localParticles[], int numParticles, int distance ) { 
    parallel_for(blocked_range(0,(size_t)this->particleSetLength), FindInitialFandMove(this->particleSet,this->particleSetLength,this->stepSize)); 
} 
[/cpp]

So the compiler resolves the second parameter to parallel_for as a call to Capsule::FindInitialFandMove, and not as the constructor of a body instance as you expect. The function returns void, and so parallel_for is attempted to instantiate with "void" as the type for one of its template parameters, which understandably fails.
The error list shows you why every overload of parallel_for in TBB has failed to match the call, with the very last message giving the clue.

That didn't occur to me. I'll have another go in the morning. It would be nice to have it working.

[RafSchietekat]

Now that's funny, how we could have missed that! :-)

[Carey]

Quoting - Raf Schietekat

Now that's funny, how we could have missed that! :-)

I'm not surprised I did, I can miss all sorts of stuff:) I need to sleep now, but I will repair code in the morning and then say what the result was.

[vu64]

Quoting - carey.pridgeongmail.com

I'm not surprised I did, I can miss all sorts of stuff:) I need to sleep now, but I will repair code in the morning and then say what the result was.

Glad to see the problem solved.

[Carey]

Quoting - Alexey Kukanov (Intel)

You have named the loop body class for parallel_for the same as the method where you call parallel_for from:

[cpp]void Capsule::FindInitialFandMove(Particle localParticles[], int numParticles, int distance ) { 
    parallel_for(blocked_range(0,(size_t)this->particleSetLength), FindInitialFandMove(this->particleSet,this->particleSetLength,this->stepSize)); 
} 
[/cpp]

So the compiler resolves the second parameter to parallel_for as a call to Capsule::FindInitialFandMove, and not as the constructor of a body instance as you expect. The function returns void, and so parallel_for is attempted to instantiate with "void" as the type for one of its template parameters, which understandably fails.
The error list shows you why every overload of parallel_for in TBB has failed to match the call, with the very last message giving the clue.

This was indeed what was wrong (which you already knew of course). I just tried it, and everything works perfectly now.

This is great, now my software can support OpenMP and TBB.

Thanks for the help everyone.

[vu64]

Quoting - carey.pridgeongmail.com

This was indeed what was wrong (which you already knew of course). I just tried it, and everything works perfectly now.

This is great, now my software can support OpenMP and TBB.

Thanks for the help everyone.

Although OpenMP and TBB can be used together, this can lead to problems with the scheduling. I think you'd better follow one model OpenMP or TBB.

[Carey]

Quoting - vu64

Although OpenMP and TBB can be used together, this can lead to problems with the scheduling. I think you'd better follow one model OpenMP or TBB.

The idea with this particuler peice of software is that a number of options for experiment creation are available. At the moment I'm setting it up so you can use OpenMP, or TBB, but only one or the other at any one time (I thought the TBB documentation said this was possible), I realise it's not good to mix them in the same method.

I've only just started on the concurrency aspect, and I realise that TBB has a lot more to offer, so depending on feedback I may drop OpenMP and stick with TBB.

The software itself is a framework for n-boy modelling, a lot more complex then the example on this site, this is an 'everything you need to create an experiment' product. It's also an 'eating all my spare time for months' product too. I'll be releasing it soon (APL licenced), so I should be able to get some feedback then on the OpenMP issue.