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

(Atomic) Boundary Condition Patches (pt. 1/2) #737

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

henryleberre
Copy link
Member

@henryleberre henryleberre commented Nov 17, 2024

This part (pt. 1)

  • Removes (net) ~4k LOC by abstracting the iteration over the buffer region and making more code common between the three MFC targets.
  • Adds (opt-in) "Boundary Condition Patches" (cuboid & spheroid).
  • Adds --dry-run to testing so that CI can build all necessary versions of MFC before submitting jobs for testing to a queue system.
  • Fixes an IBM method bug for slip-walls (credit @anshgupta1234).
  • Fixes a CheMFC bug that prevented the use of analytically defined patches.

Checklist for getting out of the "draft" stage

  • Check GPU performance.
  • Verify the boundary patches still work with any number of ranks.
  • Support parallel_io=T.
  • Add spheroid boundary patch code back.
  • Fix the three test cases that fail (some variations of cylindrical coordinates).
  • Documentation.

Next part (pt. 2)

  • All edge cases addressed. That is, bc_[x,y,z] should no longer be used anywhere in the code. One example of such an oversight in pt. 1 is the lack of support for CBCs.

Why two parts?

> 8k LOC changes is a lot to carry around in one atomic commit.

@henryleberre henryleberre added bug Something isn't working or doesn't seem right enhancement New feature or request continuous-integration Continuous integration (CI) labels Nov 17, 2024
@sbryngelson
Copy link
Member

@henryleberre GCBCs from @anandrdbz have been merged #698

@sbryngelson
Copy link
Member

Removes (net) ~4k LOC by abstracting the iteration over the buffer region and making more code common between the three MFC targets.

A true beauty.

@henryleberre henryleberre force-pushed the bc-patches branch 7 times, most recently from 25ed449 to 0144937 Compare November 24, 2024 02:36
@henryleberre henryleberre marked this pull request as ready for review November 24, 2024 03:44
@henryleberre henryleberre force-pushed the bc-patches branch 5 times, most recently from fb841b5 to c393a60 Compare November 26, 2024 00:34
Copy link
Member

@sbryngelson sbryngelson left a comment

Choose a reason for hiding this comment

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

Add to test suite 2D and 3D tests that have square and circle patches on at least one side.

Copy link

codecov bot commented Nov 26, 2024

Codecov Report

Attention: Patch coverage is 56.44444% with 196 lines in your changes missing coverage. Please review.

Project coverage is 44.37%. Comparing base (d1b8028) to head (7912215).

Files with missing lines Patch % Lines
src/common/m_mpi_common.fpp 53.23% 71 Missing and 23 partials ⚠️
src/common/m_boundary_conditions_common.fpp 68.96% 21 Missing and 6 partials ⚠️
src/simulation/m_mpi_proxy.fpp 40.90% 23 Missing and 3 partials ⚠️
src/simulation/m_cbc.fpp 41.66% 8 Missing and 6 partials ⚠️
src/common/m_helper.fpp 22.22% 7 Missing ⚠️
src/simulation/m_start_up.fpp 62.50% 6 Missing ⚠️
src/common/m_checker_common.fpp 16.66% 5 Missing ⚠️
src/simulation/m_viscous.fpp 0.00% 2 Missing and 2 partials ⚠️
src/post_process/m_data_input.fpp 40.00% 3 Missing ⚠️
src/simulation/m_ibm.fpp 25.00% 3 Missing ⚠️
... and 5 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #737      +/-   ##
==========================================
+ Coverage   43.63%   44.37%   +0.73%     
==========================================
  Files          61       63       +2     
  Lines       16893    15320    -1573     
  Branches     1948     1798     -150     
==========================================
- Hits         7372     6798     -574     
+ Misses       8404     7469     -935     
+ Partials     1117     1053      -64     

☔ 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
bug Something isn't working or doesn't seem right continuous-integration Continuous integration (CI) enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

2 participants