Home Original page

[WIP] defect correction of Nishikawa for skewed meshes by bigfooted · Pull Request #2584 · su2code/SU2

@bigfooted

@bigfooted

github-advanced-security[bot]

Comment on lines +676 to +687

switch (correct) {
case VISCOUS_GRAD_CORR::EDGE_NORMAL:
diss = proj_vector_ij / max(dist_ij_2,EPS*EPS);
break;
case VISCOUS_GRAD_CORR::FACE_TANGENT:
diss = Area2 / max(proj_vector_ij,EPS);
break;
case VISCOUS_GRAD_CORR::ALPHA_DAMPING:
const su2double alpha = 4.0 / 3.0;
diss = alpha * Area2 / max(abs(proj_vector_ij),EPS);
break;
}

Check warning

Code scanning / CodeQL

Missing enum case in switch Warning

Switch statement does not have a case for

NONE

.

Copilot Autofix

AI 6 months ago

To fix the problem, all enum members of VISCOUS_GRAD_CORR used in the switch statement should be handled explicitly or, alternatively, a default case should be added. The best way to proceed here is to add a case for VISCOUS_GRAD_CORR::NONE to document intent and control the handling for this value. If NONE should have specific behavior, provide the logic; otherwise, you can make it explicit that nothing should be done (e.g., leave diss at 0.0). Adding a comment may further clarify intent.

Make this change directly in the ComputeProjectedGradient function, at the switch statement on correct (lines 676–687).

Suggested changeset 1

SU2_CFD/include/numerics/CNumerics.hpp

Autofix patch

Autofix patch

Run the following command in your local git repository to apply this patch

cat << 'EOF' | git apply
diff --git a/SU2_CFD/include/numerics/CNumerics.hpp b/SU2_CFD/include/numerics/CNumerics.hpp
--- a/SU2_CFD/include/numerics/CNumerics.hpp
+++ b/SU2_CFD/include/numerics/CNumerics.hpp
@@ -681,9 +681,12 @@
         diss = Area2 / max(proj_vector_ij,EPS);
         break;
       case VISCOUS_GRAD_CORR::ALPHA_DAMPING:
-        const su2double alpha = 4.0 / 3.0;        
+        const su2double alpha = 4.0 / 3.0;
         diss = alpha * Area2 / max(abs(proj_vector_ij),EPS);
         break;
+      case VISCOUS_GRAD_CORR::NONE:
+        // No correction applied; diss remains 0.0
+        break;
     }
 
     //proj_vector_ij /= max(dist_ij_2,EPS);
EOF
@@ -681,9 +681,12 @@
diss = Area2 / max(proj_vector_ij,EPS);
break;
case VISCOUS_GRAD_CORR::ALPHA_DAMPING:
const su2double alpha = 4.0 / 3.0;
const su2double alpha = 4.0 / 3.0;
diss = alpha * Area2 / max(abs(proj_vector_ij),EPS);
break;
case VISCOUS_GRAD_CORR::NONE:
// No correction applied; diss remains 0.0
break;
}

//proj_vector_ij /= max(dist_ij_2,EPS);

Copilot is powered by AI and may make mistakes. Always verify output.

break;
}

//proj_vector_ij /= max(dist_ij_2,EPS);

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.

Copilot Autofix

AI 6 months ago

To fix the problem, we should remove the commented-out code on line 689: //proj_vector_ij /= max(dist_ij_2,EPS);. We should not replace it with any alternative code nor attempt to integrate it unless its function is necessary, as there is no indication in the comment or surrounding code that it should be reinstated or has a continuing purpose. The change only involves deleting the line, and no additional definitions, imports, or method changes are required. Only SU2_CFD/include/numerics/CNumerics.hpp, at the flagged location, needs to be updated.

Suggested changeset 1

SU2_CFD/include/numerics/CNumerics.hpp

Autofix patch

Autofix patch

Run the following command in your local git repository to apply this patch

cat << 'EOF' | git apply
diff --git a/SU2_CFD/include/numerics/CNumerics.hpp b/SU2_CFD/include/numerics/CNumerics.hpp
--- a/SU2_CFD/include/numerics/CNumerics.hpp
+++ b/SU2_CFD/include/numerics/CNumerics.hpp
@@ -686,7 +686,6 @@
         break;
     }
 
