2018-06-23 13:00 BST

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0002956OpenFOAM[All Projects] Bugpublic2018-06-01 08:58
Assigned Towill 
Product Version 
Target VersionFixed 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>();
            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.
Attached Files




will (manager)

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 (manager)

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
+Issue History