View Issue Details

IDProjectCategoryView StatusLast Update
0001304OpenFOAMBugpublic2014-05-23 12:13
Reportertniemi Assigned Touser2 
PrioritynormalSeveritymajorReproducibilityhave not tried
Status resolvedResolutionfixed 
Summary0001304: Kinematic parcel, serialisation problem when switching processors
DescriptionI'm not an expert on c++ memory layout rules, but I think that there may be a problem with the way how parcels are serialised in binary mode. Currently, when writing parcels in binary mode, a pointer is given to the first member variable of the parcel and then the length of the write is given as a sum of the member variable sizes. However, in c++ it is not guaranteed that the member variables are packed in memory and the compiler might add padding for better memory access. (the size of the object should align in memory, sizeof(obj) is not necessary sum(sizeof(member)))

In our running environment, the z-component of UTurb will always corrupt when switching from one processor to another. If I write each vector component separately, everything works ok. Also all other fields of a kinematicParcel or even a reactingMultiphaseParcel seem to work ok.

I read somewhere, that padding is usually added before the last member variable, so this would explain why there is a problem just with the z-component of UTurb, because it is the last member of the kinematicParcel.
TagsNo tags attached.

Activities

tniemi

2014-05-23 09:50

reporter  

KinematicParcelIO.C (8,258 bytes)   
/*---------------------------------------------------------------------------*\
  =========                 |
  \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
   \\    /   O peration     |
    \\  /    A nd           | Copyright (C) 2011-2013 OpenFOAM Foundation
     \\/     M anipulation  |
-------------------------------------------------------------------------------
License
    This file is part of OpenFOAM.

    OpenFOAM is free software: you can redistribute it and/or modify it
    under the terms of the GNU General Public License as published by
    the Free Software Foundation, either version 3 of the License, or
    (at your option) any later version.

    OpenFOAM is distributed in the hope that it will be useful, but WITHOUT
    ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
    FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
    for more details.

    You should have received a copy of the GNU General Public License
    along with OpenFOAM.  If not, see <http://www.gnu.org/licenses/>.

\*---------------------------------------------------------------------------*/

#include "KinematicParcel.H"
#include "IOstreams.H"
#include "IOField.H"
#include "Cloud.H"

// * * * * * * * * * * * * * * Static Data Members * * * * * * * * * * * * * //

template<class ParcelType>
Foam::string Foam::KinematicParcel<ParcelType>::propertyList_ =
    Foam::KinematicParcel<ParcelType>::propertyList();

// * * * * * * * * * * * * * * * * Constructors  * * * * * * * * * * * * * * //

template<class ParcelType>
Foam::KinematicParcel<ParcelType>::KinematicParcel
(
    const polyMesh& mesh,
    Istream& is,
    bool readFields
)
:
    ParcelType(mesh, is, readFields),
    active_(false),
    typeId_(0),
    nParticle_(0.0),
    d_(0.0),
    dTarget_(0.0),
    U_(vector::zero),
    rho_(0.0),
    age_(0.0),
    tTurb_(0.0),
    UTurb_(vector::zero),
    rhoc_(0.0),
    Uc_(vector::zero),
    muc_(0.0)
{
    if (readFields)
    {
        if (is.format() == IOstream::ASCII)
        {
            active_ = readBool(is);
            typeId_ = readLabel(is);
            nParticle_ = readScalar(is);
            d_ = readScalar(is);
            dTarget_ = readScalar(is);
            is >> U_;
            rho_ = readScalar(is);
            age_ = readScalar(is);
            tTurb_ = readScalar(is);
            is >> UTurb_;
        }
        else
        {
            /*is.read
            (
                reinterpret_cast<char*>(&active_),
                sizeof(active_)
              + sizeof(typeId_)
              + sizeof(nParticle_)
              + sizeof(d_)
              + sizeof(dTarget_)
              + sizeof(U_)
              + sizeof(rho_)
              + sizeof(age_)
              //+ sizeof(tTurb_)
              //+ sizeof(UTurb_)
            );*/
            is >> active_
               >> typeId_
               >> nParticle_
               >> d_
               >> dTarget_
               >> U_
               >> rho_
               >> age_       
               >> tTurb_
               >> UTurb_;
        }
    }

    // Check state of Istream
    is.check
    (
        "KinematicParcel<ParcelType>::KinematicParcel"
        "(const polyMesh&, Istream&, bool)"
    );
}


