|
|
Thanks for the proposed optimization, I would be happy to include it but first I need to convert it into standard OpenFOAM form. Can you propose a suitable test-case to validate the change and demonstrate the speed-up? |
|
|
|
|
|
I did some test lately using my own solver for simulating a bubblecolumn. unfortunately the case data wouldnt fit to the standard sprayFoam anymore. You can still have a look at the case and the first CPU time comparison. I therefore added CPU time checking only for the collide step including collideParcels substep in the code. Downside of the testcase is, that it does not have a uniformly distribution of parcels in the domain, which will of corse influence the CPU time. Most parcels will accumulate in the middle (bubble plume). |
|
|
I have tried to run the case you provided with the solver specified: icoUncoupledKinematicParcelFoam but it fails to read transportProperties. I need a case to test the changes to ORourkeCollision to ensure that I do not make any mistakes in updating the code you have provided. It can be any case which uses the ORourkeCollision model so I can make sure that the behavior of the model before and after the change is the same. |
|
|
|
|
|
I uploaded a case which should work with the standard sprayFoam solver. Nevertheless, there is a bug in ORourkeCollision wich will cause it to crash: // interpolate to find average surface tension scalar sigmaAve = sigma1 + (sigma2 - sigma1)*(Tave - T1)/(T2 - T1); solution: scalar sigmaAve = sigma1 + (sigma2 - sigma1)*(Tave - T1)/max(ROOTVSMALL,(T2 - T1)); because T1 and T2 can be the same value |
|
|
Found another major bug in ORourkeCollision: ... scalar collProb = exp(-nu); scalar xx = this->owner().rndGen().template sample01<scalar>(); // collision occurs if (xx > collProb) ---- > has to be switched to: if (xx < collProb) |
|
|
Thank you for spending the time to sort-out the ORourkeCollision model, it has had very little testing so far and not validated. |
|
|
I don't think max(ROOTVSMALL,(T2 - T1)) is the appropriate stabilization, T2 might be less than T1. |
|
|
I could not get the code snippet you provided to compile and found that there were some important details missing, in particular regarding access to the particles and changes to i and j. In the end I had to write it from scratch using OpenFOAM classes rather than STL classes adding the missing components and updating some of the other functions consistently. Along the way I found some other bugs in the code which were causing spurious crashes and also corrected the stabilization of the division by (T2 -T1). All of these changes are included in OpenFOAM-dev: commit 91e84b9004c4a9be2c135465617783f21d28dc71 Could you test these changes and let me know if this model is now working correctly? |
|
|
It all seems to work perfectly. I'll convert all my other coalescence models accordingly and will have them tested. I'll also do some more CPU time testing, I'll let you know about the results. |
|
|
CPUtime.png (7,394 bytes) |
|
|
CPU time testing done: Clearly a faster code with the new algorithm. I think it is the usage of the CompactListList which makes it a little bit slower when having a particle number under 300, but it is much more important do have no memory reallocation ( i haven't thought about that at first). Anyhow, a normal simulation (in my case) should easyly consist of over 300 parcels. |
|
|
Do you have the equivalent graph of the version with the List of DynamicList or your original version using std::vector? I would guess that using CompactListList will be slightly more expensive for very small numbers of parcels because the parcels have to be scanned twice but cheaper for larger numbers because there is only a single allocation for the ListList. |
|
|
CPUtime_detail.png (7,060 bytes) |
|
|
There you go. For a low number of parcels, it seems that the compactListList is still better than using the std::vector. dynamicList has only a slightly faster computation. I've tested up to 10^5 particles, where there is no real difference between the three "new" algorithms. |
|
|
It is interesting that the DynamicList version is slightly faster but I think that using CompactListList is likely to give good bahavior on a wider range of hardware and we should stick with it. |
|
|
Resolved by commit 91e84b9004c4a9be2c135465617783f21d28dc71 |
|
|
I had a closer look again recently: // collision occurs if (xx > collProb) is actually correct, sorry. It works because it is not the direct propability nu0 but the exp(-nu) scalar collProb = exp(-nu); scalar xx = this->owner().rndGen().template sample01<scalar>(); |
|
|
Resolved in OpenFOAM-4.x by commit 7dfa780c481b8b79b1ee4d5bcf3e6b839a5ef017 Resolved in OpenFOAM-dev by commit cc8781f47d11d4ee6a760bf356253b638851977d |
- Anonymous
