CFD Online Logo CFD Online URL
www.cfd-online.com
[Sponsors]
Home > Forums > Software User Forums > OpenFOAM > OpenFOAM Programming & Development

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

Register Blogs Members List Search Today's Posts Mark Forums Read

Like Tree3Likes
  • 3 Post By alexeym

Reply
 
LinkBack Thread Tools Search this Thread Display Modes
Old   April 28, 2016, 06:01
Default reduce(f,sumOp<scalarField>) ambiguous conversion with clang
  #1
Member
 
Join Date: Aug 2012
Posts: 33
Rep Power: 13
gigilentini8 is on a distinguished road
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
gigilentini8 is offline   Reply With Quote

Old   April 29, 2016, 05:19
Default
  #2
Senior Member
 
Alexey Matveichev
Join Date: Aug 2011
Location: Nancy, France
Posts: 1,930
Rep Power: 38
alexeym has a spectacular aura aboutalexeym has a spectacular aura about
Send a message via Skype™ to alexeym
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.
bigphil, olegrog and 2538sukham like this.
alexeym is offline   Reply With Quote

Old   May 5, 2021, 08:34
Default
  #3
Senior Member
 
Hojatollah Gholami
Join Date: Jan 2019
Posts: 171
Rep Power: 7
Hgholami is on a distinguished road
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
Hgholami is offline   Reply With Quote

Old   December 7, 2021, 13:54
Default
  #4
Super Moderator
 
bigphil's Avatar
 
Philip Cardiff
Join Date: Mar 2009
Location: Dublin, Ireland
Posts: 1,086
Rep Power: 34
bigphil will become famous soon enoughbigphil will become famous soon enough
Quote:
Originally Posted by alexeym View Post
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.
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.
bigphil is offline   Reply With Quote

Old   December 7, 2021, 16:13
Default
  #5
Senior Member
 
Mark Olesen
Join Date: Mar 2009
Location: https://olesenm.github.io/
Posts: 1,679
Rep Power: 40
olesen has a spectacular aura aboutolesen has a spectacular aura about
Quote:
Originally Posted by bigphil View Post
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 is offline   Reply With Quote

Old   December 7, 2021, 16:26
Default
  #6
Senior Member
 
Mark Olesen
Join Date: Mar 2009
Location: https://olesenm.github.io/
Posts: 1,679
Rep Power: 40
olesen has a spectacular aura aboutolesen has a spectacular aura about
Quote:
Originally Posted by bigphil View Post
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).
olesen is offline   Reply With Quote

Old   December 8, 2021, 12:32
Default
  #7
Super Moderator
 
bigphil's Avatar
 
Philip Cardiff
Join Date: Mar 2009
Location: Dublin, Ireland
Posts: 1,086
Rep Power: 34
bigphil will become famous soon enoughbigphil will become famous soon enough
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.

Last edited by bigphil; December 8, 2021 at 12:48. Reason: Add comment about clang on linux showing the same behaviour as on macOS
bigphil is offline   Reply With Quote

Old   December 8, 2021, 13:14
Default
  #8
Super Moderator
 
bigphil's Avatar
 
Philip Cardiff
Join Date: Mar 2009
Location: Dublin, Ireland
Posts: 1,086
Rep Power: 34
bigphil will become famous soon enoughbigphil will become famous soon enough
Quote:
Originally Posted by olesen View Post
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 Reduce operation for an array 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.

Last edited by bigphil; December 8, 2021 at 13:19. Reason: Add link to other thread
bigphil is offline   Reply With Quote

Old   December 8, 2021, 15:28
Default
  #9
Senior Member
 
Mark Olesen
Join Date: Mar 2009
Location: https://olesenm.github.io/
Posts: 1,679
Rep Power: 40
olesen has a spectacular aura aboutolesen has a spectacular aura about
Quote:
Originally Posted by bigphil View Post
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?

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.
olesen is offline   Reply With Quote

Old   December 8, 2021, 18:45
Default
  #10
Super Moderator
 
bigphil's Avatar
 
Philip Cardiff
Join Date: Mar 2009
Location: Dublin, Ireland
Posts: 1,086
Rep Power: 34
bigphil will become famous soon enoughbigphil will become famous soon enough
Great, thanks for this. I will come back and post the code that works in this case.
bigphil is offline   Reply With Quote

Old   December 9, 2021, 10:28
Default
  #11
Super Moderator
 
bigphil's Avatar
 
Philip Cardiff
Join Date: Mar 2009
Location: Dublin, Ireland
Posts: 1,086
Rep Power: 34
bigphil will become famous soon enoughbigphil will become famous soon enough
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)
bigphil is offline   Reply With Quote

Old   December 9, 2021, 13:57
Default
  #12
Senior Member
 
Mark Olesen
Join Date: Mar 2009
Location: https://olesenm.github.io/
Posts: 1,679
Rep Power: 40
olesen has a spectacular aura aboutolesen has a spectacular aura about
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);
olesen is offline   Reply With Quote

Old   December 9, 2021, 15:49
Default
  #13
Super Moderator
 
bigphil's Avatar
 
Philip Cardiff
Join Date: Mar 2009
Location: Dublin, Ireland
Posts: 1,086
Rep Power: 34
bigphil will become famous soon enoughbigphil will become famous soon enough
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

Last edited by bigphil; December 9, 2021 at 15:54. Reason: Add example definition using tmp
bigphil is offline   Reply With Quote

Old   December 9, 2021, 16:00
Default
  #14
Senior Member
 
Mark Olesen
Join Date: Mar 2009
Location: https://olesenm.github.io/
Posts: 1,679
Rep Power: 40
olesen has a spectacular aura aboutolesen has a spectacular aura about
Quote:
Originally Posted by bigphil View Post
...
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.
olesen is offline   Reply With Quote

Old   December 9, 2021, 16:04
Default
  #15
Super Moderator
 
bigphil's Avatar
 
Philip Cardiff
Join Date: Mar 2009
Location: Dublin, Ireland
Posts: 1,086
Rep Power: 34
bigphil will become famous soon enoughbigphil will become famous soon enough
Quote:
Originally Posted by olesen View Post
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!
bigphil is offline   Reply With Quote

Reply

Thread Tools Search this Thread
Search this Thread:

Advanced Search
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are Off
Pingbacks are On
Refbacks are On


Similar Threads
Thread Thread Starter Forum Replies Last Post
[OpenFOAM.org] compile error in dynamicMesh and thermophysicalModels libraries NickG OpenFOAM Installation 3 December 30, 2019 01:21
this->refValue()[faceI] = ?? peteryuan OpenFOAM Programming & Development 14 August 29, 2018 15:11
CFD by anderson, chp 10.... supersonic flow over flat plate varunjain89 Main CFD Forum 18 May 11, 2018 08:31
[Commercial meshers] star-ccm mesh to O\/F DLC OpenFOAM Meshing & Mesh Conversion 77 September 19, 2016 10:25
Mesh conversion exits prematurely (Salome to OF) DMcP OpenFOAM 0 July 14, 2011 03:07


All times are GMT -4. The time now is 11:06.