template<class ParcelType>
template<class CloudType>
void Foam::KinematicParcel<ParcelType>::readFields(CloudType& c)
{
    if (!c.size())
    {
        return;
    }

    ParcelType::readFields(c);

    IOField<label> active(c.fieldIOobject("active", IOobject::MUST_READ));
    c.checkFieldIOobject(c, active);

    IOField<label> typeId(c.fieldIOobject("typeId", IOobject::MUST_READ));
    c.checkFieldIOobject(c, typeId);

    IOField<scalar>
        nParticle(c.fieldIOobject("nParticle", IOobject::MUST_READ));
    c.checkFieldIOobject(c, nParticle);

    IOField<scalar> d(c.fieldIOobject("d", IOobject::MUST_READ));
    c.checkFieldIOobject(c, d);

    IOField<scalar> dTarget(c.fieldIOobject("dTarget", IOobject::MUST_READ));
    c.checkFieldIOobject(c, dTarget);

    IOField<vector> U(c.fieldIOobject("U", IOobject::MUST_READ));
    c.checkFieldIOobject(c, U);

    IOField<scalar> rho(c.fieldIOobject("rho", IOobject::MUST_READ));
    c.checkFieldIOobject(c, rho);

    IOField<scalar> age(c.fieldIOobject("age", IOobject::MUST_READ));
    c.checkFieldIOobject(c, age);

    IOField<scalar> tTurb(c.fieldIOobject("tTurb", IOobject::MUST_READ));
    c.checkFieldIOobject(c, tTurb);

    IOField<vector> UTurb(c.fieldIOobject("UTurb", IOobject::MUST_READ));
    c.checkFieldIOobject(c, UTurb);

    label i = 0;

    forAllIter(typename CloudType, c, iter)
    {
        KinematicParcel<ParcelType>& p = iter();

        p.active_ = active[i];
        p.typeId_ = typeId[i];
        p.nParticle_ = nParticle[i];
        p.d_ = d[i];
        p.dTarget_ = dTarget[i];
        p.U_ = U[i];
        p.rho_ = rho[i];
        p.age_ = age[i];
        p.tTurb_ = tTurb[i];
        p.UTurb_ = UTurb[i];

        i++;
    }
}


template<class ParcelType>
template<class CloudType>
void Foam::KinematicParcel<ParcelType>::writeFields(const CloudType& c)
{
    ParcelType::writeFields(c);

    label np =  c.size();

    IOField<label> active(c.fieldIOobject("active", IOobject::NO_READ), np);
    IOField<label> typeId(c.fieldIOobject("typeId", IOobject::NO_READ), np);
    IOField<scalar> nParticle
    (
        c.fieldIOobject("nParticle", IOobject::NO_READ),
        np
    );
    IOField<scalar> d(c.fieldIOobject("d", IOobject::NO_READ), np);
    IOField<scalar> dTarget(c.fieldIOobject("dTarget", IOobject::NO_READ), np);
    IOField<vector> U(c.fieldIOobject("U", IOobject::NO_READ), np);
    IOField<scalar> rho(c.fieldIOobject("rho", IOobject::NO_READ), np);
    IOField<scalar> age(c.fieldIOobject("age", IOobject::NO_READ), np);
    IOField<scalar> tTurb(c.fieldIOobject("tTurb", IOobject::NO_READ), np);
    IOField<vector> UTurb(c.fieldIOobject("UTurb", IOobject::NO_READ), np);

    label i = 0;

    forAllConstIter(typename CloudType, c, iter)
    {
        const KinematicParcel<ParcelType>& p = iter();

        active[i] = p.active();
        typeId[i] = p.typeId();
        nParticle[i] = p.nParticle();
        d[i] = p.d();
        dTarget[i] = p.dTarget();
        U[i] = p.U();
        rho[i] = p.rho();
        age[i] = p.age();
        tTurb[i] = p.tTurb();
        UTurb[i] = p.UTurb();

        i++;
    }

    active.write();
    typeId.write();
    nParticle.write();
    d.write();
    dTarget.write();
    U.write();
    rho.write();
    age.write();
    tTurb.write();
    UTurb.write();
}


