-
Notifications
You must be signed in to change notification settings - Fork 847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Newton-Raphson for NEMO temperature computations #1627
Changes from all commits
3115641
8d7762f
9cce43a
274c34e
8368b55
faab298
6537a36
837bf4f
6ba0643
c2e6445
9e7dc31
9c0b4fe
ec73ef8
3588426
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1986,9 +1986,8 @@ void CSU2TCLib::ThermalConductivitiesGY(){ | |
|
||
} | ||
|
||
vector<su2double>& CSU2TCLib::ComputeTemperatures(vector<su2double>& val_rhos, su2double rhoE, su2double rhoEve, su2double rhoEvel){ | ||
vector<su2double>& CSU2TCLib::ComputeTemperatures(vector<su2double>& val_rhos, su2double rhoE, su2double rhoEve, su2double rhoEvel, su2double Tve_old) { | ||
|
||
vector<su2double> val_eves; | ||
rhos = val_rhos; | ||
|
||
/*----------Translational temperature----------*/ | ||
|
@@ -2004,39 +2003,77 @@ vector<su2double>& CSU2TCLib::ComputeTemperatures(vector<su2double>& val_rhos, s | |
T = (rhoE - rhoEve - rhoE_f + rhoE_ref - rhoEvel) / rhoCvtr; | ||
|
||
/*--- Set temperature clipping values ---*/ | ||
su2double Tmin = 50.0; su2double Tmax = 8E4; | ||
su2double Tve_o = 50.0; su2double Tve2 = 8E4; | ||
const su2double Tmin = 50.0; const su2double Tmax = 8E4; | ||
const su2double Tvemin = 50.0; const su2double Tvemax = 8E4; | ||
su2double Tve_o = 50.0; su2double Tve2 = 8E4; | ||
|
||
/* Determine if the temperature lies within the acceptable range */ | ||
if (T < Tmin) T = Tmin; | ||
else if (T > Tmax) T = Tmax; | ||
if (Tve_old < 1) Tve_old = T; //For first fluid iteration | ||
if (T < Tmin) T = Tmin; else if (T > Tmax) T = Tmax; | ||
if (Tve_old<Tvemin) Tve_old = Tvemin; else if (Tve_old>Tvemax) Tve_old = Tvemax; | ||
|
||
/*--- Set vibrational temperature algorithm parameters ---*/ | ||
su2double Btol = 1.0E-6; // Tolerance for the Bisection method | ||
unsigned short maxBIter = 50; // Maximum Bisection method iterations | ||
const su2double NRtol = 1.0E-6; // Tolerance for the Newton-Raphson method | ||
const su2double Btol = 1.0E-6; // Tolerance for the Bisection method | ||
const unsigned short maxBIter = 50; // Maximum Bisection method iterations | ||
const unsigned short maxNIter = 50; // Maximum Newton-Raphson iterations | ||
const su2double scale = 0.9; // Scaling factor for Newton-Raphson step | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have a sense of the stability of the method wrt to learning rate? 0.9 ~ 1, seems like you could get away without this parameter. Are there cases for which the method fails to converge for large learning rates, but converges for small ones? Would it make sense to have this be an adaptive parameter? (i.e. scale = 1, if NR does not converge, scale = 0.2, break?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From my limited test cases, I haven't seen any issue with stability. I think I can test once I find a case where we seen temperature non-convergence. I think having the parameter adaptive could be interesting, though may not be necessary if 0.9 (or 1 is sufficient). |
||
|
||
/*--- Execute a Newton-Raphson root-finding method for Tve ---*/ | ||
//Initialize solution | ||
Tve = T; | ||
Tve = Tve_old; | ||
|
||
// Execute the root-finding method | ||
bool Bconvg = false; | ||
su2double rhoEve_t = 0.0; | ||
|
||
for (unsigned short iIter = 0; iIter < maxBIter; iIter++) { | ||
Tve = (Tve_o+Tve2)/2.0; | ||
val_eves = ComputeSpeciesEve(Tve); | ||
rhoEve_t = 0.0; | ||
for (iSpecies = 0; iSpecies < nSpecies; iSpecies++) rhoEve_t += rhos[iSpecies] * val_eves[iSpecies]; | ||
if (fabs(rhoEve_t - rhoEve) < Btol) { | ||
Bconvg = true; | ||
break; | ||
bool NRconvg = false; | ||
su2double rhoEve_t = 0.0, rhoCvve = 0.0; | ||
|
||
/*--- Newton-Raphson Method --*/ | ||
for (unsigned short iIter = 0; iIter < maxNIter; iIter++) { | ||
rhoEve_t = rhoCvve = 0.0; | ||
const auto& val_eves = ComputeSpeciesEve(Tve); | ||
const auto& val_cvves = ComputeSpeciesCvVibEle(Tve); | ||
|
||
for (iSpecies = 0; iSpecies < nSpecies; iSpecies++){ | ||
rhoEve_t += rhos[iSpecies] * val_eves[iSpecies]; | ||
rhoCvve += rhos[iSpecies] * val_cvves[iSpecies]; | ||
} | ||
|
||
/*--- Find the roots ---*/ | ||
su2double f = rhoEve - rhoEve_t; | ||
su2double df = -rhoCvve; | ||
Tve2 = Tve - (f/df)*scale; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the question on learning rate above, do we know any properties about the minimization problem we are solving here that help us guarantee convergence, or maybe can help us demonstrate robustness? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it a backtracking method like e.g. the Armijo rule used for gradient descent/line search what you have in mind? Such a thing could at least prevent you from divergence. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree we could have a common folder of these methods, though Im not sure where else in the code they are used. |
||
|
||
/*--- Check for convergence ---*/ | ||
if ((fabs(Tve2-Tve) < NRtol) && (Tve > Tvemin) && (Tve < Tvemax)) { | ||
NRconvg = true; | ||
Tve = Tve2; | ||
break; | ||
} else { | ||
if (rhoEve_t > rhoEve) Tve2 = Tve; | ||
else Tve_o = Tve; | ||
Tve = Tve2; | ||
} | ||
} | ||
|
||
// If the Newton-Raphson method has converged, assign the value of Tve. | ||
// Otherwise, execute a bisection root-finding method | ||
Tve_o = Tvemin; Tve2 = Tvemax; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is having the bisection as a backup still necessary? Is this how it was implemented in feature_TNE2? Have any of the test cases you tried required the use of the bisection to solve for value? Are there specific circumstances where bisection will work where NR will fail? The only examples I can think of are bad initialization or if there is some weirdness/non-smoothness in the function. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was how the original TNE2 was implemented, and generally doesn't seem to get activated except the first iteration. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So at the first iteration, NR fails and it switches to bisection, then it switches back to NR? That sounds like a problematic initialization. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, the issue is the first iteration where the primitive vector isn't set ie: So the two option in my head would be set the Primitive temperature early in the process, or have a boolean check. The boolean check works fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added the boolean above and there hasn't been a case where the bisection was needed. |
||
if (!NRconvg) { | ||
for (unsigned short iIter = 0; iIter < maxBIter; iIter++) { | ||
Tve = (Tve_o+Tve2)/2.0; | ||
const auto& val_eves = ComputeSpeciesEve(Tve); | ||
rhoEve_t = 0.0; | ||
for (iSpecies = 0; iSpecies < nSpecies; iSpecies++) rhoEve_t += rhos[iSpecies] * val_eves[iSpecies]; | ||
if (fabs(rhoEve_t - rhoEve) < Btol) { | ||
Bconvg = true; | ||
break; | ||
} else { | ||
if (rhoEve_t > rhoEve) Tve2 = Tve; | ||
else Tve_o = Tve; | ||
} | ||
} | ||
} | ||
|
||
// If absolutely no convergence, then assign to the TR temperature | ||
if (!Bconvg) { | ||
if (!NRconvg && !Bconvg ) { | ||
Tve = T; | ||
cout <<"Warning: temperatures did not converge, error= "<< fabs(rhoEve_t-rhoEve)<<endl; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you tried this on any cases where we previously didn't get convergence? Does it help with that, or does it just speed up the solver? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a case with the Fire2 waiting to be run, I will report back on that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new temperature routine avoid the "did not converge" errors, but did not improve the solution quality. |
||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During the first fluid iteration, the Tve_old is zero. This causes the newton solver to fail.
This line fixes that issue, and seemed to be least intrusive fix, though open to other ideas.