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

Heat equation smoothing of IC #805

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

Conversation

wilfonba
Copy link
Contributor

@wilfonba wilfonba commented Mar 7, 2025

Description

This PR adds heat equation smoothing for ICs.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Scope

  • This PR comprises a set of related changes with a common goal

How Has This Been Tested?

The following figures show a bunch of smoothing configurations, with black borders denoting processor boundaries:

The following examples were ran on Wingtip with 64 CPU ranks:

image image image image image image image image

Checklist

  • I have added comments for the new code
  • I have made corresponding changes to the documentation (docs/)
  • I have added regression tests to the test suite so that people can verify in the future that the feature is behaving as expected
  • I ran ./mfc.sh format before committing my code
  • New and existing tests pass locally with my changes, including with GPU capability enabled (both NVIDIA hardware with NVHPC compilers and AMD hardware with CRAY compilers) and disabled
  • This PR does not introduce any repeated code (it follows the DRY principle)
  • I cannot think of a way to condense this code and reduce any introduced additional line count

If your code changes any code source files (anything in src/simulation)

To make sure the code is performing as expected on GPU devices, I have:

  • Checked that the code compiles using NVHPC compilers
  • Checked that the code compiles using CRAY compilers
  • Ran the code on either V100, A100, or H100 GPUs and ensured the new feature performed as expected (the GPU results match the CPU results)
  • Ran the code on MI200+ GPUs and ensure the new features performed as expected (the GPU results match the CPU results)
  • Ran my code using various numbers of different GPUs (1, 2, and 8, for example) in parallel and made sure that the results scale similarly to what happens if you run without the new code/feature

The following shows strong and weak scaling on Frontier with and without RDMA mpi. Strong scaling was done with a 64m grid cell 3D_performance_test and starts at two ranks and ends at 32 ranks. Weak scaling was done with 8m grid cells per GPU using the same case file, beginning at two ranks and ending at 32 ranks.
image

@wilfonba wilfonba requested review from a team as code owners March 7, 2025 02:40
Copy link

codecov bot commented Mar 7, 2025

Codecov Report

Attention: Patch coverage is 37.19298% with 179 lines in your changes missing coverage. Please review.

Project coverage is 43.67%. Comparing base (d863869) to head (e63a112).

Files with missing lines Patch % Lines
src/common/m_mpi_common.fpp 25.25% 138 Missing and 7 partials ⚠️
src/pre_process/m_perturbation.fpp 58.97% 13 Missing and 3 partials ⚠️
src/pre_process/m_mpi_proxy.fpp 0.00% 6 Missing ⚠️
src/common/m_helper_basic.f90 73.33% 0 Missing and 4 partials ⚠️
src/common/m_variables_conversion.fpp 0.00% 4 Missing ⚠️
src/post_process/m_global_parameters.fpp 0.00% 2 Missing ⚠️
src/pre_process/m_initial_condition.fpp 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #805      +/-   ##
==========================================
- Coverage   43.94%   43.67%   -0.27%     
==========================================
  Files          65       65              
  Lines       19039    19153     +114     
  Branches     2329     2338       +9     
==========================================
  Hits         8366     8366              
- Misses       9262     9361      +99     
- Partials     1411     1426      +15     

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

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wilfonba wilfonba requested a review from sbryngelson as a code owner March 7, 2025 17:36
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.

+700 lines for just this one feature? is there anyway to trim this down?

@wilfonba
Copy link
Contributor Author

wilfonba commented Mar 8, 2025

@sbryngelson, I've addressed your comments. Now that I've modified code in simulation, I'll have to do more extensive testing tomorrow before this can be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants