Fix underrelaxation #1057
Fix underrelaxation #1057
Conversation
jayantmukho
left a comment
jayantmukho
left a comment
There was a problem hiding this comment.
Simple bug-fix. Thanks for finding it!
Do you mean user-specified UR instead / on top of the automatic one? I thought the clipping was necessary to get physical solutions (at least the lower bounds). |
I just meant the automatic one. Maybe I need to read up more on k-omega but I thought the solution just had to be positive to be physical, and an allowable ratio < 1 should prevent any negative solutions, assuming the initial solution is positive. But for now, I think we should probably just get this bug fix in and I can look into the solution clipping separately. |
| /* Choose the minimum factor between mean flow and turbulence. */ | ||
|
|
||
| localUnderRelaxation = min(localUnderRelaxation, solver_container[FLOW_SOL]->GetNodes()->GetUnderRelaxation(iPoint)); |
There was a problem hiding this comment.
I guess some numerical issues might occur if numbers get too large/small during convergence, although that sounds more like a single precision consideration.
Is it worth keeping just this min() between flow and turbulence for now? And maybe investigate if it makes sense as part of #1036?
There was a problem hiding this comment.
Fair enough. I'll add it back in for now since it shouldn't hurt anything
| su2double localUnderRelaxation = 1.00; | ||
| const su2double allowableRatio = 0.99; |
There was a problem hiding this comment.
Yep, just fyi, the original intent was to allow it to increase and decrease by separate percentages. For S-A, for instance, we often have very large increases at the start of the solve, but only very small decreases, so one may want to allow for a much larger percent increase but limit the decrease to a smaller percentage.
As for swapping out the clipping in SST for something related to under-relaxation thresholds, it could be a good idea. Worth some testing and prob a follow up PR. As you have probably seen, we are only applying the under-relaxation to some S-A variants.. I played around a little with under-relaxation with SST, but I did not have time to experiment properly (and the results seemed very sensitive to it). I do not believe the SST articles in the literature have much to say about clipping. Actually, we had been clipping S-A unnecessarily before putting in the under-relaxation and CFL adaption, which we removed, so it would be good to experiment with SST as well.
Proposed Changes
There was a bug in the CTurbSolver::ComputeUnderRelaxation() that would return a negative UR if the change in solution was too negative (less than the allowable decrease), which could cause the solution to head in the wrong direction. I'm not really sure why the turbulent solver version of this function was written differently than the flow solver (maybe the intention was to have a different allowable increase and decrease but they ended up being the same in the end?), but I changed it to make it consistent with the flow solver version.
Also, as I mentioned in #1036, the UR parameters for the flow solver and turbulent solver both took the min of each other and were never reset for steady, meaning it was non-increasing.
While on the topic of UR, I was wondering what the community's thoughts are on changing the SST models to use UR instead of solution clipping. The limits seem somewhat arbitrary to me (is there a ref?), and the tests I've done on my mesh adaptation cases have performed well where I've replaced clipping with UR. I realize this is a very limited dataset so if anyone has any strong opinions of why we should be clipping, let me know.
PR Checklist
Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.