Skip to content

Remove unused chemistry and capillary declarations - #2022

Draft
sbryngelson wants to merge 2 commits into
MFlowCode:masterfrom
sbryngelson:consolidate/cleanup-fixes
Draft

sbryngelson wants to merge 2 commits into
MFlowCode:masterfrom
sbryngelson:consolidate/cleanup-fixes

Conversation

@sbryngelson

Copy link
Copy Markdown
Member

Summary

Chemistry and capillary routines retained unused declarations and an unused argument. This draft combines the two existing dead-code removals.

Consolidation

Each source change is retained as a separate commit, with its original author, message, and a cherry-pick -x reference. The complete original PR descriptions are reproduced below, including their verification records and limitations. Original review discussions remain available through the source links. Those original verification claims are historical records, not fresh runs on this combined branch.

Verification of this combined branch

  • Every source commit retains exactly the same added and deleted lines, verified against the original commit.
  • git diff --check origin/master...HEAD passes.
  • ./mfc.sh precheck passes all seven gates: formatting, spelling, toolchain lint/tests, source lint, documentation references, parameter documentation, and example case validation.
  • Full solver regressions and hardware-specific compiler/GPU runs were not repeated for this consolidation; their original records are preserved below, and CI should validate the new head.

This consolidation was performed with OpenAI Codex.

Contribution Policy

We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.

All contributions are expected to demonstrate:

  • A clear understanding of the codebase
  • Alignment with product direction
  • Thoughtful reasoning behind changes
  • Evidence of real-world usage or hands-on experience with the problem

If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.


Acknowledgement

  • I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

PR template credit: junegunn

Original PR documentation

#1997: Remove dead locals and unused argument in s_get_capillary

Source: #1997

Original head: 2943123eba93ad7596680865cea52ebf81d3125d

Complete original PR description

isx/isy/isz were set but never read (and the isy guard tested m instead
of n). s_reconstruct_cell_boundary_values_capillary never used norm_dir,
which the caller filled from the loop index i, undefined at that point.

Pure dead-code removal; surface tension tests pass unchanged.


Acknowledgement

  • I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

#1998: Remove dead shadowed variables in m_chemistry

Source: #1998

Original head: f90317771f99eed236a0a17c7fad5737ac977d1c

Complete original PR description

The module-level offsets array (and its GPU_DECLARE) was never used:
s_compute_chemistry_diffusion_flux declares its own local offsets and
copies that in. The local n in the same routine was unused and shadowed
the grid extent n from m_global_parameters.

Dead-code removal only; chemistry tests pass unchanged.


Acknowledgement

  • I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

isx/isy/isz were set but never read (and the isy guard tested m instead
of n). s_reconstruct_cell_boundary_values_capillary never used norm_dir,
which the caller filled from the loop index i, undefined at that point.

Pure dead-code removal; surface tension tests pass unchanged.

(cherry picked from commit 2943123)
The module-level offsets array (and its GPU_DECLARE) was never used:
s_compute_chemistry_diffusion_flux declares its own local offsets and
copies that in. The local n in the same routine was unused and shadowed
the grid extent n from m_global_parameters.

Dead-code removal only; chemistry tests pass unchanged.

(cherry picked from commit f903177)

This branch has not been deployed

No deployments
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.

1 participant