Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

including two flap case #39

Merged
merged 14 commits into from
Jan 7, 2021
Merged

including two flap case #39

merged 14 commits into from
Jan 7, 2021

Conversation

carme-hp
Copy link

With this modifications, the dealii-adapter would support the tutorial case with two perpendicular flaps. Besides FSI3 and PF, now we have the case PFleft and PFright, who only differ from PF by the x coordinates in point_bottom and point_tip.

Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

I think this is ready to review. It compiles and seems to be running with precice/tutorials#110.

The code looks reasonable.

@davidscn what do you think? Do we need anything else?

@MakisH MakisH changed the base branch from master to develop November 26, 2020 21:45
Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

Actually, we need an additional step: it looks like we pass the wrong vertices to setMeshVertices and each flap is still located in the middle.

@davidscn where can we modify the support_points? It looks like you call DoFTools::map_dofs_to_support_points() in adapter.h, but I am not sure what is going on under the hood. Maybe you have an idea?

@davidscn
Copy link
Member

@davidscn where can we modify the support_points? It looks like you call DoFTools::map_dofs_to_support_points() in adapter.h, but I am not sure what is going on under the hood. Maybe you have an idea?

I will have a look at the deal.II side later.

@davidscn
Copy link
Member

Actually, we need an additional step: it looks like we pass the wrong vertices to setMeshVertices and each flap is still located in the middle.

How did you come to this conclusion? Running both cases works just as expected for me. How can I reproduce the issue?

@MakisH
Copy link
Member

MakisH commented Nov 27, 2020

How did you come to this conclusion? Running both cases works just as expected for me. How can I reproduce the issue?

Exactly, here is the confusing part:

  • deal.II writes the mesh in the correct locations
  • the meshes exported by preCICE are in the wrong side

You can reproduce this by adding <export:vtk directory="preCICE-output" /> in the solid1 / solid2 participants in the precice-config.xml and loading the surface meshes in ParaView (and visualizing them e.g. with glyphs).

@uekerman
Copy link
Member

Should we unify the definition of PF, PFleft and PFright to avoid the code duplication? In the end only the location in x direction is different, right?

@davidscn
Copy link
Member

You can reproduce this by adding <export:vtk directory="preCICE-output" /> in the solid1 / solid2 participants in the precice-config.xml and loading the surface meshes in ParaView (and visualizing them e.g. with glyphs).

Done, looks for me reasonable. Also, selecting other files results in two separate beams.

Should we unify the definition of PF, PFleft and PFright to avoid the code duplication? In the end only the location in x direction is different, right?

Yes. I think a more general 'mesh interface' without the duplication in each code is even a better approach and should be added in case this becomes larger.

@carme-hp
Copy link
Author

That's weird, in my case if I visualize in paraview the same files you did in the screenshot the flaps are located in the same position. Can this be a paraview related issue? in my case I use 5.7.0

@davidscn
Copy link
Member

If you have doubts about paraview you could directly look into the vtks.
Running head -n 20 Solid1_mesh-Fluid.init.vtk prints the following:

# vtk DataFile Version 2.0

ASCII

DATASET UNSTRUCTURED_GRID

POINTS 157 float 

-1.0500000000000000e+00  0.0000000000000000e+00  0.0000000000000000e+00
-1.0500000000000000e+00  5.5555555555555552e-02  0.0000000000000000e+00
-1.0500000000000000e+00  9.5929535914450782e-03  0.0000000000000000e+00
-1.0500000000000000e+00  2.7777777777777776e-02  0.0000000000000000e+00
-1.0500000000000000e+00  4.5962601964110471e-02  0.0000000000000000e+00
-9.4999999999999996e-01  0.0000000000000000e+00  0.0000000000000000e+00
-9.4999999999999996e-01  5.5555555555555552e-02  0.0000000000000000e+00
-9.4999999999999996e-01  9.5929535914450782e-03  0.0000000000000000e+00
-9.4999999999999996e-01  2.7777777777777776e-02  0.0000000000000000e+00
-9.4999999999999996e-01  4.5962601964110471e-02  0.0000000000000000e+00
-1.0500000000000000e+00  1.1111111111111110e-01  0.0000000000000000e+00
-1.0500000000000000e+00  6.5148509147000627e-02  0.0000000000000000e+00

and head -n 20 Solid2_mesh-Fluid.init.vtk the following:

# vtk DataFile Version 2.0

ASCII

DATASET UNSTRUCTURED_GRID

POINTS 157 float 

