Make primal residual for adjoint like normal screen output. by TobiKattmann · Pull Request #1371 · su2code/SU2

TobiKattmann · 2021-09-14T11:56:54Z

Hi all,

lately I was fixing restarts. Making sure the adjoint also has a correct primal restart is somewhat annoying due to the way the residuals are printed to screen

I want to change that to look like a normal screen output of all residuals. Here the old variant on top and the new below (field names need to be adapted to usual screen output)

The code as of today only changes this for single zone and does not use the usual output routines (and is a bit hacky). I want to add multizone and use the output routines instead of this handwritten PrintDirectResidual routine. I'll look into both

Let me know if there are reasons against this or any other comments

Related Work

The printing tool box of Tim

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.

So far only for SingleZoneDriver. Using the PrintingToolbox. Multizone should be adapted as well. Prob it is best to use the normal output routines but this is just a first step.

SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp

pcarruscag

👍 I think with some tweaks it would be no worse code-wise and much better for functionality.

SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp

Essentially addressing review comments. Still, only SingleZoneDriver was changed.

SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp

Co-authored-by: Pedro Gomes <[email protected]>

Similar to the SingleZone version this also prints the direct residuals using the Table Printer. And now all residuals instead of just the first of each solver.

TobiKattmann · 2021-09-15T11:50:36Z

SU2_CFD/src/drivers/CDiscAdjMultizoneDriver.cpp

                                                         config_container, iZone, INST_0);
      AD::Push_TapePosition(); /// leave_zone
    }
+    Print_DirectResidual(kind_recording);


I made that bit a function for Multizone just as there is for SingleZone. Using the same function for both is maybe possible without to much effort. But I did not do that for now

Did that, see latest commit af7d12c

TobiKattmann · 2021-09-15T11:51:29Z

SU2_CFD/src/drivers/CDiscAdjMultizoneDriver.cpp

+  /*--- Helper lambda func to return lenghty [iVar][iZone] string.  ---*/
+  auto iVar_iZone2string = [](unsigned short ivar, unsigned short izone) {
+    return "[" + std::to_string(ivar) + "][" + std::to_string(izone) + "]";
+  };


Here one would need to have the iZone part only for GetMultizoneProblem()

TobiKattmann · 2021-09-15T11:53:14Z

SU2_CFD/src/drivers/CDiscAdjMultizoneDriver.cpp

