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

Subtract mesh velocity before computing whether a boundary experiences in- or outflow for fixed boundary conditions #6067

Merged
merged 2 commits into from
Nov 11, 2024

Conversation

anne-glerum
Copy link
Contributor

@anne-glerum anne-glerum commented Oct 10, 2024

As described in #5994, this PR corrects the velocity that is used to determine whether a boundary is an in- or outflow boundary by subtracting the mesh velocity. Otherwise some boundary parts can be considered inflow boundaries and fixed composition will be prescribed, eventhough no inflow occurs, and vice versa.

I have added a new benchmark prm to test these changes, and can turn that into a test.

I would like input on what to do during the 0th and 1st timestep as I had to set the whole boundary to outflow at t0 and t1, because:

  1. The compositional boundary conditions are computed at the beginning of the timestep before mesh deformation.
  2. Mesh deformation is first computed at t1, not t0.
  3. Stokes and mesh velocity in the benchmark setup are therefore 0 during the evaluation of in/outflow in timestep t0 and t1, and the whole top boundary would become a prescribed field boundary.

For all pull requests:

For new features/models or changes of existing features:

  • I have tested my new feature locally to ensure it is correct.
  • I have created a testcase for the new feature/benchmark in the tests/ directory.
  • I have added a changelog entry in the doc/modules/changes directory that will inform other users of my change.

@gassmoeller
Copy link
Member

Thanks for the fix Anne! The code looks good, and yes a test made out of this benchmark would be great.

To your questions: I think this is linked to our perennial discussion on whether open boundaries through which no material is flowing at the moment should be considered inflow or outflow boundaries. I think the current state and remaining problems are summarized in #5500 (other relevant issues are #5042, #5481, and in particular also #5479). I am not sure what an optimal solution would look like. Do you think input parameters in the boundary temperature and boundary composition subsection that allow the user to select how to treat boundaries with no flow at the current time would solve some problems? In #5479 I suggested to make the treatment dependent on whether there is diffusion in the field or not, would that work for this case?

@anne-glerum
Copy link
Contributor Author

Hi Rene, I've added the test.

I think adding a parameter that specifies how to treat the deforming boundary for zero flow in timestep 0 would be good. With mesh deformation though, it also has to apply in t1, as the mesh velocity will be evaluated to determine in/outflow before mesh velocity has actually been solved for.

I don't fully understand the diffusive field option. Do you mean that if a field does not undergo natural diffusion, but only numerical because of stabilization, like a standard compositional field, other boundary conditions should apply then for temperature?

@anne-glerum
Copy link
Contributor Author

Lot's of tests now fail because I changed the behavior at t0 and t1, but this will change again if we settle on some other way of specifying when to not apply boundary conditions.

I can remove the additional check for being in t0 and t1 for now, only subtracting the mesh velocity is by itself an improvement on the current situation.

@gassmoeller
Copy link
Member

Could you: 1. Reduce the size of the test by one or two mesh refinement levels to keep the output smaller, and 2. Remove the change to the t0 and t1 and update the test results? Then we can merge the improvement of considering mesh velocities first and deal with the t0 and t1 in another way.

Maybe we can try to revive #5479 where I tried before to allow the user to select whether to apply boundary conditions on boundaries that have no flow across them. That is not exactly the problem here though, so we will have to think how to fix it completely.

Copy link
Member

@gassmoeller gassmoeller left a comment

Choose a reason for hiding this comment

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

Thanks, this is ready to merge. I took the liberty of squashing your commits and adding a changelog to speed up the merge process ;-).

@gassmoeller gassmoeller merged commit 301faeb into geodynamics:main Nov 11, 2024
8 checks passed
@anne-glerum
Copy link
Contributor Author

Thanks Rene!

@anne-glerum anne-glerum deleted the adjust_v_for_outflow_BC branch November 18, 2024 09:38
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.

2 participants