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

Fix NS-VOF stong residual formulation #1180

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Conversation

hepap
Copy link
Collaborator

@hepap hepap commented Jun 26, 2024

Description

  • The term associated with the viscosity jump in the strong residual for the NS-VOF assembler added in PR Add missing VOF term in the strong form of the residual #1149 led to a ill-posed formulation in phase change cases where the solid is represented by a highly viscous fluid. Hence, we don't consider this term in the current formulation.
  • There was no appropriate application test for lpbf/phase change with vof cases that could have allowed us to identify this ill-posed formulation prior to the merge of PR Add missing VOF term in the strong form of the residual #1149.
  • The LPBF benchmark example prm files were almost up to date, but did not exactly matched the one we used to generate the benchmark results. The mesh files were missing from the repo.

Solution

  • This PR removes the term associated with the viscosity jump.
  • A new test is now included to avoid future mistake in lpbf/phase change with vof cases.
  • LPBF benchmark example prm files were modified and are now up to date with the current working version. The mesh files are now available in the repo.

Testing

  • The new test simulate the lpbf benchmark case using the extra-extra-coarse mesh. Temperature statistics and mass conservation monitoring are the outputted metrics (to track both the change in HT and NS-VOF solvers).
  • Other NS-VOF tests were updated, and results for those tests returned to their previous values (or almost with small differences).

Documentation

No modification in the documentation.

Miscellaneous (will be removed when merged)

I guess the tests will fail because of the CI issue. I have still opened the PR since the formulation won't change and I will update the restart of the new test when I have updated my P4est version :)

Checklist (will be removed when merged)

Code related list:

  • All in-code documentation related to this PR is up to date (Doxygen format)
  • Lethe documentation is up to date
  • Fix has unit test(s) (preferred) or application test(s)
  • The branch is rebased onto master
  • Changelog (CHANGELOG.md) is up to date
  • Code is indented with indent-all and .prm files (examples and tests) with prm-indent

Pull request related list:

  • Labels are applied
  • There are at least 2 reviewers (or 1 if small feature) excluding the responsible for the merge
  • If this PR closes an issue or is related to a project, it is linked in the "Projects" or "Development" section
  • If the fix is temporary, an issue is opened
  • The PR description is cleaned and ready for merge

@hepap hepap added WIP When a PR is open but not ready for review and removed Ready for review labels Jun 26, 2024
@blaisb
Copy link
Contributor

blaisb commented Jul 1, 2024

The debug test still seems to crash?

@AmishgaAlphonius
Copy link
Collaborator

AmishgaAlphonius commented Jul 1, 2024

The debug test still seems to crash?

It's a bit weird, the load from a checkpoint function for the triangulation has an assertion that checks if all cells are at the same level of refinement. In Release there is no issue, but in Debug, it crashes because of this. I tried it locally also and I had the same issue. Is it that assertions are only checked on Debug?

source/solvers/navier_stokes_vof_assemblers.cc Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@blaisb
Copy link
Contributor

blaisb commented Jul 2, 2024

The debug test still seems to crash?

It's a bit weird, the load from a checkpoint function for the triangulation has an assertion that checks if all cells are at the same level of refinement. In Release there is no issue, but in Debug, it crashes because of this. I tried it locally also and I had the same issue. Is it that assertions are only checked on Debug?

You need to make sure the restart files does NOT refine the triangulation. Generally this is the case for all the solvers (we do not do mesh refinement when we are reading a restart file), however we don't have any restart tests for VOF I guess,so maybe this issue was there all along. You can check a restart with any VOF cse in the debug to see if that's the issue.
But otherwise, the triangulation/ mesh must have the same properties as the one used in the restart (aka the coarse grid has to be the same)

Assertions are not run in DEBUG mode, so that it works in release does not mean anything.

Comment on lines 1 to 24
Running on 1 MPI rank(s)...
Number of active cells: 16
Number of degrees of freedom: 75
Volume of triangulation: 0.36
Number of thermal degrees of freedom: 25
Number of VOF degrees of freedom: 25
Initial refinement in box - Step 1 of 3
Number of active cells: 40
Number of degrees of freedom: 165
Volume of triangulation: 0.36
Number of thermal degrees of freedom: 55
Number of VOF degrees of freedom: 55
Initial refinement in box - Step 2 of 3
Number of active cells: 88
Number of degrees of freedom: 339
Volume of triangulation: 0.36
Number of thermal degrees of freedom: 113
Number of VOF degrees of freedom: 113
Initial refinement in box - Step 3 of 3
Number of active cells: 268
Number of degrees of freedom: 942
Volume of triangulation: 0.36
Number of thermal degrees of freedom: 314
Number of VOF degrees of freedom: 314
Copy link
Contributor

Choose a reason for hiding this comment

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

This part should not be happening in a restart file. Maybe this option just has neverbeen tested with a restart. IF we are restarting, we should not do any mesh adaptation :)

@hepap
Copy link
Collaborator Author

hepap commented Jul 2, 2024

I will fix the restart with the box refinement feature on another branch, and I will come back to this PR so that it stays a coherent PR.

@hepap
Copy link
Collaborator Author

hepap commented Jul 2, 2024

So I removed the box refinement for the new test in this PR, and I fixed the restart problem with box ref in PR #1184.

This PR (#1180) can be merge before PR #1184 (I added a test in PR #1184 to verify restart with box ref).

@hepap hepap added Ready for review and removed WIP When a PR is open but not ready for review labels Jul 2, 2024
@blaisb
Copy link
Contributor

blaisb commented Jul 2, 2024

@hepap can you rebase? Then this will be ready to go :)

Copy link
Collaborator

@AmishgaAlphonius AmishgaAlphonius left a comment

Choose a reason for hiding this comment

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

Great work Hepap! Everything looks good to me! Thanks for the fix! :)

Copy link
Contributor

@blaisb blaisb left a comment

Choose a reason for hiding this comment

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

two minor things after rebase and this is ready to merge :)

applications_tests/lethe-fluid/CMakeLists.txt Outdated Show resolved Hide resolved
applications_tests/lethe-fluid/CMakeLists.txt Outdated Show resolved Hide resolved
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.

None yet

3 participants