-      case DISC_ADJ_INC_EULER: case DISC_ADJ_INC_NAVIER_STOKES:
-        cout << " Zone " << iZone << " (flow)       - log10[U(0)]    : "
-             << log10(solvers[FLOW_SOL]->GetRes_RMS(0)) << endl;
-        if (config_container[iZone]->AddRadiation()) {
+      for (unsigned short iZone = 0; iZone < nZone; iZone++) {

-          cout << " Zone " << iZone << " (radiation)  - log10[Rad(0)]  : "
-               << log10(solvers[RAD_SOL]->GetRes_RMS(0)) << endl;
-        }
-        break;
+        auto solvers = solver_container[iZone][INST_0][MESH_0];
+        auto configs = config_container[iZone];


I am not sure whether one can easily swap those containers for singlezone and Multizone on this level

Well I guess this works out as is ...

SU2_CFD/src/drivers/CDiscAdjMultizoneDriver.cpp

TobiKattmann · 2021-09-15T11:56:22Z

SU2_CFD/src/drivers/CDiscAdjMultizoneDriver.cpp

-      case DISC_ADJ_EULER:     case DISC_ADJ_NAVIER_STOKES:
-      case DISC_ADJ_INC_EULER: case DISC_ADJ_INC_NAVIER_STOKES:
-        cout << " Zone " << iZone << " (flow)       - log10[U(0)]    : "
-             << log10(solvers[FLOW_SOL]->GetRes_RMS(0)) << endl;
-        if (config_container[iZone]->AddRadiation()) {
+      for (unsigned short iZone = 0; iZone < nZone; iZone++) {

-          cout << " Zone " << iZone << " (radiation)  - log10[Rad(0)]  : "
-               << log10(solvers[RAD_SOL]->GetRes_RMS(0)) << endl;


I am not sure why there is this separation of the laminar and turbulence cases, i.e. having the same stuff for flow and radiation twice. So did that together like in the singlezone

SU2_CFD/src/drivers/CDiscAdjMultizoneDriver.cpp

TobiKattmann · 2021-09-15T12:04:59Z

SU2_CFD/src/drivers/CDiscAdjMultizoneDriver.cpp

  if (rank == MASTER_NODE && kind_recording == RECORDING::SOLUTION_VARIABLES) {

-    auto solvers = solver_container[iZone][INST_0][MESH_0];
+    const unsigned short fieldWidth = 15;


Unfortunately I had to increase to a min of 14 to fit e.g. rms_Flow[0][0] into the table. The fixed screen output size for the primal is currently 12... that is not ideal but better than before imo

Namely the Single and Multizone version are now consolidated in CDriver which has to use minor specific things to accomodate for both. The function is protected.

SU2_CFD/src/drivers/CDriver.cpp

SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp

SU2_CFD/src/drivers/CDriver.cpp

Co-authored-by: Pedro Gomes <[email protected]>

It might be double (only the screen output) for multizone for the secondaries.

…to feature_directres

And not in PrintDirectRes. This is consistent with the multizone driver.

TobiKattmann · 2021-09-15T15:28:55Z

SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp

+  if (rank == MASTER_NODE) {
+    cout << "\n-------------------------------------------------------------------------\n";
+    switch(kind_recording) {
+    case RECORDING::CLEAR_INDICES: cout << "Clearing the computational graph." << endl; break;
+    case RECORDING::MESH_COORDS:   cout << "Storing computational graph wrt MESH COORDINATES." << endl; break;
+    case RECORDING::SOLUTION_VARIABLES:
+      cout << "Direct iteration to store the primal computational graph." << endl;
+      cout << "Compute residuals to check the convergence of the direct problem." << endl; break;
+    default: break;
+    }
+  }
+


This is now consistent/similar with the handling in the CDiscAdjMultizoneDriver. This allows the early return in Print_DirectResiduals

TobiKattmann · 2021-09-15T15:29:30Z

SU2_CFD/src/drivers/CDriver.cpp

+void CDriver::Print_DirectResidual(RECORDING kind_recording) {
+
+  if (!(rank == MASTER_NODE && kind_recording == RECORDING::SOLUTION_VARIABLES)) return;


There it is

SU2_CFD/src/drivers/CDriver.cpp

SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp

TobiKattmann · 2021-09-16T07:24:30Z

Thanks for the review and the helpful comments @pcarruscag .

Make direct res for adjoint like normal screen output.

a0ff626

So far only for SingleZoneDriver. Using the PrintingToolbox. Multizone should be adapted as well. Prob it is best to use the normal output routines but this is just a first step.

TobiKattmann added the changelog:chore label Sep 14, 2021

pr-triage bot added the PR: unreviewed label Sep 14, 2021

TobiKattmann marked this pull request as draft September 14, 2021 11:57

TobiKattmann changed the title ~~Make primal residual for adjoint like normal screen output.~~ [WIP] Make primal residual for adjoint like normal screen output. Sep 14, 2021

pr-triage bot added PR: draft and removed PR: unreviewed labels Sep 14, 2021

TobiKattmann commented Sep 14, 2021

View reviewed changes

SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp Outdated Show resolved Hide resolved

pcarruscag reviewed Sep 14, 2021

View reviewed changes

SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp Outdated Show resolved Hide resolved

SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp Outdated Show resolved Hide resolved

Loop over nVar for flow and turb. Use bulk-KindSolver functions.

1d8c236

Essentially addressing review comments. Still, only SingleZoneDriver was changed.

TobiKattmann commented Sep 15, 2021

View reviewed changes

SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp Outdated Show resolved Hide resolved

pcarruscag reviewed Sep 15, 2021

View reviewed changes

SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp Outdated Show resolved Hide resolved

TobiKattmann and others added 2 commits September 15, 2021 12:42

Use \n line breaks instead of std::endl.

dec128b

Co-authored-by: Pedro Gomes <[email protected]>

Add PrintDirectResidual func for DiscAdjMultizoneDriver.

6af0f3b

Similar to the SingleZone version this also prints the direct residuals using the Table Printer. And now all residuals instead of just the first of each solver.

TobiKattmann commented Sep 15, 2021

View reviewed changes

SU2_CFD/src/drivers/CDiscAdjMultizoneDriver.cpp Outdated Show resolved Hide resolved

MultizoneDriver Res-printing didn't have weakly coupled heat before.

85a1d1b

TobiKattmann commented Sep 15, 2021

View reviewed changes

Bring PrintDirectRes to CDriver level and remove Adj specific ones.

af7d12c

Namely the Single and Multizone version are now consolidated in CDriver which has to use minor specific things to accomodate for both. The function is protected.

TobiKattmann commented Sep 15, 2021

View reviewed changes

SU2_CFD/src/drivers/CDriver.cpp Show resolved Hide resolved

TobiKattmann marked this pull request as ready for review September 15, 2021 13:22

pr-triage bot removed the PR: draft label Sep 15, 2021

TobiKattmann changed the title ~~[WIP] Make primal residual for adjoint like normal screen output.~~ Make primal residual for adjoint like normal screen output. Sep 15, 2021

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

TobiKattmann commented Sep 15, 2021

View reviewed changes

SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp Show resolved Hide resolved

TobiKattmann commented Sep 15, 2021

View reviewed changes

SU2_CFD/src/drivers/CDriver.cpp Outdated Show resolved Hide resolved

TobiKattmann commented Sep 15, 2021

View reviewed changes

SU2_CFD/src/drivers/CDriver.cpp Show resolved Hide resolved

pcarruscag approved these changes Sep 15, 2021

View reviewed changes

SU2_CFD/src/drivers/CDriver.cpp Outdated Show resolved Hide resolved

SU2_CFD/src/drivers/CDriver.cpp Show resolved Hide resolved

SU2_CFD/src/drivers/CDriver.cpp Outdated Show resolved Hide resolved

SU2_CFD/src/drivers/CDriver.cpp Outdated Show resolved Hide resolved

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels Sep 15, 2021

Apply suggestions from code review

b0ba03b

Co-authored-by: Pedro Gomes <[email protected]>

pr-triage bot added PR: unreviewed and removed PR: reviewed-approved labels Sep 15, 2021

TobiKattmann added 3 commits September 15, 2021 16:56

Re-add screen output for secondary-recording for PrintDirectRes.

67008b4

It might be double (only the screen output) for multizone for the secondaries.

Merge branch 'feature_directres' of https://github.com/su2code/SU2 in…

5766359

…to feature_directres

Handle Secondary Variables ScreenOutput in SetRecording.

03c0281

And not in PrintDirectRes. This is consistent with the multizone driver.

TobiKattmann commented Sep 15, 2021

View reviewed changes

SU2_CFD/src/drivers/CDriver.cpp Outdated Show resolved Hide resolved

TobiKattmann commented Sep 15, 2021

View reviewed changes

SU2_CFD/src/drivers/CDriver.cpp Outdated Show resolved Hide resolved

Indentation

bceccb7

pcarruscag reviewed Sep 15, 2021

View reviewed changes

SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp Outdated Show resolved Hide resolved

Update SU2_CFD/src/drivers/CDiscAdjSinglezoneDriver.cpp

6f50158

TobiKattmann merged commit 68e8de1 into develop Sep 16, 2021

TobiKattmann deleted the feature_directres branch September 16, 2021 07:24

pr-triage bot added PR: merged and removed PR: unreviewed labels Sep 16, 2021

		void CDriver::Print_DirectResidual(RECORDING kind_recording) {

		if (!(rank == MASTER_NODE && kind_recording == RECORDING::SOLUTION_VARIABLES)) return;

Conversation

TobiKattmann commented Sep 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Work

PR Checklist

Uh oh!

Uh oh!

pcarruscag left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TobiKattmann commented Sep 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TobiKattmann commented Sep 14, 2021 •

edited

Loading