Skip to content

Conversation

@Nicola-Fonzi
Copy link
Contributor

@Nicola-Fonzi Nicola-Fonzi commented Dec 5, 2020

Proposed changes

Dear all,

I am very new to collaboration on a open source project, but I have been working on an update for the FSI computation in python that I think may be useful also for other researcher. Would you mind giving me a feedback about the modifications?

In few words, I have updated the python interface for fluid structure interaction to the new version of SU2, including the new driver, the new mesh deformation methods and few other modifications in the python scripts. It works with python3.

As far as the C++ code is concerned, I only added the possibility to separate the symmetry boundary conditions for the fluid solver from the ones of the mesh solver. There is an explanation of this modification in the ReadMe file in SU2_PY/FSI_tools, together with an example of application.

The interface is still general and new structural solvers can be coupled by adding it to the fsi_computation.py file. With the present pull request I only include one structural solver, which provides a coupling with the commercial code Nastran.

I added ReadMe files to explain in detail all the modifications and also the keywords to be used in the config files. Finally, there is also a complete test case where the flutter of a flat plate is studied.

I am looking forward for your comments

Nicola

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.

Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very interesting especially if it makes it easier to interface other solvers.

One think I must ask is to move all mesh files to the TestCases repository and all the config files to TestCases folder.
You could also try to re-use the mesh files across your different examples, we should try to reduce the size of the repository.

Alternatively you can also create some tutorials (since you already have some readme files), we have a special repository for those, I think a tutorial would give your work more visibility.

I will leave some comments on the code once I go through everything.

@Nicola-Fonzi
Copy link
Contributor Author

Nicola-Fonzi commented Dec 7, 2020

Thank you for the feedbacks, I am working on the modifications you suggested. I will perform a couple of tests to be sure I did not break anything.

Hope to commit the new code soon!

@Nicola-Fonzi
Copy link
Contributor Author

I was trying to merge the develop to resolve conflicts and I found an error in the compilation. I thought I had broken something so I tried to start fresh new with the commands:
git clone https://github.com/su2code/SU2.git
./meson.py build -Denable-pywrapper=true --prefix=/path/to/my/folder
./ninja -C build install
Even from the master branch of your repo I still find the error attached below. Is it a problem with my compiler? I could always compile with no issues before. The last one I tried from you repo was 7.0.6.
error

@Nicola-Fonzi
Copy link
Contributor Author

I use openMPI 4.0.4 on a Linux CentOS 7

@pcarruscag
Copy link
Member

What about compiler version? If you do gcc --version what do you get? (assuming it is gcc)

@Nicola-Fonzi
Copy link
Contributor Author

gcc version 4.8.5

@pcarruscag
Copy link
Member

I think that was the first gcc with official full c++11 support, and so it might not be full support...
Try pushing the merge anyway so that github can run the tests, I'll see if something can be done about 4.8.5.

@Nicola-Fonzi
Copy link
Contributor Author

Okok, I may also try to update gcc. The only issue is that it is on a server so I should do it locally and it may take a bit more time. I will test the merge and then, with more time, update gcc.

@Nicola-Fonzi
Copy link
Contributor Author

I solved for the compilation issues by upgrading gcc. Thank you for the suggestion!

@pcarruscag
Copy link
Member

I'll merge this now to try to conciliate it with #902
I will probably ask you to check if I broke anything.

@pcarruscag pcarruscag merged commit 6691b70 into su2code:develop Dec 11, 2020
@Nicola-Fonzi
Copy link
Contributor Author

Ok I will now try to run two main test cases I have. But there were no conflicting files after the last commit so I do not think there should be any issue.

@Nicola-Fonzi
Copy link
Contributor Author

Everything seems to be fine. The test cases run smoothly.

I have been following the conversation with Rocco, I will now contact him to understand how to avoid duplication and how to best integrate our works

@TobiKattmann
Copy link
Contributor

gcc version 4.8.5

yep I had to move to gcc 5.3.0 recently from 4.8.5 actually quite recently... which would be in line with 7.0.6 working for you. I don't recall exactly what feature that was 🤔 Beginning of October with change to 7.0.7 and the SIMD stuff

@pcarruscag
Copy link
Member

Guilty as charged.

Copy link
Contributor

@TobiKattmann TobiKattmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I am a bit late to the party. Thanks for the contribution and adding the tutorial as well 💐
It would be nice to have a testcase in tutorials.py to safeguard it against future changes. Maybe it is wise to wait for after #902 if that changes sth (but I have no clue tbh).
Usually not everything needs its own testcase but as this adds a tutorial, so I vote here in favor of testcase.

(btw I stumbled upon this today and thought I already heard that name of the first author somewhere already :D . Big fan of Mr Brunton 👍 )

Comment on lines +903 to +909
for (unsigned long iPoint = 0; iPoint < geometry_container[ZONE_0][INST_0][iMesh]->GetnPoint(); iPoint++) {

/*--- Overwrite fictitious velocities ---*/
su2double Grid_Vel[3] = {0.0, 0.0, 0.0};

/*--- Set the grid velocity for this coarse node. ---*/
geometry_container[ZONE_0][INST_0][iMesh]->nodes->SetGridVel(iPoint, Grid_Vel);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess because it is automatic memory allocation with fixed values the compiler optimizes this su2double Grid_Vel[3] = {0.0, 0.0, 0.0}; away. Otherwise I would argue that Grid_vel could be created outside of at least the iPoint loop

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right!

Comment on lines 662 to 670
/*--- Symmetry plane is clamped, for now. ---*/
for (iMarker = 0; iMarker < config->GetnMarker_All(); iMarker++) {
if ((config->GetMarker_All_Deform_Mesh(iMarker) == NO) &&
(config->GetMarker_All_Deform_Mesh_Sym_Plane(iMarker) == NO) &&
(config->GetMarker_All_Moving(iMarker) == NO) &&
(config->GetMarker_All_KindBC(iMarker) == SYMMETRY_PLANE)) {

BC_Clamped(geometry, numerics, config, iMarker);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me: Is the symmetry boundary clamped?
Su2:
image
... but no

(not your issue but i found it funny that above explicitly is stated Exceptions: symmetry plane, the receive boundaries and periodic boundaries should get a different treatment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahaha yes I also found it quite funny when I first saw it. But it is fine now to clamp it... As the sym plane for fluid and mesh are now separated, one will explicity use the deform mesh sym plane marker if she/he wants to obtain a symmetric deformation. Otherwise, the code will clamp deformation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was because the FEA solver did not use to have a symmetry BC.
And then when I added it, it was better to clamp the symmetry plane for my testcases :)
But now that Nicola decoupled the physical symmetry from the deformation symmetry all is well xD

@Nicola-Fonzi
Copy link
Contributor Author

The most critical part is the unsteady FSI, but that actually takes its time... You think it would be fine to compare the results after just few iterations for a testcase? We could use one of the simulations in the tutorial.

Thanks for the interest in the youtube video!

@pcarruscag
Copy link
Member

All testcases run for only a few steps, not entirely to convergence, the purpose is really just to detect change.

@Nicola-Fonzi
Copy link
Contributor Author

Ok, I will prepare a test case. I will take the Mach=0.1 case from the tutorial and change it slightly. Now the coupling between fluid and structure occurs after 100 steps, I will just couple them immediately.
The only issue with #902 I see at the moment is with the file fsi_computation.py. The others are in separate folders so they should not create conflicts...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants