-
Notifications
You must be signed in to change notification settings - Fork 121
Fix masked variables from parent scope #959
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
Conversation
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||
Codecov ReportAttention: Patch coverage is
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. 🚀 New features to boost your workflow:
|
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
pbtopb_in,mvtomv_in, and added_localor_insuffixes 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
File Walkthrough
15 files
m_patches.fpp
Fix variable masking in patch geometry subroutinessrc/pre_process/m_patches.fpp
• Renamed parameter
ibtoib_flagin multiple subroutines to avoidvariable name conflicts
• Renamed variable
catoca_ininairfoil-related subroutines to prevent masking
• Renamed local
variables like
x_centroid,y_centroid,eta,smooth_coeffto include_localsuffix• Renamed
non_axis_symtonon_axis_sym_into avoid scopeconflicts
m_boundary_common.fpp
Resolve parameter name conflicts in boundary condition routinessrc/common/m_boundary_common.fpp
• Renamed parameters
pbandmvtopb_inandmv_inin multiple boundarycondition subroutines
• Updated all references to use the new
parameter names throughout the file
• Renamed
old_gridparameter toold_grid_inin boundary condition file writing functionm_ibm.fpp
Fix variable masking in immersed boundary method functionssrc/simulation/m_ibm.fpp
• Renamed
pbandmvparameters topb_inandmv_inin IBM correctionsubroutines
• Renamed function parameters like
ghost_points,levelset,levelset_normwith_insuffix• Updated variable names like
num_gpstonum_gps_outfor output parametersm_rhs.fpp
Resolve variable name masking in RHS computation modulesrc/simulation/m_rhs.fpp
• Renamed
pbandmvparameters topb_inandmv_inin RHS computationfunctions
• Renamed
flux_src_nparameter toflux_src_n_into avoidconflicts
• Fixed incorrect variable reference in additional physics
RHS computation
m_helper.fpp
Fix variable name conflicts in helper utility functionssrc/common/m_helper.fpp
• Renamed local variables
m,ntolocal_m,local_nin matrix printingfunction
• Renamed function parameters like
weight,R0tolocal_weight,local_R0• Renamed variables in mathematical functions
to avoid conflicts with global scope
• Updated parameter names in
transformation and polynomial functions
m_mpi_common.fpp
Resolve variable masking in MPI communication routinessrc/common/m_mpi_common.fpp
• Renamed
ierrtolocal_ierrin MPI communication functions• Renamed
pbandmvparameters topb_inandmv_inin MPI buffer operations•
Updated all references to use the new parameter names consistently
m_start_up.fpp
Fix parameter name masking in startup data reading functionssrc/pre_process/m_start_up.fpp
• Renamed function parameters
q_cons_vfandib_markersto include_insuffix
• Updated parameter names in both serial and parallel IC data
file reading functions
• Maintained consistency in parameter naming
across abstract interface definitions
m_bubbles_EE.fpp
Fix variable masking in bubble computation subroutinessrc/simulation/m_bubbles_EE.fpp
• Renamed parameter
divutodivu_inin subroutines_compute_bubbles_EE_rhsto avoid variable masking• Added
divu_inparameter to
s_compute_bubble_EE_sourcesubroutine signature• Renamed
local variables
pbandmvtopb_localandmv_localto prevent scopemasking
• Updated all references to use the new parameter and variable
names
m_hyperelastic.fpp
Rename btensor parameter to prevent variable maskingsrc/simulation/m_hyperelastic.fpp
• Renamed parameter
btensortobtensor_inin boths_neoHookean_cauchy_solverands_Mooney_Rivlin_cauchy_solver• Updated
all references to use the new parameter name
btensor_in• Maintains
same functionality while avoiding variable name conflicts
m_bubbles_EL.fpp
Fix omegaN variable masking in bubble frequency calculationsrc/simulation/m_bubbles_EL.fpp
• Renamed local variable
omegaNtoomegaN_localto avoid masking fromparent scope
• Updated all references to use the new variable name
m_bubbles.fpp
Rename parameters in bubble wall property subroutinesrc/simulation/m_bubbles.fpp
• Renamed parameter
pbtopb_inins_bwpropertysubroutine• Renamed
output parameters to include
_outsuffix:chi_vw_out,k_mw_out,rho_mw_out• Updated all references to use the new parameter names
m_data_output.fpp
Fix parameter masking in center of mass output subroutinesrc/simulation/m_data_output.fpp
• Removed empty line at beginning of file
• Renamed parameter
c_massto
c_mass_inins_write_com_filessubroutine• Updated all references
to use the new parameter name
m_time_steppers.fpp
Fix parameter masking in time stepping subroutinessrc/simulation/m_time_steppers.fpp
• Added
divuparameter tos_compute_bubble_EE_sourcefunction call•
Renamed parameters in
s_apply_bodyforcesfromq_prim_vfandrhs_vftoq_prim_vf_inandrhs_vf_in• Updated all references to use the new
parameter names
m_data_input.f90
Rename start_idx parameter to prevent variable maskingsrc/post_process/m_data_input.f90
• Renamed parameter
start_idxtolocal_start_idxins_allocate_field_arrayssubroutine• Updated all references to use the
new parameter name
m_finite_differences.fpp
Fix buffer size parameter masking in finite differencessrc/common/m_finite_differences.fpp
• Renamed parameter
buff_sizetolocal_buff_sizeins_compute_finite_difference_coefficients• Updated array dimension
declaration to use the new parameter name
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