View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002956 | OpenFOAM | Bug | public | 2018-05-25 06:46 | 2018-06-01 08:58 |
| Reporter | niklas | Assigned To | will | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Fixed in Version | dev | ||||
| Summary | 0002956: multiprocessor disc ConeNozzleInjection | ||||
| Description | The 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 Reproduce | Take 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 Information | This might be present in other injection models as well, but I havent checked. | ||||
| Tags | No tags attached. | ||||
|
|
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. |
|
|
Fixed in dev by commit 11a3b3b9cd |
| 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 |
