Fix masked variables from parent scope #959

sbryngelson · 2025-07-19T23:26:09Z

User description

For a long time, we have had the issue of masked variables from the parent scope. I'm not sure if this has caused bugs or not, but it certainly isn't good practice. This PR fixes the issue.

PR Type

Bug fix, Documentation

Description

• Fixed variable masking issues across multiple modules by renaming parameters and local variables to avoid conflicts with parent scope
• Renamed parameters like pb to pb_in, mv to mv_in, and added _local or _in suffixes to prevent variable shadowing
• Updated function signatures and parameter names in critical modules including RHS computation, boundary conditions, IBM, bubble dynamics, and MPI communication
• Fixed parameter masking in time stepping, data output, and finite difference computation routines
• Updated documentation formatting and fixed typos in agent rules file

Diagram Walkthrough

flowchart LR
  A["Global Variables"] --> B["Function Parameters"]
  B --> C["Parameter Renaming"]
  C --> D["pb → pb_in"]
  C --> E["mv → mv_in"] 
  C --> F["local vars → *_local"]
  D --> G["Fixed Masking"]
  E --> G
  F --> G

File Walkthrough

Relevant files

Bug fix

15 files

m_patches.fpp `Fix variable masking in patch geometry subroutines` src/pre_process/m_patches.fpp • Renamed parameter `ib` to `ib_flag` in multiple subroutines to avoid variable name conflicts • Renamed variable `ca` to `ca_in` in airfoil-related subroutines to prevent masking • Renamed local variables like `x_centroid`, `y_centroid`, `eta`, `smooth_coeff` to include `_local` suffix • Renamed `non_axis_sym` to `non_axis_sym_in` to avoid scope conflicts	+96/-96
m_boundary_common.fpp `Resolve parameter name conflicts in boundary condition routines` src/common/m_boundary_common.fpp • Renamed parameters `pb` and `mv` to `pb_in` and `mv_in` in multiple boundary condition subroutines • Updated all references to use the new parameter names throughout the file • Renamed `old_grid` parameter to `old_grid_in` in boundary condition file writing function	+102/-102
m_ibm.fpp `Fix variable masking in immersed boundary method functions` src/simulation/m_ibm.fpp • Renamed `pb` and `mv` parameters to `pb_in` and `mv_in` in IBM correction subroutines • Renamed function parameters like `ghost_points`, `levelset`, `levelset_norm` with `_in` suffix • Updated variable names like `num_gps` to `num_gps_out` for output parameters	+70/-70
m_rhs.fpp `Resolve variable name masking in RHS computation module` src/simulation/m_rhs.fpp • Renamed `pb` and `mv` parameters to `pb_in` and `mv_in` in RHS computation functions • Renamed `flux_src_n` parameter to `flux_src_n_in` to avoid conflicts • Fixed incorrect variable reference in additional physics RHS computation	+35/-34
m_helper.fpp `Fix variable name conflicts in helper utility functions` src/common/m_helper.fpp • Renamed local variables `m`, `n` to `local_m`, `local_n` in matrix printing function • Renamed function parameters like `weight`, `R0` to `local_weight`, `local_R0` • Renamed variables in mathematical functions to avoid conflicts with global scope • Updated parameter names in transformation and polynomial functions	+69/-69
m_mpi_common.fpp `Resolve variable masking in MPI communication routines` src/common/m_mpi_common.fpp • Renamed `ierr` to `local_ierr` in MPI communication functions • Renamed `pb` and `mv` parameters to `pb_in` and `mv_in` in MPI buffer operations • Updated all references to use the new parameter names consistently	+18/-18
m_start_up.fpp `Fix parameter name masking in startup data reading functions` src/pre_process/m_start_up.fpp • Renamed function parameters `q_cons_vf` and `ib_markers` to include `_in` suffix • Updated parameter names in both serial and parallel IC data file reading functions • Maintained consistency in parameter naming across abstract interface definitions	+13/-13
m_bubbles_EE.fpp `Fix variable masking in bubble computation subroutines` src/simulation/m_bubbles_EE.fpp • Renamed parameter `divu` to `divu_in` in subroutine `s_compute_bubbles_EE_rhs` to avoid variable masking • Added `divu_in` parameter to `s_compute_bubble_EE_source` subroutine signature • Renamed local variables `pb` and `mv` to `pb_local` and `mv_local` to prevent scope masking • Updated all references to use the new parameter and variable names	+23/-22
m_hyperelastic.fpp `Rename btensor parameter to prevent variable masking` src/simulation/m_hyperelastic.fpp • Renamed parameter `btensor` to `btensor_in` in both `s_neoHookean_cauchy_solver` and `s_Mooney_Rivlin_cauchy_solver` • Updated all references to use the new parameter name `btensor_in` • Maintains same functionality while avoiding variable name conflicts	+14/-14
m_bubbles_EL.fpp `Fix omegaN variable masking in bubble frequency calculation` src/simulation/m_bubbles_EL.fpp • Renamed local variable `omegaN` to `omegaN_local` to avoid masking from parent scope • Updated all references to use the new variable name	+5/-5
m_bubbles.fpp `Rename parameters in bubble wall property subroutine` src/simulation/m_bubbles.fpp • Renamed parameter `pb` to `pb_in` in `s_bwproperty` subroutine • Renamed output parameters to include `_out` suffix: `chi_vw_out`, `k_mw_out`, `rho_mw_out` • Updated all references to use the new parameter names	+10/-10
m_data_output.fpp `Fix parameter masking in center of mass output subroutine` src/simulation/m_data_output.fpp • Removed empty line at beginning of file • Renamed parameter `c_mass` to `c_mass_in` in `s_write_com_files` subroutine • Updated all references to use the new parameter name	+14/-15
m_time_steppers.fpp `Fix parameter masking in time stepping subroutines` src/simulation/m_time_steppers.fpp • Added `divu` parameter to `s_compute_bubble_EE_source` function call • Renamed parameters in `s_apply_bodyforces` from `q_prim_vf` and `rhs_vf` to `q_prim_vf_in` and `rhs_vf_in` • Updated all references to use the new parameter names	+6/-6
m_data_input.f90 `Rename start_idx parameter to prevent variable masking` src/post_process/m_data_input.f90 • Renamed parameter `start_idx` to `local_start_idx` in `s_allocate_field_arrays` subroutine • Updated all references to use the new parameter name	+6/-6
m_finite_differences.fpp `Fix buffer size parameter masking in finite differences` src/common/m_finite_differences.fpp • Renamed parameter `buff_size` to `local_buff_size` in `s_compute_finite_difference_coefficients` • Updated array dimension declaration to use the new parameter name	+3/-3

Documentation

1 files

mfc-agent-rules.mdc `Update documentation formatting and fix typos in agent rules` .cursor/rules/mfc-agent-rules.mdc • Fixed YAML front matter formatting by changing dashes to proper YAML format • Fixed typo: "respository" to "repository" • Reorganized sections and improved GPU programming guidelines documentation • Enhanced error handling documentation with clearer examples	+37/-38

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull Request Overview

This PR fixes instances of variable masking from parent scope throughout the MFC codebase. Variable masking occurs when local variables or parameters in subroutines have the same name as variables from the module or parent scope, which can lead to confusion and potential bugs. The changes rename these variables with more descriptive names (often adding suffixes like _in, _out, _local) to avoid masking.

Renames procedure parameters to avoid masking module-level variables
Updates local variable names to prevent conflicts with parent scope
Ensures consistent naming patterns while maintaining functionality

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/simulation/m_time_steppers.fpp	Renames parameters in bubble computation and body forces routines
src/simulation/m_rhs.fpp	Updates parameter names in RHS computation and physics routines
src/simulation/m_ibm.fpp	Renames variables in IBM state correction and ghost point routines
src/simulation/m_hyperelastic.fpp	Updates tensor parameter names in stress computation
src/simulation/m_data_output.fpp	Renames center of mass parameter and removes blank line
src/simulation/m_bubbles_EL.fpp	Updates local variable name in bubble frequency computation
src/simulation/m_bubbles_EE.fpp	Renames parameters in bubble source computation
src/simulation/m_bubbles.fpp	Updates bubble property computation parameter names
src/pre_process/m_start_up.fpp	Renames parameters in data file reading routines
src/pre_process/m_patches.fpp	Updates variable names in patch geometry routines
src/post_process/m_data_input.f90	Renames array allocation parameter
src/common/m_mpi_common.fpp	Updates MPI communication parameter names
src/common/m_helper.fpp	Renames various utility function parameters
src/common/m_finite_differences.fpp	Updates finite difference parameter name
src/common/m_boundary_common.fpp	Renames boundary condition parameter names
.cursor/rules/mfc-agent-rules.mdc	Minor formatting and documentation updates

Comments suppressed due to low confidence (2)

src/simulation/m_bubbles_EL.fpp:296

[nitpick] The variable name 'omegaN_local' follows an inconsistent naming pattern. Consider using 'omega_n_local' to align with Fortran naming conventions.

        real(wp) :: omegaN_local, PeG, PeT, rhol, pcrit, qv, gamma, pi_inf, dynP

src/pre_process/m_patches.fpp

src/common/m_helper.fpp

qodo-merge-pro · 2025-07-19T23:26:57Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Variable Masking The variable `ca` is renamed to `ca_in` to avoid masking, but this creates potential inconsistency if other parts of the codebase still reference the original variable name. Need to verify all references are updated consistently across the entire codebase. ca_in = patch_ib(patch_id)%c pa = patch_ib(patch_id)%p Parameter Renaming Parameters `pb` and `mv` are renamed to `pb_in` and `mv_in` throughout multiple subroutines. This is a widespread change that could introduce bugs if any calls to these subroutines weren't updated to match the new parameter names. real(wp), optional, dimension(idwbuff(1)%beg:, idwbuff(2)%beg:, idwbuff(3)%beg:, 1:, 1:), intent(inout) :: pb_in, mv_in type(integer_field), dimension(1:num_dims, -1:1), intent(in) :: bc_type Variable Shadowing The variable `ierr` is renamed to `local_ierr` to avoid masking. However, need to ensure this doesn't conflict with existing error handling patterns and that all MPI error checking still functions correctly. integer :: i, local_ierr integer, allocatable :: recounts(:), displs(:)

qodo-merge-pro · 2025-07-19T23:27:56Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Add consistent local variable naming The variable `radius` is not renamed with a `_local` suffix like other local variables in this scope. This inconsistency could lead to confusion about variable scope and potential conflicts with module-level variables. src/pre_process/m_patches.fpp [1420-1421] -real(wp) :: radius, x_centroid_local, y_centroid_local, z_centroid_local, eta_local, smooth_coeff_local +real(wp) :: radius_local, x_centroid_local, y_centroid_local, z_centroid_local, eta_local, smooth_coeff_local logical :: non_axis_sym_in Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that `radius` was not renamed to `radius_local` for consistency with other local variables in the same scope, which improves code clarity and maintainability.	Low
General	Rename radius to radius_local consistently The variable `radius` should be renamed to `radius_local` to maintain consistency with the local variable naming pattern and avoid potential conflicts with module-level variables. src/pre_process/m_patches.fpp [1431-1479] -radius = patch_icpp(patch_id)%radius +radius_local = patch_icpp(patch_id)%radius ... r = sqrt((x_cc(i) - x_centroid_local)2 + (cart_y - y_centroid_local)2 + (cart_z - z_centroid_local)*2) + eps ... -r - as(2)Ps(2) - as(3)Ps(3) - as(4)Ps(4) - as(5)Ps(5) - as(6)Ps(6) - as(7)Ps(7) <= radius +r - as(2)Ps(2) - as(3)Ps(3) - as(4)Ps(4) - as(5)Ps(5) - as(6)Ps(6) - as(7)*Ps(7) <= radius_local Suggestion importance[1-10]: 6 __ Why: This suggestion correctly points out the usages of the `radius` variable that should be renamed to `radius_local` for consistency, following on from the variable declaration.	Low
Update

