Fix grid velocities in dual-time RANS simulations with deforming grids (aeroelastic and legacy FSI) #1199

pcarruscag · 2021-02-15T12:07:27Z

Proposed Changes

Found while debugging differences between legacy and new mesh deformation with @Nicola-Fonzi in #1197.
Explained below.

Related Work

#1197

PR Checklist

I am submitting my contribution to the develop branch.
My contribution generates no new compiler warnings (try with the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags, or simply --warnlevel=2 when using meson).
My contribution is commented and consistent with SU2 style.
I have added a test case that demonstrates my contribution, if necessary.
I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp) , if necessary.

pcarruscag · 2021-02-15T12:11:48Z

SU2_CFD/src/integration/CIntegration.cpp

+void CIntegration::SetDualTime_Solver(const CGeometry *geometry, CSolver *solver, const CConfig *config, unsigned short iMesh) {
+
+  SU2_OMP_PARALLEL
+  {
+  /*--- Store old solution, volumes and coordinates (in case there is grid movement). ---*/
+
+  solver->GetNodes()->Set_Solution_time_n1();
+  solver->GetNodes()->Set_Solution_time_n();


This function would push the coordinates and volumes back in time.
This function is called 2 times with the same geometry in RANS simulations.
Grid velocities would then be ~1.5 times what they should be.
This was not a problem with the new mesh solver, but the volumes would also be incorrect.

Just saw I need to update the comment... FML

pcarruscag · 2021-02-15T12:12:14Z

SU2_CFD/src/integration/CIntegration.cpp

-void CIntegration::SetDualTime_Solver(CGeometry *geometry, CSolver *solver, CConfig *config, unsigned short iMesh) {
+void CIntegration::SetDualTime_Geometry(CGeometry *geometry, CSolver *mesh_solver, const CConfig *config, unsigned short iMesh) {


Now there is a specific function for dual time geometry updates.

pcarruscag · 2021-02-15T12:14:23Z

SU2_CFD/src/iteration/CFluidIteration.cpp

-    if (config[val_iZone]->GetDeform_Mesh()) {
-      solver[val_iZone][val_iInst][MESH_0][MESH_SOL]->SetDualTime_Mesh();
-    }
+    SetDualTime_Aeroelastic(config[val_iZone], iMesh);


The aeroelastic feature was also broken since it has its own dual time update which was also repeated for RANS

pcarruscag · 2021-02-15T12:16:01Z

SU2_CFD/src/iteration/CFluidIteration.cpp

    }
+
+    /*--- Update dual time solver for the weakly coupled energy equation ---*/
+
+    if (config[val_iZone]->GetWeakly_Coupled_Heat()) {
+      integration[val_iZone][val_iInst][HEAT_SOL]->SetDualTime_Solver(geometry[val_iZone][val_iInst][MESH_0],
+                                                                      solver[val_iZone][val_iInst][MESH_0][HEAT_SOL],
+                                                                      config[val_iZone], MESH_0);
+      integration[val_iZone][val_iInst][HEAT_SOL]->SetConvergence(false);
+    }


@TobiKattmann @oleburghardt , I think this was missing for dual time weakly coupled stuff, can you see if it makes sense?

LGTM. Although I didn't use the WEAKLY_COUPLED_HEAT in a long time tbh and for now do not intend to use in the future.
I also checked: There is a Testcase discadj_heat using WEAKLY_COUPLED_HEAT (although it is of course not unsteady)

Thanks for actualy thinking of that 👍

Alright, thanks for having a look. I don't blame you, there is not a lot going for weakly coupled if the flow solver still drags temperature around. Otherwise having 4 variables (in 3D) would make computers very happy.

Ok i dont get the last sentence :) Do you mean the unnecessary energy-equation that is carried around even for cases without it being active. Or did I miss the memo that states 4>>5 for #equations. Please elaborate if you have a few moments to spare ✍️

Yes that too since it makes the linear system 1.44 times larger and slower (cost of sparse operations is proportional to amount of memory used).
Then 4 is a really good number because all the blocks in the matrix would be aligned and all the operations would pack perfectly into AVX registers.
The vector blocks would also be aligned which would make prefetching (loading into cache ahead of time) effective because it operates on cache lines (and 8 doubles in one of those).
Finally the output vector could be written to with non-temporal stores (algo because of alignment) which improves caching of the input vector.
So in all these ways 4 > 5 (by about 5-10% maybe on top of the 1.44).

Oh yes sure, if forgot that computers had this thing going with powers of 2 🤦 thanks!

Nicola-Fonzi · 2021-02-15T13:55:11Z

Great fix! The code is also much better organised now.

Nicola-Fonzi · 2021-02-15T15:59:16Z

The error was spotted while updating the old python routines to the new drivers/mesh solver. In particular, the test case in py_wrapper folder, related to a flat plate plunging up and down, showed a different coefficient of lift as computed using the old and new mesh solver.

I attach here the comparison of the coefficient of lift history for three cases:

The legacy mesh solver, after the modification in this PR
The new mesh solver
The legacy mesh solver, before the modification

The first two are completely coincident

TobiKattmann

Thanks for fixing that. Good thing to split up geometry and solver " solution pushing" and making use of inheritance for the former and specialisation if needed for the latter.
And the constification of the code is a good thing as well 💐

TobiKattmann · 2021-02-17T14:12:45Z

SU2_CFD/src/iteration/CFluidIteration.cpp

    }
+
+    /*--- Update dual time solver for the weakly coupled energy equation ---*/
+
+    if (config[val_iZone]->GetWeakly_Coupled_Heat()) {
+      integration[val_iZone][val_iInst][HEAT_SOL]->SetDualTime_Solver(geometry[val_iZone][val_iInst][MESH_0],
+                                                                      solver[val_iZone][val_iInst][MESH_0][HEAT_SOL],
+                                                                      config[val_iZone], MESH_0);
+      integration[val_iZone][val_iInst][HEAT_SOL]->SetConvergence(false);
+    }


LGTM. Although I didn't use the WEAKLY_COUPLED_HEAT in a long time tbh and for now do not intend to use in the future.
I also checked: There is a Testcase discadj_heat using WEAKLY_COUPLED_HEAT (although it is of course not unsteady)

Thanks for actualy thinking of that 👍

pcarruscag added 2 commits February 15, 2021 11:50

geometry was modified twice for RANS problems in SetDualTime_Solver

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

25be22d

update dual time weakly coupled heat

c047306

pcarruscag added the changelog:fix label Feb 15, 2021

pr-triage bot added the PR: unreviewed label Feb 15, 2021

pcarruscag commented Feb 15, 2021

View reviewed changes

pcarruscag added 2 commits February 15, 2021 14:51

small fix for aeroelastic, update rigid motion regressions

3e75f50

forgot to save

d9db4a0

TobiKattmann approved these changes Feb 17, 2021

View reviewed changes

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels Feb 17, 2021

Merge branch 'develop' into fix_legacy_deformation

f32e453

pr-triage bot added PR: unreviewed and removed PR: reviewed-approved labels Feb 17, 2021

Nicola-Fonzi merged commit f7207ac into develop Feb 17, 2021

Nicola-Fonzi deleted the fix_legacy_deformation branch February 17, 2021 15:01

pr-triage bot added PR: merged PR: unreviewed and removed PR: unreviewed PR: merged labels Feb 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix grid velocities in dual-time RANS simulations with deforming grids (aeroelastic and legacy FSI) #1199

Fix grid velocities in dual-time RANS simulations with deforming grids (aeroelastic and legacy FSI) #1199

pcarruscag commented Feb 15, 2021

pcarruscag Feb 15, 2021

pcarruscag Feb 15, 2021

pcarruscag Feb 15, 2021

pcarruscag Feb 15, 2021

TobiKattmann Feb 17, 2021

pcarruscag Feb 18, 2021

TobiKattmann Feb 18, 2021

pcarruscag Feb 18, 2021

TobiKattmann Feb 18, 2021

Nicola-Fonzi commented Feb 15, 2021

Nicola-Fonzi commented Feb 15, 2021

TobiKattmann left a comment

TobiKattmann Feb 17, 2021

		void CIntegration::SetDualTime_Solver(CGeometry geometry, CSolver solver, CConfig *config, unsigned short iMesh) {
		void CIntegration::SetDualTime_Geometry(CGeometry geometry, CSolver mesh_solver, const CConfig *config, unsigned short iMesh) {

Fix grid velocities in dual-time RANS simulations with deforming grids (aeroelastic and legacy FSI) #1199

Fix grid velocities in dual-time RANS simulations with deforming grids (aeroelastic and legacy FSI) #1199

Conversation

pcarruscag commented Feb 15, 2021

Proposed Changes

Related Work

PR Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nicola-Fonzi commented Feb 15, 2021

Nicola-Fonzi commented Feb 15, 2021

TobiKattmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment