Fix restart logic in python FSI #1295

Nicola-Fonzi · 2021-06-02T11:08:32Z

Dear all,

This PR resolves an issue with the updated restart logic. Further, it also provides a couple of modifications which will be required in a future PR aimed at the automatic identification of the aerodynamic system.

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.

* new function inside existing sst source term class (no new class since never needed alone) * implement function to compute residual * update authors * one more duplicate * fix typo k2 not k1 * fix error * add production * Update AUTHORS.md * update authors - alphabetical, duplication * first attempt complete * add diffusion terms to jacobian * small change * Corrections based on PR comments * cosmetics * remove else * add AD::SetPreaccIn * cleanup, try to make vertex tractions more general * Removing old setInitialMesh * Compute the interface mapping starting from the undeformed mesh * Loads computed for all the solid markers * Tentative modification of test case * Introduced start time for forced motion * Final version of new test case * Updated values in regression * Complete reorganisation of the interface * Fixes to vector data * minor changes and correction * Better initialisation of variables * Small typo in parallel regression * comment out jacobian to conserve diagonal dominance * spaces and comments * error * geometry was modified twice for RANS problems in SetDualTime_Solver * update dual time weakly coupled heat * small fix for aeroelastic, update rigid motion regressions * forgot to save * Fixed regression values after PR#1199 * array deleted twice * Multiple superposed forced motions * Error with array dimension * add test case and regression list entry * remove jacobian contribution and test case config file options * Introduced condition on mesh boundary * Update SU2_CFD/src/solvers/CMeshSolver.cpp * fix_typoLinael * constification, remove legacy python FSI * fix su2code#1202 * more const Co-authored-by: emanresu <florian.dittmann@protonmail.com> Co-authored-by: Pedro Gomes <pcarruscag@gmail.com> Co-authored-by: Florian <55834287+FlorianDm@users.noreply.github.com> Co-authored-by: cvencro <cr109@ic.ac.uk> Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com> Co-authored-by: TobiKattmann <Tobias.Kattmann@de.bosch.com> Co-authored-by: TobiKattmann <31306376+TobiKattmann@users.noreply.github.com>

SU2_CFD/include/drivers/CDriver.hpp

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>

Nicola-Fonzi · 2021-06-10T15:23:28Z

The unsteady CHT regression is failing, but I cannot understand why. I should not have touched parts of the code that influence that regression test. May I ask you if you have any idea of the reason for this fail?

pcarruscag · 2021-06-11T06:38:08Z

@TobiKattmann any idea about what may be happening with the unsteady cht tutorial?

TobiKattmann · 2021-06-11T12:03:50Z

@TobiKattmann any idea about what may be happening with the unsteady cht tutorial?

Oh man I am so sorry, It was already failing in #1290 that was merged 3 days ago by me. So it is not your problem @Nicola-Fonzi .

Let me explain how that happened (not to justify that or anything): I wanted to move the unsteady CHT case to the tutorial.py file because it is, in fact, a tutorial. As I didn't have a PR open that time I put that into #1226 because it fit into the PR-scope. The PR was not worked on ever since so I now thought I just squeeze it into #1290 to have that covered.
The testcase was not failing in #1226 so I cherry-picked that into #1290 and obviously didn't check if it still works. Code Factor was always giving me red-crosses for all the commits so I didn't pay much attention to that :( 👎

I apologize again and will fix that asap and see what is going on

pcarruscag · 2021-06-11T12:52:23Z

Hmmm looks like we have an issue with the setup of tutorials.py then, because the merge button should not show up as green with failing regressions.
The code quality tests are not hard requirements but everything else should be.

Nicola-Fonzi · 2021-06-11T12:59:41Z

Indeed the tutorial.py is not a required test.
Anyway, I am happy I did not break anything. Do you think we can merge this?

TobiKattmann · 2021-06-11T15:02:48Z

Ok, I think I understood and fixed what happened (and did 2 more stupid mistakes).

In #1226 2 things happened:

I moved the reg-test to tutorials and therefore I had to add files to /su2code/Tutorials/12 of course. There I changed some values (Linear Solver Iter, Outer_Iter, Time_Iter, to name the most important) because the case until then did not get unsteady. Because the original case worked in Heat Transfer boundary condition #1226 before my commit and I only changed cfg-options over at Tutorials and touched no more than tutorials.py -> there was no unwanted introduced code changed that is overlooked!
in Mesh type used in optimization and numerical simulation #1229 three cfg option names for heat were changed ... so I changed them over at /su2code/Tutorials as well cause I thought that the test have to pass in Mesh type used in optimization and numerical simulation #1229 and /su2code/Tutorials contains the cfg's as well (other than for the usual Testcases). I also thought that Mesh type used in optimization and numerical simulation #1229 would be merged in quick succession so that premature change wouldn't hurt much ... well that merge didnt happen until today.

