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

Velocity transfer at fluid-structure interface #1174

Merged
merged 47 commits into from
Apr 28, 2021

Conversation

cvencro
Copy link
Contributor

@cvencro cvencro commented Jan 22, 2021

Proposed Changes

This PR enforces the continuity of velocity explicitly at the fluid structure interface for each time step. The velocity was previously recalculated for the fluid nodes based on displacement history of the nodes. This is still needed for fluid only problems with deforming meshes. However, for FSI problems, the velocity at the interface can be transferred from the structural solution and propagated to the fluid nodes similarly to the displacement which ensures exact agreement at the interface at each time-step. This change has been validated using a typical FSI benchmark of square cylinder with a flexible cantilever beam, which gives good agreement in peak tip displacement and Strouhal number.

Related Work

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.

SU2_CFD/src/solvers/CFEASolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CMeshSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CMeshSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CMeshSolver.cpp Show resolved Hide resolved
SU2_CFD/src/solvers/CSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CMeshSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/include/solvers/CSolver.hpp Outdated Show resolved Hide resolved
SU2_CFD/src/drivers/CMultizoneDriver.cpp Outdated Show resolved Hide resolved
@pcarruscag
Copy link
Member

I was looking a bit more into this because of the issue with having to treat the structural solver differently in the Update function.
(I gotta say I feel lucky I never had to look into time domain stuff, because 3 nested iterations... not easy).

Anyway, I think there is a problem here, the "predicted displacement" that is transferred to the fluid during outer iterations (the coupling iterations within each time iteration) is the one set in CFEASolver::SetAitken_Relaxation, which is part of the Corrector step at the end of the structural inner iterations (the ones within each outer iteration).

The "predicted velocity" is not set in SetAitken_Relaxation thus the grid velocities are lagged by one time step, since they are only set to the current velocity of the structure at the end of the time step (in the Update methods).

I have little trust in the predictor step, looking at it I thing only zero order has a chance of working and anything else will cause the fluid and structural sides to separate / overlap, I'll test the theory.

@pcarruscag
Copy link
Member

The predictor thing is fine, the fluid displacement may start different from the structural but this is corrected in the outer iterations.

I think the best solution for the velocity issue (if there is one, but it really looks like there is) would be to include ImplicitNewmark_Relaxation in the corrector step (this sets the solution to the relaxed solution and recomputes velocity and acceleration) and to transfer velocity directly, i.e. without any prediction.

@cvencro
Copy link
Contributor Author

cvencro commented Feb 8, 2021

Hi Pedro, thanks for looking into this more. I had run into inconsistencies for FSI problems with relaxation which was the reason for the domain specific calls coming into the multizone driver Update function. With the changes I introduced, the velocity at the interface was being transferred correctly with and without relaxation, so I left it there but it is a bit messy. I'll test with the modified calls for the Relaxation as you suggest. I completely agree that if we can simplify the velocity transfer by just using the velocity directly, then we should. Especially since the predicted velocity is only zero order anyway.

@cvencro
Copy link
Contributor Author

cvencro commented Apr 18, 2021

Thanks for the review Pedro. Most of the regression tests pass now as well. The dynamic FSI reg tests were updated for the serial and parallel tests since the new velocity bc brings about a change for them inherently. The hybrid regression tests haven't been changed yet, but they are diverging in the tests to begin with. It is very likely that some specific calls for the hybrid implementation might be missing. I tried to follow the existing implementation style but I'm not sure what might be missing/wrong. Can you suggest anything for me to try or look for?

@pcarruscag
Copy link
Member

No problem :) the hybrid parallel should be fixed (it was a parallel section inside another).
Have you had a chance to look into the other issue I think I saw? Namely the structural velocity being transferred is not updated once per outer iteration, but rather once per time step which makes it lag (that is what it looks but I might be wrong, I explained it better on my Feb 7 comment).

@cvencro
Copy link
Contributor Author

cvencro commented Apr 19, 2021

Hi Pedro, sorry I'd forgotten to get to that one. I've just had a look through the logic again. Inside SetAiken_Relaxation, the predicted velocity is set which I think should keep the velocity update to be within the same time iteration, but if you think this still isn't actually the case let me know. Also, I've just spotted that inside Update in CFEAIteration, the logic for dynamic and/or fsi isn't complementary, so the separate "else if" handling for ImplicitNewmark_Relaxation isn't actually called at all. The SetDualTime_Solver which is called earlier in the same function in turn calls ImplicitNewmark_Relaxation so this hadn't been spotted earlier. I've removed the redundant else if statement...

I've had to reintroduce this back since the regression tests for discadj_fsi and discadj_fsi_airfoil failed, which was a bit surprising since they are static fsi problems and the statements removed only considered problems with Newmark time integration.

@cvencro
Copy link
Contributor Author

cvencro commented Apr 19, 2021

No problem :) the hybrid parallel should be fixed (it was a parallel section inside another).
Have you had a chance to look into the other issue I think I saw? Namely the structural velocity being transferred is not updated once per outer iteration, but rather once per time step which makes it lag (that is what it looks but I might be wrong, I explained it better on my Feb 7 comment).

Thanks for the fix for the hybrid parallel reg test! The dynamic fsi test cases were the only remaining ones which was expected, this has been updated now.

Comment on lines +584 to +586
if (config->GetnZone() == 1)
SU2_MPI::Error("It is not possible to compute grid velocity from boundary velocity for single zone problems.\n"
"MARKER_FLUID_LOAD should only be used for structural boundaries.", CURRENT_FUNCTION);
Copy link
Member

Choose a reason for hiding this comment

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

@Nicola-Fonzi this should prevent this type of grid velocity method from being used when the python wrapper is used.

@pcarruscag
Copy link
Member

Oh right... It was there all along and I missed it. Hope you don't mind I made that consistent and merged the velocity prediction with the displacement prediction.
And so everything LGTM.

@pcarruscag pcarruscag changed the title [WIP] Velocity transfer at fluid-structure interface Velocity transfer at fluid-structure interface Apr 19, 2021
@cvencro
Copy link
Contributor Author

cvencro commented Apr 19, 2021

Hi Pedro, neat solution. Thanks for all your help!

@pcarruscag
Copy link
Member

👍 shall we merge this PR then?

@cvencro
Copy link
Contributor Author

cvencro commented Apr 27, 2021

Yes, nothing more from me on this. Once this is merged, the comparison in PR #1260 should become simpler too. I've just updated this branch with develop, we can merge once the regression tests pass again.

@pcarruscag pcarruscag merged commit 0b04842 into develop Apr 28, 2021
@pcarruscag pcarruscag deleted the feature_velocity_bc_fsi branch April 28, 2021 08:40
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.

4 participants