codecov · 2025-07-20T08:40:49Z

Codecov Report

Attention: Patch coverage is 25.31328% with 298 lines in your changes missing coverage. Please review.

Project coverage is 44.08%. Comparing base (658f188) to head (71a9bf0).
Report is 9 commits behind head on master.

Files with missing lines	Patch %	Lines
src/common/m_boundary_common.fpp	16.45%	65 Missing and 1 partial ⚠️
src/pre_process/m_patches.fpp	28.57%	43 Missing and 12 partials ⚠️
src/common/m_helper.fpp	20.00%	40 Missing ⚠️
src/simulation/m_ibm.fpp	38.18%	33 Missing and 1 partial ⚠️
src/simulation/m_rhs.fpp	24.13%	22 Missing ⚠️
src/simulation/m_data_output.fpp	18.18%	18 Missing ⚠️
src/simulation/m_hyperelastic.fpp	0.00%	18 Missing ⚠️
src/common/m_mpi_common.fpp	7.14%	12 Missing and 1 partial ⚠️
src/simulation/m_bubbles_EE.fpp	35.29%	11 Missing ⚠️
src/pre_process/m_start_up.fpp	0.00%	6 Missing ⚠️
... and 7 more

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #959   +/-   ##
=======================================
  Coverage   44.08%   44.08%           
=======================================
  Files          69       69           
  Lines       19573    19573           
  Branches     2428     2428           
=======================================
  Hits         8628     8628           
  Misses       9444     9444           
  Partials     1501     1501

☔ 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.

sbryngelson and others added 9 commits July 19, 2025 09:57

update cursor rules.

736abd6

update

a35a3a5

typos

ed33816

Merge branch 'master' into cursor-rules

58110c5

Apply suggestion from @Copilot

3c10b13

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update mfc-agent-rules.mdc

2a84897

Update mfc-agent-rules.mdc

e8353f7

Apply suggestion from @Copilot

2b8c796

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix masked variables

6c7183a

Copilot AI review requested due to automatic review settings July 19, 2025 23:26

sbryngelson added the enhancement New feature or request label Jul 19, 2025

sbryngelson requested a review from a team as a code owner July 19, 2025 23:26

Copilot AI reviewed Jul 19, 2025

View reviewed changes

src/pre_process/m_patches.fpp Outdated Show resolved Hide resolved

src/pre_process/m_patches.fpp Show resolved Hide resolved

src/common/m_helper.fpp Show resolved Hide resolved

qodo-merge-pro bot added the Review effort 4/5 label Jul 19, 2025

sbryngelson and others added 4 commits July 19, 2025 18:34

vscode

2009032

vscode

c8ca9bc

Update m_rhs.fpp

69d1689

fix

32f8c09

sbryngelson added 4 commits July 20, 2025 09:29

fix ierr

8a09731

fix

87076dc

fix capillary spelling

e8bc4d3

fix

71a9bf0

sbryngelson merged commit 45ff24a into MFlowCode:master Jul 21, 2025
31 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix masked variables from parent scope #959

Fix masked variables from parent scope #959

Uh oh!

sbryngelson commented Jul 19, 2025 •

edited by qodo-merge-pro bot

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-merge-pro bot commented Jul 19, 2025

Uh oh!

qodo-merge-pro bot commented Jul 19, 2025 •

edited

Loading

Uh oh!

codecov bot commented Jul 20, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Fix masked variables from parent scope #959

Fix masked variables from parent scope #959

Uh oh!

Conversation

sbryngelson commented Jul 19, 2025 • edited by qodo-merge-pro bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-merge-pro bot commented Jul 19, 2025

PR Reviewer Guide 🔍

Uh oh!

qodo-merge-pro bot commented Jul 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

codecov bot commented Jul 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

sbryngelson commented Jul 19, 2025 •

edited by qodo-merge-pro bot

Loading

qodo-merge-pro bot commented Jul 19, 2025 •

edited

Loading

codecov bot commented Jul 20, 2025 •

edited

Loading