Skip to content

Fix axi auxvargrad #1192

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.

Sign up for GitHub

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

Merged
FlorianDm merged 5 commits into from
Feb 9, 2021
Merged

Fix axi auxvargrad #1192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c608706
change auxvargrad contribution in energy equation (correctness to be …
FlorianDm Jan 12, 2021
d6da267
Merge branch 'develop' into fix_axi_auxvargrad
FlorianDm Jan 12, 2021
e35e7e8
Merge branch 'develop' into fix_axi_auxvargrad
FlorianDm Feb 6, 2021
e0b30cc
fix bug implicit conversion with auto (reference)
FlorianDm Feb 7, 2021
a20ac10
Merge branch 'develop' into fix_axi_auxvargrad
FlorianDm Feb 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions SU2_CFD/src/numerics/flow/flow_sources.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,14 @@ void CSourceAxisymmetric_Flow::ResidualDiffusion(){

residual[0] -= 0.0;
residual[1] -= Volume*(yinv*total_viscosity_i*(PrimVar_Grad_i[1][1]+PrimVar_Grad_i[2][0])
-TWO3*AuxVar_Grad_i[0][0]);
-TWO3*AuxVar_Grad_i[0][0]);
residual[2] -= Volume*(yinv*total_viscosity_i*2*(PrimVar_Grad_i[2][1]-v*yinv)
-TWO3*AuxVar_Grad_i[0][1]);
-TWO3*AuxVar_Grad_i[0][1]);
residual[3] -= Volume*(yinv*(total_viscosity_i*(u*(PrimVar_Grad_i[2][0]+PrimVar_Grad_i[1][1])
+v*TWO3*(2*PrimVar_Grad_i[1][1]-PrimVar_Grad_i[1][0]
-v*yinv+U_i[0]*turb_ke_i))
-total_conductivity_i*PrimVar_Grad_i[0][1])
-TWO3*(AuxVar_Grad_i[1][1]+AuxVar_Grad_i[2][1]));
+v*TWO3*(2*PrimVar_Grad_i[1][1]-PrimVar_Grad_i[1][0]
-v*yinv+U_i[0]*turb_ke_i))
-total_conductivity_i*PrimVar_Grad_i[0][1])
-TWO3*(AuxVar_Grad_i[1][1]+AuxVar_Grad_i[2][0]));
}


Expand Down
8 changes: 4 additions & 4 deletions SU2_CFD/src/solvers/CSolver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2456,8 +2456,8 @@ void CSolver::SetRotatingFrame_GCL(CGeometry *geometry, const CConfig *config) {

void CSolver::SetAuxVar_Gradient_GG(CGeometry *geometry, const CConfig *config) {

const auto solution = base_nodes->GetAuxVar();
auto gradient = base_nodes->GetAuxVarGradient();
const auto& solution = base_nodes->GetAuxVar();
auto& gradient = base_nodes->GetAuxVarGradient();
Comment on lines -2459 to +2460

@pcarruscag pcarruscag Feb 8, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed the not so auto thing about auto...

@pcarruscag pcarruscag Feb 8, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there no regression for axisymmetric flow? One would think something like this could break something.

@FlorianDm FlorianDm Feb 8, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is but it did just set them to zero and we don't divide by them or anything so nothing broke just inaccurate results and perhaps not converging when it should in some cases. No compiler error or warning so i guess with auto one needs to be careful

@pcarruscag pcarruscag Feb 8, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a regression that "exercises" this part of the implementation? Perhaps just modifying an existing case?

@WallyMaier WallyMaier Feb 8, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FlorianDm @pcarruscag believe there was another issue within NEMO axisymmetric that @jtneedels fixed. Maybe it would be a good time to merge that in as well?

@jtneedels jtneedels Feb 9, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FlorianDm Yes I agree with these changes, thanks for catching that! @pcarruscag Yes, as @WallyMaier mentioned I do have another small fix to pull in for axisymmetric in CNEMOEulerSolver, should I go ahead ad open a separate PR to merge that in?

@FlorianDm FlorianDm Feb 9, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try to create a test/regression case after adding the k-omega source terms (almost done) because I think it is more useful if it is a rans case


computeGradientsGreenGauss(this, AUXVAR_GRADIENT, PERIODIC_NONE, *geometry,
*config, solution, 0, base_nodes->GetnAuxVar(), gradient);
Expand All @@ -2466,8 +2466,8 @@ void CSolver::SetAuxVar_Gradient_GG(CGeometry *geometry, const CConfig *config)
void CSolver::SetAuxVar_Gradient_LS(CGeometry *geometry, const CConfig *config) {

bool weighted = true;
const auto solution = base_nodes->GetAuxVar();
auto gradient = base_nodes->GetAuxVarGradient();
const auto& solution = base_nodes->GetAuxVar();
auto& gradient = base_nodes->GetAuxVarGradient();
auto& rmatrix = base_nodes->GetRmatrix();

computeGradientsLeastSquares(this, AUXVAR_GRADIENT, PERIODIC_NONE, *geometry, *config,
Expand Down