View Issue Details

IDProjectCategoryView StatusLast Update
0002956OpenFOAMBugpublic2018-06-01 08:58
Reporterniklas Assigned Towill  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Fixed in Versiondev 
Summary0002956: multiprocessor disc ConeNozzleInjection
DescriptionThe calculation of tanVec1_, tanVec2_, beta and frac in ConeNozzleInjection.C
(and also in ConeInjection) are using the sample01 which leads to different
values for these variables on each processors. So when a parcel is injected it will have different position on each processor. Which means it can have a position on each processor that is outside the domain.
Just change the calls of sample01 to globalSample01 at these positions.

In the constructor:
        vector v = rndGen.globalSample01<vector>();


In setPositionAndCell:
        scalar beta = mathematical::twoPi*rndGen.globalSample01<scalar>();
and
            scalar frac = rndGen.globalSample01<scalar>();




Steps To ReproduceTake the aachenBomb tutorial, increase the outerDiameter to 0.01
Do a simple decomposition on 8 processors as (2 2 2) and run it.
run 2 iterations by setting
endTime 5.0e-6;
deltaT 2.5e-06;

run it on single processor and compare the mass introduced
Additional InformationThis might be present in other injection models as well, but I havent checked.
TagsNo tags attached.

Activities

will

2018-05-31 14:28

manager   ~0009678

The first one shouldn't be needed. Perpendicular vectors should be calculable reliably without random numbers.

The second one you are probably right. I don't like it, though. It's very inefficient to be doing a synchronisation every time the generator is called, and it might lead to non-randomness the next time the non-global-sample01 method is called.

For positioning, the random number generator should really stay synchronised across all processors. E.g., this is being done:

    if (/* processor specific condition */)
    {
        const scalar r = rndGen.sample01();
        // do something with r ...
    }

When the pattern should be:

    const scalar r = rndGen.sample01();
    if (/* processor specific condition */)
    {
        // do something with r ...
    }

For other stuff (e.g., velocities, diameters, turbulent dispersion, MPPIC-isotropy, DSMC-everything) we don't want the processors in sync. Ideally, we want to seed differently on the different cores so that each processor has a unique random sequence.

We need sub-models to be able to create their own generators and seed and/or sync them appropriately for their intended purpose. Unfortunately the Random and cachedRandom classes are wrappers over global-static-based systems, so this isn't possible at the moment (see bug 2772). They need rewriting with class-local state, probably using the random number implementation in C++11.

For now I'll replace the constructor call with a non-random algorithm, and do as you suggest in setPositionAndCell. Should be done later today.

will

2018-05-31 17:47

manager   ~0009679

Fixed in dev by commit 11a3b3b9cd

Issue History

Date Modified Username Field Change
2018-05-25 06:46 niklas New Issue
2018-05-31 14:28 will Note Added: 0009678
2018-05-31 17:47 will Assigned To => will
2018-05-31 17:47 will Status new => resolved
2018-05-31 17:47 will Resolution open => fixed
2018-05-31 17:47 will Fixed in Version => dev
2018-05-31 17:47 will Note Added: 0009679