Skip to content

Fix 3D IBM Infinite CFL Number on GPUs #519

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

Merged
merged 3 commits into from
Jul 13, 2024

Conversation

Sam-Briney
Copy link
Contributor

@Sam-Briney Sam-Briney commented Jul 12, 2024

Description

Fixes 3D IBM infinite CFL number when running on GPUs. These modifications were suggested by @haochey and @anandrdbz via slack

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Scope

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

How Has This Been Tested?

  • Tested by running the 3D_ibm example on two A100 GPUs
    visit0000

Test Configuration:

  • What computers and compilers did you use to test this:

The test was run using Nvidia A100 GPUs on HiPerGator at UF
python/3.8, nvhpc/24.5, openmpi/4.1.6, cmake/3.26.4

Checklist

  • I ran ./mfc.sh format before committing my code
  • This PR does not introduce any repeated code (it follows the [DRY])
  • 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

@Sam-Briney Sam-Briney requested a review from sbryngelson as a code owner July 12, 2024 14:12
@sbryngelson
Copy link
Member

I'm confused by the removal of these:

        if (ib .and. t_step == 1) then
            if (qbmm .and. .not. polytropic) then
                call s_ibm_correct_state(q_cons_ts(1)%vf, q_prim_vf, pb_ts(1)%sf, mv_ts(1)%sf)
            else
                call s_ibm_correct_state(q_cons_ts(1)%vf, q_prim_vf)
            end if
        end if

If you remove all of these then the whole subroutine s_ibm_correct_state can be removed because these are the only places it gets called. What exactly is the purpose of this routine (or removing it?) @anandrdbz @Sam-Briney

@sbryngelson sbryngelson added the enhancement New feature or request label Jul 12, 2024
@sbryngelson sbryngelson requested a review from wilfonba July 12, 2024 14:20
@Sam-Briney
Copy link
Contributor Author

@sbryngelson I think it gets called in a few other places as well. For example:

I believe it was getting called twice instead of just once at the first time step.

@sbryngelson
Copy link
Member

Ah I see, you're correct @Sam-Briney. Does this fix the issue with the visualization inside the IBM being incorrect on >1 GPU?

@Sam-Briney
Copy link
Contributor Author

@sbryngelson It does not fix the visualization issue. I thought it might be better to make a more focused PR to make this functionality available in the short term

@sbryngelson
Copy link
Member

Makes sense. Merging.

Copy link

codecov bot commented Jul 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.44%. Comparing base (78fd0c5) to head (0febbdd).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #519      +/-   ##
==========================================
+ Coverage   58.41%   58.44%   +0.03%     
==========================================
  Files          57       57              
  Lines       14453    14442      -11     
  Branches     1892     1891       -1     
==========================================
- Hits         8443     8441       -2     
+ Misses       5449     5440       -9     
  Partials      561      561              

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

@sbryngelson sbryngelson merged commit 8c7f281 into MFlowCode:master Jul 13, 2024
22 checks passed
AiredaleDev pushed a commit to AiredaleDev/MFC that referenced this pull request Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

2 participants