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

Adding a new two fluid solver for hydraulic application #11025

Merged
merged 28 commits into from
Sep 22, 2023

Conversation

uxuech
Copy link
Contributor

@uxuech uxuech commented Apr 17, 2023

📝 Description
This PR introduces a new two-fluid solver with a hydraulic application, and includes several key differences from the previous "two_fluid_hydraulic_solver":

  • The addition of an option to implement artificial viscosity based on the residual, which improves robustness and eliminates potential numerical noise.

  • A correction is applied to the historical velocity variable before solving the N.S equations, achieved by solving a convection problem where the velocity variable from the previous step is convected (eulerian fm ale problem), thus preserving energy.

  • The addition of an option to calculate the stabilization parameter tau for the convection element nodally, which prevents sudden jumps in the value of the tau between elements.

In addition, a new test has been added to verify all changes made in both 2D and 3D of the sloshing benchmark.

@uxuech uxuech requested a review from a team as a code owner April 17, 2023 15:20
@ddiezrod
Copy link
Contributor

ddiezrod commented Apr 18, 2023

@rubenzorrilla @uxuech Is there any plan to unify the developments here with the navier stokes two fluid solver? I understand that separating both solvers will accelerate the implementation but I would like to have only 1 solver at some point....

@rubenzorrilla
Copy link
Member

@rubenzorrilla @uxuech Is there any plan to unify the developments here with the navier stokes two fluid solver? I understand that separating both solvers will accelerate the implementation but I would like to have only 1 solver at some point....

First, I'd like to note that this is what we tried first.

Answering to your question, not in the short term, an I foresee that not in the mid term either. The point is that while developing we realized that @uxuech 's target application needs various specifics (mass conservation algorithm, history convection, artificial viscosity, etc.) that would result in an (even more) cumbersome implementation in current solver.

Besides, I think that the defaults are to be rather different in both cases. Note that current 2-fluid solver is being used for quite different stuff (yours, microfluidics, etc.) while this one is intended to be applied large scale hydraulic applications.

@ddiezrod
Copy link
Contributor

ddiezrod commented Apr 21, 2023

Thanks for the answer @rubenzorrilla I am afraid that the whole structure might be a little misleading (for example I think my casting filling examples will have more in common with this hydraulic solver than with some of the more recent developments in the two fluid solver, mostly related to bubble formation etc). Besides the code duplication will be a little of a headache. This being said, I understand that we have to be flexible until everything is properly tested and stabilized, so for me it is ok to add this new solver.

@rubenzorrilla
Copy link
Member

Thanks for the answer @rubenzorrilla I am afraid that the whole structure might be a little misleading (for example I think my casting filling examples will have more in common with this hydraulic solver than with some of the more recent developments in the two fluid solver, mostly related to bubble formation etc). Besides the code duplication will be a little of a headache. This being said, I understand that we have to be flexible until everything is properly tested and stabilized, so for me it is ok to add this new solver.

Thinking in the long run, the best solution could be to have a base "do nothing" two-fluid solver with all the ingredients and different derived solvers re-implementing the defaults and key functions.

Copy link
Member

@rubenzorrilla rubenzorrilla left a comment

Choose a reason for hiding this comment

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

There are some concerns to be amended.

Besides, from my understanding I think you should have one results file per test . However there are only two. Regarding the mdpa(s), do we need them to be that fine? Couldn't we use a coarser mesh?

self.model,
distance_modification_settings)

#TODO: Temporary solution for fixed alpha scheme
Copy link
Contributor Author

@uxuech uxuech Sep 20, 2023

Choose a reason for hiding this comment

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

Answering to why the "time_scheme" is present in the default Parameters, it's here. This solver is derived from the fluid_solver, and therefore, we decided to implement this temporary solution to make it work. @rubenzorrilla , do you think we should modify this part of the code?

Copy link
Member

Choose a reason for hiding this comment

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

Considering that this new solver is specifically tailored for your element, which hardcodes an alpha-scheme within the RHS and LHS implementation, I'd leave it as it is. In other words, this solver is only compatible with ResidualBasedIncrementalUpdateStaticSchemeSlip.

@uxuech
Copy link
Contributor Author

uxuech commented Sep 20, 2023

Regarding the Python tests, I would like to know how much you suggest we should reduce the MDPA (Mesh Data and Parameter Archive). Thank you for your help @rubenzorrilla.

@rubenzorrilla
Copy link
Member

Regarding the Python tests, I would like to know how much you suggest we should reduce the MDPA (Mesh Data and Parameter Archive). Thank you for your help @rubenzorrilla.

If it works, I'd do a mesh of 5 times 5 divisions (5 times 5 times 5 in 3D). At the end of the day you just want to check one of the very first steps.

Copy link
Member

@rubenzorrilla rubenzorrilla left a comment

Choose a reason for hiding this comment

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

Ready 🚀

@uxuech uxuech merged commit 13b795a into master Sep 22, 2023
@uxuech uxuech deleted the 2F_hydraulic_application_solver branch September 22, 2023 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants