Skip to content

Remove the fluid center-of-mass output, which no case can ask for or turn off - #1873

Open
sbryngelson wants to merge 1 commit into
MFlowCode:masterfrom
sbryngelson:cleanup/remove-com-output
Open

Remove the fluid center-of-mass output, which no case can ask for or turn off#1873
sbryngelson wants to merge 1 commit into
MFlowCode:masterfrom
sbryngelson:cleanup/remove-com-output

Conversation

@sbryngelson

Copy link
Copy Markdown
Member

s_derive_center_of_mass computes each fluid's mass, mass-weighted position and total volume every timestep and writes them to fluid<i>_com.dat. There is no com_wrt parameter — the feature has no switch anywhere in toolchain/mfc/params/, so no case can request it and no case can turn it off. It is instead gated on probe_wrt:

if (proc_rank == 0 .and. probe_wrt) then
    call s_open_probe_files()
    call s_open_com_files()

So every probe-enabled case pays for a full-domain reduction per step — three to five GPU_ATOMIC accumulations per cell per fluid, then up to five MPI_ALLREDUCEs per fluid — and gets fluid<i>_com.dat files in the case directory that nobody asked for. Nothing in examples/, tests/, benchmarks/ or the docs reads them.

Removed: s_open_com_files, s_write_com_files, s_close_com_files, s_derive_center_of_mass, the c_mass array with its GPU_DECLARE/ALLOCATE/DEALLOCATE, and the four call sites. m_derived_variables no longer needs m_mpi_proxy — the allreduces were its only use of it.

Not touched: s_compute_centroid_offset in m_ibm.fpp, whose local center_of_mass is the immersed-body centroid. Different quantity, and it is used.

248 lines net removed. Goldens are unaffected — the CoM files were written to the case root, not D/.

Verified: ./mfc.sh precheck clean; CPU build; 25/25 on the probe, hypoelastic and IBM tests (the probe case 18DB27D5 is the only one that reached this code), and no *_com.dat is produced.

Copilot AI lite review requested due to automatic review settings September 13, 2026 02:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Removes an always-on (when probe_wrt is enabled) center-of-mass (CoM) reduction + file output pipeline that had no user-facing parameter to enable/disable it, eliminating unnecessary per-timestep GPU atomics and MPI reductions and stopping unexpected fluid<i>_com.dat outputs.

Changes:

  • Removed CoM file lifecycle routines (s_open_com_files, s_write_com_files, s_close_com_files) and their call sites.
  • Removed the CoM reduction routine (s_derive_center_of_mass) and associated c_mass storage/allocation.
  • Dropped m_mpi_proxy dependency from m_derived_variables since CoM allreduces were its only use there.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/simulation/m_derived_variables.fpp Removes CoM derivation + I/O calls and simplifies finalization to only close probe files when enabled.
src/simulation/m_data_output.fpp Removes CoM-related public APIs, c_mass storage, and CoM file open/write/close implementations.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

Copy link
Copy Markdown

Lines of Code

File Lines Diff
src/simulation/m_derived_variables.fpp 302 -139
src/simulation/m_data_output.fpp 1306 -61
Directory Lines Diff
simulation 27889 -200
total 46163 -200

@codecov

codecov Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 61.35%. Comparing base (dc0aec1) to head (0ab85c0).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/simulation/m_derived_variables.fpp 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1873      +/-   ##
==========================================
+ Coverage   61.26%   61.35%   +0.08%     
==========================================
  Files          84       84              
  Lines       22330    22215     -115     
  Branches     3265     3254      -11     
==========================================
- Hits        13680    13629      -51     
+ Misses       6207     6151      -56     
+ Partials     2443     2435       -8     

☔ View full report in Codecov by Harness.
📢 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.

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