Remove unused variables, fix compiler warnings, and standardize constants for -pedantic builds#626
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses compiler warnings (notably when building with -pedantic) by cleaning up unused/conditionally-used locals and tightening some constant declarations.
Changes:
- Removes unused local variables across multiple Fortran modules/programs.
- Moves MPI/HDF5-only locals (e.g.,
ierr, loop indices) behind preprocessor guards to avoid unused-variable warnings in non-MPI/HDF5 builds. - Converts some literal “constants” to
parameterand standardizes a few numeric literals (e.g.,0.01→1.d-2), plus minor formatting tweaks.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Prog/Wrapgr_mod.F90 | Removes unused locals in update/sort routines. |
| Prog/WaveFunction_mod.F90 | Drops unused overlap routine locals. |
| Prog/upgrade_mod.F90 | Removes unused tmp_r local. |
| Prog/udv_state_mod.F90 | Eliminates unused dummy variable; adjusts huge/tiny kind usage; warning message touched region. |
| Prog/tau_p_mod.F90 | Removes unused locals from projector tau routine. |
| Prog/tau_m_mod.F90 | Removes unused locals from finite-T tau routine. |
| Prog/Set_random_mod.F90 | Makes loop index MPI-only to avoid unused warnings without MPI. |
| Prog/Predefined_Trial_mod.F90 | Removes unused locals; standardizes small numeric literals; makes Test a parameter. |
| Prog/Predefined_Obs_mod.F90 | Cleans up unused locals; makes small constant a parameter. |
| Prog/Predefined_Hop_mod.F90 | Cleans up unused locals; converts Zero/Test to parameters in several routines. |
| Prog/Operator_mod.F90 | Makes Zero a parameter; guards ierr declaration with #ifdef MPI. |
| Prog/observables_mod.F90 | Cleans up unused locals; changes lattice/unit-cell dummy intents; removes unused locals in print routines. |
| Prog/main.F90 | Guards HDF5/MPI-only locals (file_dat, Ierr) to avoid unused warnings. |
| Prog/Langevin_HMC_mod.F90 | Removes unused locals from force routine. |
| Prog/Hamiltonians/LRC_mod.F90 | Removes unused locals; makes a test flag a parameter. |
| Prog/Hamiltonian_main_mod.F90 | Removes unused locals from Get_Delta_S0_global_base. |
| Prog/Global_mod.F90 | Removes unused locals in global update/ratio routines. |
| Prog/Fields_mod.F90 | Guards MPI-only locals; removes unused filename variable(s). |
| Prog/DynamicMatrixArray_mod.F90 | Changes dummy intent for stored target object in pushback. |
| Prog/cgr1_mod.F90 | Guards DLJ declaration behind stabilization macros. |
| Libraries/Modules/mpi_shared_mem_mod_v2.F90 | Moves MPI-only locals under #ifdef MPI in deallocation routine. |
| Libraries/Modules/maxent_stoch_mod.F90 | Standardizes small numeric literals; makes Test a parameter; removes unused locals. |
| Libraries/Modules/entanglement_mod.F90 | Reorganizes First_call and MPI-guarded declarations to reduce warnings. |
| Libraries/Modules/alf_hdf5_mod.F90 | Adds a non-HDF5 stub module under #else and fixes module end formatting. |
| configure.sh | Adds an example dev-flag line using -pedantic (commented). |
| Analysis/maxent_wrapper_mod.F90 | Replaces low-precision pi literal; makes Test a parameter; makes Zero a parameter; minor cleanup. |
| Analysis/Max_SAC.F90 | Removes unused locals; makes Test a parameter. |
| Analysis/cov_tau.F90 | Removes unused local; makes Zero a parameter. |
| Analysis/cov_eq.F90 | Removes unused locals and allocations; updates related declarations. |
| Analysis/ana_mod.F90 | Removes unused locals in analysis routines. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…les_mod and DynamicMatrixArray_mod Agent-Logs-Url: https://github.com/ALF-QMC/ALF/sessions/299b5004-ab69-41e9-8c94-0780b75987db Co-authored-by: johanneshofmann87 <129625831+johanneshofmann87@users.noreply.github.com>
… Libraries/Modules/mpi_shared_mem_mod_v2.F90
- Replaced hardcoded small values (e.g., 1.D-10, 1.D-8) with Eps_small for better numerical stability across multiple modules including alf_hdf5_mod, fourier_mod, lattices_v3_mod, maxent_mod, and Hamiltonian modules. - Updated calculations involving pi to use twopi for consistency and clarity. - Removed redundant definitions of pi in various modules, ensuring a single source of truth for mathematical constants. - Enhanced convergence checks in maxent_mod to utilize Eps_convergence for improved iterative processes. - Ensured all modules that require these constants now import them from the Natural_Constants module.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 49 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/ALF-QMC/ALF/sessions/215b5272-2f97-4b91-8bb8-3a19478fd5bf Co-authored-by: johanneshofmann87 <129625831+johanneshofmann87@users.noreply.github.com>
Done in commit 81e7e68. Removed unused
|
|
@copilot update the description of this PR to reflect the actual changes implemented here. |
Updated the PR description to reflect all the changes implemented in this PR, including: unused variable removals, preprocessor-guarded MPI/HDF5 locals, the Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
Removed unused variables and cleaned up declarations:
|
fassaad
left a comment
There was a problem hiding this comment.
There are many changes here, so it would be nice to run as extensive tests as possible.
…f and entanglement_mod.F90
|
I updated the GNU DEVEL config to compile with |
|
@jonasschwab looks like the pgi illegal instructions are back. I only merged with the master to be up-to-date with the base branch... |
Hm, it's very hard to debug if you can't control on which hardware it runs. |
The temporary "fix" that works is to re-build the container images (https://github.com/ALF-QMC/alf-container/actions/workflows/docker-build.yml) and hope that the jobs are picked up by runners wit less "capable" CPUs. I did that now and it worked. |
|
Another issue I discovered with test branch: The merge seems to cause significant deviations in the HMC test between Diff on the info files: Diff on scalar analysis results: Parameter file: |
I noticed that as well and agree that we should identify where this is coming from. @jonasschwab |
fassaad
left a comment
There was a problem hiding this comment.
I cannot go through all the changes. As far as I recall Jonas or Johannes did some extensive automatic checks, and all seemed to work out. There was one exception, but the issue was the art coding of pi=3.1415927 instead of pi = acos(-1.d0) when drawing normal distributed random numbers. Is my recollection correct? If so we can merge.
|
@fassaad @jonasschwab I fixed the merge conflicts, the pipeline is running but should pass now, and auto-merge is enabled. Can one of you quickly approve the merge, please? |
Using the
-pedanticand-Wunused-variableflags, various compiler warnings were cleaned up across the codebase.Unused variables and imports:
ierr, loop indices) behind#ifdef _MPI/#ifdef _HDF5preprocessor guards to avoid warnings in non-MPI/HDF5 builds.pifromNatural_Constantsin files where onlytwopiis actually referenced.Standardized mathematical and numerical constants:
Natural_Constantsmodule withpi = acos(-1.d0),twopi,Eps_machine = 1.D-12,Eps_small = 1.D-10, andEps_convergence = 1.D-6as a single source of truth.piand small tolerance values (e.g.,1.D-10,1.D-8) across the codebase with imports fromNatural_Constants.Other fixes:
INTENT(IN), targettoINTENT(INOUT), targetfor dummy arguments whose addresses are stored in pointers inobservables_modandDynamicMatrixArray_mod, as required by flang. Added inline comments and updated Doxygen@paramtags explaining the rationale.udv_state_mod.F90warning message: "smalles" → "smallest representable value".The
libqrreffiles with non-standardcomplex*16declarations were intentionally left unchanged.