9.4999999999999996e-01  0.0000000000000000e+00  0.0000000000000000e+00
9.4999999999999996e-01  5.5555555555555552e-02  0.0000000000000000e+00
9.4999999999999996e-01  9.5929535914450782e-03  0.0000000000000000e+00
9.4999999999999996e-01  2.7777777777777776e-02  0.0000000000000000e+00
9.4999999999999996e-01  4.5962601964110471e-02  0.0000000000000000e+00
1.0500000000000000e+00  0.0000000000000000e+00  0.0000000000000000e+00
1.0500000000000000e+00  5.5555555555555552e-02  0.0000000000000000e+00
1.0500000000000000e+00  9.5929535914450782e-03  0.0000000000000000e+00
1.0500000000000000e+00  2.7777777777777776e-02  0.0000000000000000e+00
1.0500000000000000e+00  4.5962601964110471e-02  0.0000000000000000e+00
9.4999999999999996e-01  1.1111111111111110e-01  0.0000000000000000e+00
9.4999999999999996e-01  6.5148509147000627e-02  0.0000000000000000e+00

so that I can directly identify the left and the right flap respectively.

@carme-hp
Copy link
Author

Thanks! It is indeed smt related to paraview, when I open the files I see same as you:)

@MakisH
Copy link
Member

MakisH commented Nov 28, 2020

@carme-hp can it maybe be that the files were out-of-date? What if you completely remove the precice-output directory and run again?

@MakisH
Copy link
Member

MakisH commented Nov 28, 2020

Should we unify the definition of PF, PFleft and PFright to avoid the code duplication? In the end only the location in x direction is different, right?

Good point. Maybe we could just make the location a parameter in the deal.II adapter?

@carme-hp
Copy link
Author

@MakisH you are right! I could visualize it correctly when I completely removed the precice-output and run again. Sorry about the confusion. Maybe we could add the precice-output directory to the Allclean to avoid this problem happening again?

@MakisH
Copy link
Member

MakisH commented Nov 28, 2020

Maybe we could add the precice-output directory to the Allclean to avoid this problem happening again?

Yes, let's do that!

Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

Much cleaner and simpler than the previous attempt! 👍 😄

Please remove the compiled binaries from the commit, add them to the .gitignore, and let's squash before merging.

Other than that, the code looks clear, apart from one point that may need a comment. @davidscn could you please also have a quick look at the code changes?

The linear case builds and seems to be running correctly and producing reasonable results visually. Same for the nonlinear.

Nice as well: this change is backwards-compatible. I did not need to update the tutorials to run it.

Copy link
Member

@davidscn davidscn left a comment

Choose a reason for hiding this comment

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

I have (not yet) introduced a format checker, but I think the PR doesn't use the .clang-format of this repo. So please format the PR and maybe rebase your branch to the current develop branch.

linear_elasticity/include/parameter_handling.h Outdated Show resolved Hide resolved
nonlinear_elasticity/nonlinear_elasticity.cc Outdated Show resolved Hide resolved
@MakisH
Copy link
Member

MakisH commented Dec 16, 2020

I have (not yet) introduced a format checker, but I think the PR doesn't use the .clang-format of this repo. So please format the PR and maybe rebase your branch to the current develop branch.

@davidscn This needs some documentation. Related issue: #42

@davidscn
Copy link
Member

Nice as well: this change is backwards-compatible. I did not need to update the tutorials to run it.

Are you sure with this? If we change the parameter file, we also need to change it in the tutorials. Maybe we can define here a default value in order to avoid it. In addition, we need to update the parameter files in this repository.

@davidscn davidscn added the enhancement New feature or request label Dec 17, 2020
@MakisH
Copy link
Member

MakisH commented Dec 17, 2020

Nice as well: this change is backwards-compatible. I did not need to update the tutorials to run it.

Are you sure with this? If we change the parameter file, we also need to change it in the tutorials. Maybe we can define here a default value in order to avoid it. In addition, we need to update the parameter files in this repository.

I thought there was already a default value. If not, there should be. Does it not run out-of-the-box for you?

@carme-hp
Copy link
Author

so I put 0.0 as the default value for the flat location, which is the location that we had before. So I expect it works backwards (it did for the flapperpendicular2D case)

@davidscn
Copy link
Member

davidscn commented Dec 17, 2020 via email

@MakisH MakisH marked this pull request as ready for review December 18, 2020 10:13
Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

From my side, this can be merged after the files are formatted with clang-format.

@davidscn any last objections/wishes?

Copy link
Member

@davidscn davidscn left a comment

Choose a reason for hiding this comment

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

No, not really. It might be worth to add the flap location to our 'default' parameter files in this repo in order to have a complete list of all available options here. However, you can do this according to your opinion. I can also format the file after merging if you struggle with this. Feel free to merge.

@davidscn davidscn closed this Jan 7, 2021
@davidscn davidscn reopened this Jan 7, 2021
@davidscn
Copy link
Member

davidscn commented Jan 7, 2021

...so that we can see the result of the indentation check..

@davidscn davidscn merged commit 3e960ca into precice:develop Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants