CFD Online Discussion Forums - reduce(f,sumOp<scalarField>) ambiguous conversion with clang

CFD Online Discussion Forums (https://www.cfd-online.com/Forums/)

- OpenFOAM Programming & Development (https://www.cfd-online.com/Forums/openfoam-programming-development/)

- - reduce(f,sumOp<scalarField>) ambiguous conversion with clang (https://www.cfd-online.com/Forums/openfoam-programming-development/170282-reduce-f-sumop-scalarfield-ambiguous-conversion-clang.html)

gigilentini8

April 28, 2016 05:01

reduce(f,sumOp<scalarField>) ambiguous conversion with clang

hi!

I have a scalarField that has to contain some global averaging. when I do the reduce operation to make it work in parallel, clang raises the following error

Code:

OpenFOAM-3.0.x/src/OpenFOAM/lnInclude/ops.H:162:9: error: conversion from 'tmp<Field<typename typeOfSum<double, double>::type> >'

      (aka 'tmp<Field<double> >') to 'Foam::Field<double>' is ambiguous

Op(sum, x + y)

        ^~~~~

OpenFOAM-3.0.x/src/OpenFOAM/lnInclude/ops.H:130:20: note: expanded from macro 'Op'

            return op;                                                        \

                   ^~

OpenFOAM-3.0.x/src/OpenFOAM/lnInclude/gatherScatter.C:90:21: note: in instantiation of member function

      'Foam::sumOp<Foam::Field<double> >::operator()' requested here

            Value = bop(Value, value);

                    ^

OpenFOAM-3.0.x/src/OpenFOAM/lnInclude/PstreamReduceOps.H:57:14: note: in instantiation of function template specialization

      'Foam::Pstream::gather<Foam::Field<double>, Foam::sumOp<Foam::Field<double> > >' requested here

    Pstream::gather(comms, Value, bop, tag, comm);

             ^

OpenFOAM-3.0.x/src/OpenFOAM/lnInclude/PstreamReduceOps.H:74:9: note: in instantiation of function template specialization

      'Foam::reduce<Foam::Field<double>, Foam::sumOp<Foam::Field<double> > >' requested here

        reduce(UPstream::linearCommunication(comm), Value, bop, tag, comm);

        ^

myScalarTransportFoam.C:77:13: note: in instantiation of function template specialization 'Foam::reduce<Foam::Field<double>, Foam::sumOp<Foam::Field<double>

      > >' requested here

            reduce(f,sumOp<scalarField>());

            ^

OpenFOAM-3.0.x/src/OpenFOAM/lnInclude/tmp.H:120:16: note: candidate function

        inline operator const T&() const;

               ^

OpenFOAM-3.0.x/src/OpenFOAM/lnInclude/Field.H:215:9: note: candidate constructor

        Field(const tmp<Field<Type> >&);

any clue?

thanks

alexeym

April 29, 2016 04:19

Hi,

Since ambiguity is between constructor from tmp and operator returning reference, you can suppress constructor from tmp using NoConstructFromTmp symbol definition before including Field.H header. For example compilation of this:

Code:

#include "Field.H"



int main(int argc, char *argv[])

{

  scalarField T(10);

  forAll(T, i)

  {

    T[i] = ::exp(static_cast<scalar>(i));

  }

  reduce(T, sumOp<scalarField>());

  Info<< T << endl;

}

ends with ambiguity error, while this:

Code:

#define NoConstructFromTmp

#include "Field.H"

#undef NoConstructFromTmp



int main(int argc, char *argv[])

{

  scalarField T(10);

  forAll(T, i)

  {

    T[i] = ::exp(static_cast<scalar>(i));

  }

  reduce(T, sumOp<scalarField>());

  Info<< T << endl;

}

compiles without errors. If you do not include Field.H directly (for example you use fvCFD.H header) define NoConstructFromTmp before inclusion of the headers, which includes Field.H.

Hgholami

May 5, 2021 07:34

Hi,
I looked at modified dynamicMesh code (zoneVelocityLaplacianFvMotionSolver) that developed from refVelocityLaplacianFvMotionSolver.
This code developed by Dr. Stefan Hoerner a long ago, applied dynamicMesh to specific pointZone.
This code work in serial running but for parallel is not work.
I try to check line by line and think the problem comes from here

Quote:

Foam::tmp<Foam::pointField>
Foam::zoneVelocityLaplacianFvMotionSolver::curPoin ts() const
{
tmp<pointField> tcurPoints(new pointField(fvMesh_.allPoints()));
pointField& cp = tcurPoints();
const pointField& pointMotionUI = pointMotionU_.internalField();

const pointZone& pz = fvMesh_.pointZones()[movingPointsZone_];
forAll(pz, pointI)
{
cp[pz[pointI]] +=
pointMotionUI[pz[pointI]]*fvMesh_.time().deltaT().value();
}

twoDCorrectPoints(tcurPoints());

return tcurPoints;
}

for detail, I think here the problem occurs.

Quote:

const pointZone& pz = fvMesh_.pointZones()[movingPointsZone_];
forAll(pz, pointI)
{
cp[pz[pointI]] +=
pointMotionUI[pz[pointI]]*fvMesh_.time().deltaT().value();
}

I thought to use any parallel operators such as "reduce".
May I think true? or any hint for this problem.
Thanks

bigphil

December 7, 2021 12:54

Quote:

Originally Posted by alexeym (Post 597268)

Code:

#include "Field.H"



int main(int argc, char *argv[])

{

  scalarField T(10);

  forAll(T, i)

  {

    T[i] = ::exp(static_cast<scalar>(i));

  }

  reduce(T, sumOp<scalarField>());

  Info<< T << endl;

}

ends with ambiguity error, while this:

Code:

#define NoConstructFromTmp

#include "Field.H"

#undef NoConstructFromTmp



int main(int argc, char *argv[])

{

  scalarField T(10);

  forAll(T, i)

  {

    T[i] = ::exp(static_cast<scalar>(i));

  }

  reduce(T, sumOp<scalarField>());

  Info<< T << endl;

}

compiles without errors. If you do not include Field.H directly (for example you use fvCFD.H header) define NoConstructFromTmp before inclusion of the headers, which includes Field.H.

Hello Alexey!

Thank you for this. Unfortunately, the "NoConstructFromTmp" variable has been removed since at least OpenFOAM-7. Do you have any other suggested solutions?

For example, if I add the following line to any OpenFOAM code (e.g. laplacianFoam):

Code:

surfaceVectorField n = mesh.Sf()/mesh.magSf();

then I get this error with clang:

Code:

laplacianFoam.C:52:24: error: conversion from 'tmp<GeometricField<Foam::Vector<double>, fvsPatchField, Foam::surfaceMesh>>' to 'Foam::surfaceVectorField' (aka 'GeometricField<Vector<double>, fvsPatchField, Foam::surfaceMesh>') is ambiguous

    surfaceVectorField n = mesh.Sf()/mesh.magSf();

                       ^   ~~~~~~~~~~~~~~~~~~~~~~

/Users/philipc/OpenFOAM/OpenFOAM-7/src/OpenFOAM/lnInclude/tmp.H:153:16: note: candidate function

        inline operator const T&() const;

               ^

/Users/philipc/OpenFOAM/OpenFOAM-7/src/OpenFOAM/lnInclude/GeometricField.H:392:9: note: candidate constructor

        GeometricField

        ^

One solution seems to be using a different constructor, e.g. this works (even though it seems like the same constructor to me!):

Code:

surfaceVectorField n(mesh.Sf()/mesh.magSf());

However, this is not a practical solution for a library I would like to compile.

By the way, I am using Apple clang version 12.0.5 (clang-1205.0.22.9) on a m1 Mac.

olesen

December 7, 2021 15:13

Quote:

Originally Posted by bigphil (Post 818102)

Hello Alexey!

Thank you for this. Unfortunately, the "NoConstructFromTmp" variable has been removed since at least OpenFOAM-7. Do you have any other suggested solutions?

As I see it, the problem isn't really with NoConstructFromTmp or clang, but rather what are we trying to express here? For example,

Code:

scalarField fld(1+2*Pstream::nProcs());



forAll(fld, i)

{

    fld[i] = i;  // Not exciting, but just for example

}



reduce(fld, sumOp<scalarField>());

Is this supposed to sum things element-wise and then scatter??? What happens when you are summing different length fields (as here). I really don't see what this should mean.

olesen

December 7, 2021 15:26

Quote:

Originally Posted by bigphil (Post 818102)

One solution seems to be using a different constructor, e.g. this works (even though it seems like the same constructor to me!):

Code:

surfaceVectorField n(mesh.Sf()/mesh.magSf());

However, this is not a practical solution for a library I would like to compile.

Hi Phil,
They are almost, but not quite the same. With the assignment version, the compiler is spoiled for choice.

Code:

surfaceVectorField lhs = (mesh.Sf()/mesh.magSf());

The compiler wants to rewrite this for you, but the RHS is a tmp Field. So use the cast from tmp to const Field (defined in tmp.H) and use that for the assignment, copy/move tmp to Field (defined in Field.H) and use the resulting Field with copy ellision to generate content for LHS. So it simply complains that there are too many things possible and bails on you.

I'm not really sure what gcc does here. In the cast operation (tmp.H) it is using a const reference, whereas Field.H one is stealing content. Would be interesting to try and report the data addresses to see which compilation branch it is taking: if it is creating the intermediate and copying the contents or actually steals the contents.

FWIW we've been addressing clang pickiness for a long-time, so most of OpenFOAM itself should compile cleanly with clang. Occasionally we do have some things with template specialization in the wrong namespace that newer clang and gcc accepts but older gcc rejects. We try to cover the range from gcc-4.8.5 -> gcc-11.2 and some version of clang up to clang-13 (most recent on linux). Occasionally also do see some oddness with clang optimization though. You'll see some things marked as "volatile" in the EtoHthermo.H (for example), also recently hit a weird branching optimization with vtk parallel output (sigh).

bigphil

December 8, 2021 11:32

Hi Mark,

Thanks for your insightful reply!

My adventure into the world of clang only started earlier this year when I got an Apple M1 machine (GCC is not supported as far as I can see). I am trying to get solids4foam compiling with clang but have many errors like the ones above (n = ... but also reduce statements). Out of curiosity I will now also try clang-12 on ubuntu just in case there are some differences. However, from what you are saying it seems like clang is just being more strict than gcc with these ambiguous statements and so I probably just need to fix them!

Philip

Edit: I tried on Ubuntu 20.04 using clang-12 and I see the same behaviour as on macOS (which is good to see but expected). So I will start to "de-ambiguify" (it should be a word if it's not) my code.

bigphil

December 8, 2021 12:14

Quote:

Originally Posted by olesen (Post 818109)

Code:

scalarField fld(1+2*Pstream::nProcs());



forAll(fld, i)

{

    fld[i] = i;  // Not exciting, but just for example

}



reduce(fld, sumOp<scalarField>());

In the case above, fld.size() is the same on each core (unless you meant to use "Pstream::myProcNo()" or similar in the constructor).
If I wanted to perform an element by element sum, such that fld is exactly the same on each core, how would you suggest that is done?

To be more explicit, consider the following trivial example:

Code:

scalarField fld(3);



forAll(fld, i)

{

    fld[i] = Pstream::myProcNo();

}

// Assuming 3 cores, then:

// On core 0, fld = (0 0 0)

// On core 1, fld = (1 1 1)

// On core 2, fld = (2 2 2)



reduce(fld, sumOp<scalarField>());

// Ideally, I want element-by-element summing to produce:

// On core 0, fld = (3 3 3)

// On core 1, fld = (3 3 3)

// On core 2, fld = (3 3 3)

Edit: This post https://www.cfd-online.com/Forums/op...tml#post316767 suggests that above works, which it does with gcc, but it does not compile with clang. However, I understand the ambiguity in this case that clang has an issue with.

olesen

December 8, 2021 14:28

Quote:

Originally Posted by bigphil (Post 818171)

Yes, I meant to illustrate that sumOp with different size fields doesn't make much sense. If you do however have identical field sizes, you should be able to define your own operator. For example,

Code:

template<class T>

struct bigPhilSumOp

{

       Field<T> operator(const Field<T>& x, const Field<T>& y) const

       {

           Field<T> result(x);

           forAll(x, i)

           {

               x[i] += y[i];

           }

           return result;

       }

};



reduce(fld, bigPhilSumOp<label>());

Haven't checked it what I wrote actually works. There might be something similar lurking in the ListListOps.H (haven't checked either). I hope it at least gets you in the right direction.

bigphil

December 8, 2021 17:45

Great, thanks for this. I will come back and post the code that works in this case.

bigphil

December 9, 2021 09:28

The definition below seems to work. You can save it as a file called FieldSumOp.H (although the name is a bit ambiguous), and then include it as you need (and make sure to include the GPL header).

Code:

#ifndef FieldSumOp_H

#define FieldSumOp_H



#include "Field.H"

#include "tmp.H"



// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //                                                                                                                              



namespace Foam

{



    template<class Type>

    class FieldSumOp

    {

        public:



        // Op function                                                                                                                                                                                       

        tmp< Field<Type> > operator()(const Field<Type>& x, const Field<Type>& y) const

        {

            tmp<Field<Type> > tresult(new Field<Type>(x));

#ifdef OPENFOAMESIORFOUNDATION

            // OpenFOAM.org or OpenFOAM.com

            Field<Type>& result = tresult.ref();

#else

            // foam-extend-4.0 (and maybe 4.1, I haven't checked)

            Field<Type>& result = tresult();

#endif



            forAll(result, i)

            {

                result[i] += y[i];

            }



            return tresult;

        }

    };



// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //                                                                                                                              



} // End namespace Foam                                                                                                                                                                                      





// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //                                                                                                                              



#endif

It can be used like:

Code:

scalarField fld(3);



forAll(fld, i)

{

    fld[i] = Pstream::myProcNo();

}



// Assuming 3 cores, then:

// On core 0, fld = (0 0 0)

// On core 1, fld = (1 1 1)

// On core 2, fld = (2 2 2)



reduce(fld, FieldSumOp<scalar>());

// Then, the result is:

// On core 0, fld = (3 3 3)

// On core 1, fld = (3 3 3)

// On core 2, fld = (3 3 3)

olesen

December 9, 2021 12:57

What about something as simple as this?

Code:

template<class Type>

struct FieldSumOp

{                                                                       

     Field<Type> operator()(const Field<Type>& x, const Field<Type>& y) const

     {

         return Field<Type>(x + y);

    }

};



// ... later on



scalarField fld(3, scalar(Pstream::myProcNo()));



Pout<< "local: " << fld << nl;

reduce(fld, FieldSumOp<scalar>());

Pout<< "reduced: " << fld << nl;

In general we now prefer 'struct' to 'class' for type traits and simple functors.
If you are only doing this once, a lambda should also be OK. For example,

Code:

reduce

(

    fld,

    [](const scalarField& x, const scalarField& y)

    {

        return scalarField(x + y);

    }

  );

Seems to compile and run ok with clang-13.

If you are doing the same thing several times locally, can pack the lamda into a variable

Code:

auto fieldReducer =

    [](const scalarField& x, const scalarField& y)

    {

        return scalarField(x + y);

    };





// ... 



reduce(fld1, fieldReducer);

reduce(fld2, fieldReducer);

bigphil

December 9, 2021 14:49

Nice options, thanks!

I have yet to use a lambda in C++; I often don't think of it but they are clearly convenient.

In my case, I perform a field reduce operation like this across many files and classes and so defining it in a header files is easiest. But your suggestion is certainly tidier (no need to explicitly specify the forAll loop). I guess using a tmp may be a good idea if the fields may not be small. Sure, I will use a struct.

For reference, I use this approach in solids4foam for syncing "global" interface fields in FSI; a more elegant/efficient parallelisation strategy would avoid the need for these synced global fields, so I probably/hopefully won't need it in the future.

Edit: example struct definition using tmp and without the need for #ifdef

Code:

#ifndef FieldSumOp_H

#define FieldSumOp_H



#include "Field.H"

#include "tmp.H"



// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //                                                                                                                              



namespace Foam

{



    template<class Type>

    struct FieldSumOp

    {

        public:



        // Op function                                                                                                                                                                                       

        tmp< Field<Type> > operator()

        (

            const Field<Type>& x, const Field<Type>& y

        ) const

        {

            return tmp< Field<Type> >(new Field<Type>(x + y));

        }

    };



// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //                                                                                                                              



} // End namespace Foam

olesen

December 9, 2021 15:00

Quote:

Originally Posted by bigphil (Post 818257)

...
I guess using a tmp may be a good idea if the fields may not be small.

Difficult to assess that one very quickly. If you return a plain Field (non-tmp), the subsequent assignment in the reduction should be triggering the move assignment, which should be memory efficient regardless of the field size. If you count the total number of heap allocations, wrapping this one as a tmp will actually increase it.

There are still other places and reasons for "tmp" (or my new favourite "refPtr"), but I don't think that this is one of them.

bigphil

December 9, 2021 15:04

Quote:

Originally Posted by olesen (Post 818258)

Difficult to assess that one very quickly. If you return a plain Field (non-tmp), the subsequent assignment in the reduction should be triggering the move assignment, which should be memory efficient regardless of the field size.
There are still other places and reasons for "tmp" (or my new favourite "refPtr"), but I don't think that this is one of them.

Ah, OK, I did not realise that a move operator would be triggered. In that case, I'll get rid of the tmp.

Thanks again!

All times are GMT -4. The time now is 17:20.