View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0002634||OpenFOAM||[All Projects] Bug||public||2017-07-26 23:21||2017-07-27 16:57|
|Fixed in Version|
|Summary||0002634: limitVelocity function|
I guess the limitVelocity function has some bug. As far as I understand the function, it limit the magnitude of the velocity field to the one we specify with the max variable, right? However, the way it is implemented does not provide what I expect. I expect the following:
- If the magnitude of the velocity of a cell is higher the one we specified, change the velocity in a way to get the magnitude value we specified (max).
Right now it is as follow: If the magSqr of the velocity is larger than the sqr of max, we modify the velocity (reduce it) by the ratio of sqr(max)/magSqr(Ui). If we do so, the magnitude of the new vector is much smaller than the one we specified with the max keyword (example below). In addition, the actual implementation reduces the magnitude of the vector even more, the higher its magnitude is. But reducing to the one we specified will never happen.
If I got it right, we should do it as follow. If the magnitude of the velocity is larger than the one we specified, reduce the velocity in a way that the magnitude of the new vector is equal to the one we specified with the max keyword, right?
If this is correct, then we can use the simple patch I attached. Otherwise I would be happy to understand the actual imeplementation because even with a discussion with my colleague today, we did not get the point.
The example below demonstrate the above text:
|Steps To Reproduce||Example given:|
U = (2 1 4) // of the cell we check out
max = 2;
mag(U) = 4.58258 --> bigger than max so reduce it.
Actual calculation will give us a new velocity vector as:
sqrMax = sqr(2) = 4
magSqrUi = magSqr(U) = 21
Unew = U * sqrMax / magSqrUi = 4 / 21 = (0.190476 0.0952381 0.380952)
However, the magnitude of the new vector is:
mag(Unew) = 0.436436 (I expect it to be 2)
Patched version calculates the new velocity vector as:
max = 2
magUi = mag(U) = 4.58258
Unew = U * max / magUi = (0.872872 0.436436 1.74574)
Now we get the magnitude of the new vector exact to be 2 (as we specified with the max keyword)
mag(Unew) = 2
limitVelocityPatch.C (3,254 bytes)
Thanks for the report
Fixed in dev by commit 16b559c1093019a4a85aa8c810c9d837dfa8fa4c
Fixed in 5.x by commit 3ece3e9e81ef05c8336bb7fa5e9a8e0bd78aa6b2
Just one remark to the patch. Taking the sqrt() of the prefaktor is correct but we could reduce the call of functions as given in my patch :)
Well I know, this will not influence the speed of the code too much and is tweak in the micro level :)
By the way, you are welcomed Will.
Nice to see the patch already there.
||Your patch calls mag (which contains sqrt) on every velocity value. My fix only calls sqrt for the velocity values that are larger than the maximum. Sqrt is the expensive bit, so mine will be significantly quicker.|
Oh yes you are right :)
Thanks Will for the note. Then everything is fine.
|2017-07-26 23:21||Shorty||New Issue|
|2017-07-26 23:21||Shorty||File Added: limitVelocityPatch.C|
|2017-07-26 23:21||Shorty||Tag Attached: limitVelocity|
|2017-07-27 15:21||will||Assigned To||=> will|
|2017-07-27 15:21||will||Status||new => resolved|
|2017-07-27 15:21||will||Resolution||open => fixed|
|2017-07-27 15:21||will||Note Added: 0008463|
|2017-07-27 16:04||Shorty||Status||resolved => feedback|
|2017-07-27 16:04||Shorty||Resolution||fixed => reopened|
|2017-07-27 16:04||Shorty||Note Added: 0008465|
|2017-07-27 16:11||will||Status||feedback => resolved|
|2017-07-27 16:11||will||Resolution||reopened => fixed|
|2017-07-27 16:11||will||Note Added: 0008466|
|2017-07-27 16:54||Shorty||Status||resolved => feedback|
|2017-07-27 16:54||Shorty||Resolution||fixed => reopened|
|2017-07-27 16:54||Shorty||Note Added: 0008468|
|2017-07-27 16:57||will||Status||feedback => resolved|
|2017-07-27 16:57||will||Resolution||reopened => fixed|