-    //proj_vector_ij /= max(dist_ij_2,EPS);
 
     /*--- Mean gradient approximation. ---*/
     for (int iVar = 0; iVar < nVar; iVar++) {
EOF
@@ -686,7 +686,6 @@
break;
}

//proj_vector_ij /= max(dist_ij_2,EPS);

/*--- Mean gradient approximation. ---*/
for (int iVar = 0; iVar < nVar; iVar++) {

Copilot is powered by AI and may make mistakes. Always verify output.

const su2double gasConst;
const su2double prandtlTurb;
const bool correct;
//const su2double cp;

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.

Copilot Autofix

AI 6 months ago

The best way to fix this problem is to remove the commented-out code line //const su2double cp; entirely. This avoids introducing confusion for future readers and adheres to clean code principles. No additional imports, method definitions, or code changes are required. Only the specific line in the file needs to be deleted, with surrounding context left unchanged.

Suggested changeset 1

SU2_CFD/include/numerics_simd/flow/diffusion/viscous_fluxes.hpp

Autofix patch

Autofix patch

Run the following command in your local git repository to apply this patch

cat << 'EOF' | git apply
diff --git a/SU2_CFD/include/numerics_simd/flow/diffusion/viscous_fluxes.hpp b/SU2_CFD/include/numerics_simd/flow/diffusion/viscous_fluxes.hpp
--- a/SU2_CFD/include/numerics_simd/flow/diffusion/viscous_fluxes.hpp
+++ b/SU2_CFD/include/numerics_simd/flow/diffusion/viscous_fluxes.hpp
@@ -77,7 +77,6 @@
   const su2double gamma;
   const su2double gasConst;
   const su2double prandtlTurb;
-  //const su2double cp;
   const bool correct_EN;
   const bool correct_FT;
   const bool correct_AD;
EOF
@@ -77,7 +77,6 @@
const su2double gamma;
const su2double gasConst;
const su2double prandtlTurb;
//const su2double cp;
const bool correct_EN;
const bool correct_FT;
const bool correct_AD;

Copilot is powered by AI and may make mistakes. Always verify output.

@tbellosta

Hi Nijso,
Nice of you to open a WIP PR. The branch feature_2ndOrderMixedGrids is still under development. The main addition are reported in the paper you linked and consist in a correction to the convective fluxes in order to recover second order spatial convergence on arbitrary hybrid grids. The branch also deals with boundary conditions which are now computed as a loop over marker faces in such a a way to integrate exactly a linear flux (thus preserving second order convergence, if the surface normals are "good enough"). That reguarding BCs is probably the deeper change to the codebase, and would require some thinking if we plan to eventually merge the feature branch.
Other additions that are reported in the paper (and available in the branch) are:

  • more accurate Roe Jacobians (i.e. we added the jacobian of the dissipation matrix, which was missing), viscous flux Jacobians, and BC jacobians (added the derivative of the external state w.r.t. the internal state in e.g. the farfield BCs). Now the Roe Jacobian should be exact for a first order scheme, and the viscous jacobian is exact as well for zero LSQ gradients.
  • Alternative definition of cell centroid that works well with triangulated BL meshes (would loose 2nd order without the flux correction procedure)
  • Added different corrections for the interface gradient employed in the viscous flux computation i.e. edge-normal (that's whats already there in the code), face-tangent, and alpha-damping.

I will keep you up-to-date in this thread and I will open a more thorough PR when the branch feature_2ndOrderMixedGrids is ready (I have some restructuring/clean-up to do as well as additional tests to run)

@pcarruscag

Hey Tommaso,
Those are all very exciting developments.
Please consider contributing them to the main branch in incremental steps, you have many independent changes in that branch, it would be best practice to test the effects of each of them incrementally. That way, if a bug shows up, it's also a lot easier to track it down.
And if there are questions about the best way of integrating intrusive changes, I would be very happy to help.

@bigfooted

Hi @tbellosta, I started this PR to get things rolling again, because development seemed to have stalled and I think these are great developments that can really push the accuracy and robustness forward.
I think your developments might also be a solution to problems like discussed here currently:
#2576

To make it easier to get these developments into develop, I suggest to separate the 4(?) main ideas: this defect correction, the flux correction, the control volume computation and the modified Roe scheme as was also mentioned by Pedro.