// * * * * * * * * * * * * * * * IOstream Operators  * * * * * * * * * * * * //

template<class ParcelType>
Foam::Ostream& Foam::operator<<
(
    Ostream& os,
    const KinematicParcel<ParcelType>& p
)
{
    if (os.format() == IOstream::ASCII)
    {
        os  << static_cast<const ParcelType&>(p)
            << token::SPACE << p.active()
            << token::SPACE << p.typeId()
            << token::SPACE << p.nParticle()
            << token::SPACE << p.d()
            << token::SPACE << p.dTarget()
            << token::SPACE << p.U()
            << token::SPACE << p.rho()
            << token::SPACE << p.age()
            << token::SPACE << p.tTurb()
            << token::SPACE << p.UTurb();
    }
    else
    {
        os  << static_cast<const ParcelType&>(p);
        /*os.write
        (
            reinterpret_cast<const char*>(&p.active_),
            sizeof(p.active())
          + sizeof(p.typeId())
          + sizeof(p.nParticle())
          + sizeof(p.d())
          + sizeof(p.dTarget())
          + sizeof(p.U())
          + sizeof(p.rho())
          + sizeof(p.age())
          //+ sizeof(p.tTurb())
          //+ sizeof(p.UTurb())
        );*/
   
        os << p.active_
           << p.typeId_
           << p.nParticle_
           << p.d_
           << p.dTarget_
           << p.U_
           << p.rho_
           << p.age_       
           << p.tTurb_
           << p.UTurb_;
    }

    // Check state of Ostream
    os.check
    (
        "Ostream& operator<<(Ostream&, const KinematicParcel<ParcelType>&)"
    );

    return os;
}


// ************************************************************************* //
KinematicParcelIO.C (8,258 bytes)   

tniemi

2014-05-23 09:54

reporter   ~0003079

Hmm, I did some further testing and it seems that it is the last item of write, which gets corrupted, not necessarily just UTurb. So I'm not sure what causes the problem.

However, I attached a modified KinematicParcelIO, where I replaced the "reinterpret_cast<char*>" -write with << and >> -operators and it seems to work fine. I tested with ReactingMultiPhaseParcels, and they worked even though "reinterpret_cast<char*>" is used for some of the variables.

tniemi

2014-05-23 11:58

reporter   ~0003080

The problem is caused by the bool active_ in KinematicParcel. Because it's a bool, it has a size of 1 byte. However, for instance in my machine there are 3 bytes of padding between the active_ and typeId_ members because of memory align and this causes the problem.

user2

2014-05-23 12:13

  ~0003081

Last edited: 2014-05-23 12:13

Thanks for the report - resolved by commit 5614e6f

Issue History

Date Modified Username Field Change
2014-05-23 07:54 tniemi New Issue
2014-05-23 09:50 tniemi File Added: KinematicParcelIO.C
2014-05-23 09:54 tniemi Note Added: 0003079
2014-05-23 11:58 tniemi Note Added: 0003080
2014-05-23 12:13 user2 Note Added: 0003081
2014-05-23 12:13 user2 Status new => resolved
2014-05-23 12:13 user2 Fixed in Version => 2.3.x
2014-05-23 12:13 user2 Resolution open => fixed
2014-05-23 12:13 user2 Assigned To => user2
2014-05-23 12:13 user2 Note Edited: 0003081