Point 2. above bit me when I cherry picked in #1290 which introduced the failing test visible here. Because with that I changed from the Testcases-test to the Tutorial-test which already had the new cfg-names in it ... which are not in the code yet! Changing that back gave the correct values on my machine and I get the correct values with current develop wrt to the old test (which can be seen in the diff of #1226). Like so I am confident that no code changed the regression test values but rather incautious Tobi.

The 2 mistakes I made were:

I pushed the changes (different cfg-option-names) directly to develop without a PR. That is prob not too bad tbh but I would have preferred a PR
I re-ran the tests for the last commit of this PR to check but I guess it is better to leave that alone and just do that somewhere else in a new PR just to test that ... not sure if that works out

TobiKattmann · 2021-06-11T15:14:42Z

I rerun https://github.com/su2code/SU2/runs/2804205189 in the open PR #1294 which will tell us if this issue is resolved as this commit had this very tutorial case failing as well.
Edit: the rerun does not fail the unsteady cht case in tutorials so I hope this issue is solved then 👍

Nicola-Fonzi and others added 18 commits February 22, 2021 10:41

Merge branch 'develop' into publish

d98b81c

Merge branch 'develop' into publish

0389872

More output for debug

06cfde0

Merge branch 'develop' into publish

5c1cfbd

Allow to extract the normal with info on the area and the unit normal

97c45da

Merge branch 'develop' into publish

d2345ef

small typo

34884c4

Update nodal solution for all points

425c87e

Merge branch 'develop' into publish

0395775

Merge branch 'develop' into publish

82dd738

Merge branch 'develop' into publish

36c7b33

Merge branch 'develop' into publish

240f9f5

Merge branch 'develop' into publish

ab82f2a

Fix indices for restart

5e51693

Fix of restart in interface

18a7a27

Merge branch 'develop' into publish

e11df78

Removed debug

b38b990

Nicola-Fonzi changed the title ~~[WIP] Fix restart logic in python FSI~~ Fix restart logic in python FSI Jun 2, 2021

pr-triage bot added the PR: unreviewed label Jun 2, 2021

Be sure that coupling starts immediately when restarting

2ee6fed

Nicola-Fonzi requested a review from pcarruscag June 10, 2021 14:37

pcarruscag reviewed Jun 10, 2021

View reviewed changes

SU2_CFD/include/drivers/CDriver.hpp Outdated Show resolved Hide resolved

Nicola-Fonzi and others added 2 commits June 10, 2021 16:51

Addressing comment from reviewer

747b2b7

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>

Merge branch 'develop' into publish

cf62dcf

pcarruscag approved these changes Jun 11, 2021

View reviewed changes

pr-triage bot removed the PR: unreviewed label Jun 11, 2021

pr-triage bot added the PR: reviewed-approved label Jun 11, 2021

Nicola-Fonzi merged commit d42c8b7 into su2code:develop Jun 11, 2021

pr-triage bot added PR: merged and removed PR: reviewed-approved labels Jun 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix restart logic in python FSI #1295

Fix restart logic in python FSI #1295

Nicola-Fonzi commented Jun 2, 2021 •

edited

Loading

Nicola-Fonzi commented Jun 10, 2021

pcarruscag commented Jun 11, 2021

TobiKattmann commented Jun 11, 2021

pcarruscag commented Jun 11, 2021

Nicola-Fonzi commented Jun 11, 2021

TobiKattmann commented Jun 11, 2021

TobiKattmann commented Jun 11, 2021 •

edited

Loading

Fix restart logic in python FSI #1295

Fix restart logic in python FSI #1295

Conversation

Nicola-Fonzi commented Jun 2, 2021 • edited Loading

PR Checklist

Nicola-Fonzi commented Jun 10, 2021

pcarruscag commented Jun 11, 2021

TobiKattmann commented Jun 11, 2021

pcarruscag commented Jun 11, 2021

Nicola-Fonzi commented Jun 11, 2021

TobiKattmann commented Jun 11, 2021

TobiKattmann commented Jun 11, 2021 • edited Loading

Nicola-Fonzi commented Jun 2, 2021 •

edited

Loading

TobiKattmann commented Jun 11, 2021 •

edited

Loading