Updates of SU2-python by Nicola-Fonzi · Pull Request #1357 · su2code/SU2

Nicola-Fonzi · 2021-08-22T13:02:39Z

Proposed Changes

Dear all,

Here are some updates for the python fsi interface.

Namely:

Better use of mpi4py, using always numpy array (more computationally efficient)
Fix of the restarted coupled simulation
Automatic use of default values, in case of wrong inputs
Introduction of a sort of dry run, which actually prints out the mapped modes and the normals of the aerodynamic surface mesh. This is yet another step towards the automatic creation of ROM from aerodynamic response
Fix of small bugs

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.

lgtm-com · 2021-08-22T14:05:49Z

This pull request introduces 2 alerts and fixes 19 when merging 0cf08d4 into c2c78e2 - view on LGTM.com

new alerts:

1 for Unused local variable
1 for Variable defined multiple times

fixed alerts:

13 for Variable defined multiple times
6 for Unused import

SU2_PY/SU2_Nastran/pysu2_nastran.py

SU2_PY/FSI_tools/FSIInterface.py

Nicola-Fonzi · 2021-08-23T07:09:11Z

Dear @pcarruscag, thank you very much for the review. I implemented the modification you suggested and corrected the indentation (sorry about that...)

lgtm-com · 2021-08-23T07:55:14Z

This pull request introduces 2 alerts and fixes 19 when merging 4ecf3ea into c2c78e2 - view on LGTM.com

new alerts:

1 for Unused local variable
1 for Variable defined multiple times

fixed alerts:

13 for Variable defined multiple times
6 for Unused import

pcarruscag

Thanks @Nicola-Fonzi 👍

TobiKattmann

Thanks @Nicola-Fonzi 💐 of course I cannot say anything content-wise but here are a few little spots I can nag about

TobiKattmann · 2021-08-24T07:06:05Z

SU2_PY/SU2_Nastran/pysu2_nastran.py

    return len(self.markers[markerID])

-  def getInterfaceNodeGlobalIndex(self, markerID, iVertex):
+  def getVertexGlobalIndex(self, markerID, iVertex):   # TODO This solver is serial, thus global=local


Are the TODO on your list to be tackled next? I would rather error-check and document the state then to write a TODO in the code

Hi Tobi! Thank you very much for your time. Yes, you are right... TODOs in the code are ugly. They are definitely being tackled right now, this is why they are there, but I can remove them and just keep a note.

TobiKattmann · 2021-08-24T07:08:25Z

SU2_PY/SU2_Nastran/pysu2_nastran.py

+        if self.Config["RESTART_SOL"] == "NO":                                   # If yes we already set it in the __setRestart function
+          self.timeStartCoupling = time


Suggested change

if self.Config["RESTART_SOL"] == "NO": # If yes we already set it in the __setRestart function

self.timeStartCoupling = time

if self.Config["RESTART_SOL"] == "NO":

# If yes we already set it in the __setRestart function

self.timeStartCoupling = time

I am not sure whether a comment 20 spaces away is the best thing todo, I would rather have it close to its partner lines

TobiKattmann · 2021-08-24T07:14:58Z

SU2_PY/FSI_tools/FSIInterface.py

+              for key in self.FluidHaloNodeList[iProc].keys():                  # The key is the SU2 global
+                globalIndex = self.fluidIndexing[key]                           # This is the interface global, not the SU2 global


Not that I would understand what is going on but # The key is the SU2 global sounds like there is sth missing.

you mean that it should be "the keys are..."?

Hm, no I think more sth like

# The key is the SU2 global >node ID<

in case that makes any sense... That is the whole point :) I dont know what SU2 global alone should mean. I just had the feeling there was a noun missing (like e.g. node ID), not that I know it is missing.

Treat it more like a question: Is this sentence correct? If yes, then leave it as is 👍

I see what you mean. No the comment was actually saying that each key in the dictionary consists of the global ID of an interface node. However, from your question, I feel it was not really clear... I try to clarify

Please have a look at #1359

Nicola-Fonzi added 28 commits June 21, 2021 10:42

New treatment of markers

1c7dfe6

Small bug on new treatment of markers

f0cbcc3

Small changes to the MPI functions

a2f682c

Bug in Allreduce

4253cf2

New transfer of info between cores

fe5ecbf

Typo

e26e4a4

Introduced mapping of modes

742ff65

Bad copy-paste

358b2d3

Introduced some feedback when applying defaults

ef2f01e

We were not calling MapModes

6f3091b

Added support for blank fields

afbb6a3

Typo

14425c5

another bad copy paste

8864b10

typo

0bd4132

debug1

0ac4cfa

We were not updating the solution

666388f

small bug

7696e5f

Merge branch 'develop' into publish

340ada5

Slim restart

82d71bf

Introduced stop time for imposed motion

1fd0cf7

Correct print

a12fede

fixed issue with time0

dfb97c6

fixed tab

64f7366

fixed wrong index for time_0

53b5f58

Handle immediate coupling cases

ce6fd3a

raise instead of print

4c18f7c

Merge branch 'develop' into publish

a9ecfd2

Merge branch 'develop' into publish

349594a

pr-triage bot added the PR: unreviewed label Aug 22, 2021

Modification suggested by codeFactor

3f350e9

Merge branch 'develop' into publish

0cf08d4

pcarruscag reviewed Aug 22, 2021

View reviewed changes

SU2_PY/SU2_Nastran/pysu2_nastran.py Outdated Show resolved Hide resolved

SU2_PY/SU2_Nastran/pysu2_nastran.py Outdated Show resolved Hide resolved

SU2_PY/SU2_Nastran/pysu2_nastran.py Outdated Show resolved Hide resolved

SU2_PY/FSI_tools/FSIInterface.py Outdated Show resolved Hide resolved

Fixes as suggested by reviewers

4ecf3ea

pcarruscag approved these changes Aug 23, 2021

View reviewed changes

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels Aug 23, 2021

pcarruscag added the changelog:chore label Aug 23, 2021

Nicola-Fonzi merged commit c488eda into su2code:develop Aug 23, 2021

pr-triage bot added PR: merged and removed PR: reviewed-approved labels Aug 23, 2021

TobiKattmann reviewed Aug 24, 2021

View reviewed changes

pr-triage bot added PR: reviewed-approved and removed PR: merged labels Aug 24, 2021

Nicola-Fonzi mentioned this pull request Aug 24, 2021

Update SU2-python 2.0 #1359

Merged

5 tasks

		if self.Config["RESTART_SOL"] == "NO": # If yes we already set it in the __setRestart function
		self.timeStartCoupling = time

		for key in self.FluidHaloNodeList[iProc].keys(): # The key is the SU2 global
		globalIndex = self.fluidIndexing[key] # This is the interface global, not the SU2 global

Conversation

Nicola-Fonzi commented Aug 22, 2021

Proposed Changes

PR Checklist

Uh oh!

lgtm-com bot commented Aug 22, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Nicola-Fonzi commented Aug 23, 2021

Uh oh!

lgtm-com bot commented Aug 23, 2021

Uh oh!

pcarruscag left a comment

Choose a reason for hiding this comment

Uh oh!

TobiKattmann left a comment

Choose a reason for hiding this comment

Uh oh!

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

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

TobiKattmann Aug 24, 2021 •

edited

Loading