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 computation of CFL number #195

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

Conversation

keiyamamo
Copy link
Collaborator

This PR addresses two points in the current CFL number computation.

  1. The minimum cell diameter was computed from rank 0 only in the previous implementation. By using MPI.min, we now get the minimum cell diameter from all the ranks. Technically, we should use the local cell diameter to compute CFL number, but we stay in the safe range by using the minimum cell diameter and also is computationally efficient.

  2. The current implementation does not take velocity degree in account. Although I couldn’t find a formal definition of CFL number with higher order elements, there are some papers addressing order dependency of the CFL number, such as this one. Might not be mathematically perfect way of computing CFL, but this new implementation addresses this issue by using v.ufl_element().degree()

@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.59%. Comparing base (9326a44) to head (1f4bcce).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #195   +/-   ##
=======================================
  Coverage   78.58%   78.59%           
=======================================
  Files          32       32           
  Lines        3848     3849    +1     
=======================================
+ Hits         3024     3025    +1     
  Misses        824      824           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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