Skip to content

Conversation

@henryleberre
Copy link
Member

@henryleberre henryleberre commented Oct 3, 2024

Description

Fixes the build issue preventing #638. My comment (#638 (comment)) from said PR:

Haha, thanks. To be honest, I am not sure why this was not a problem in the past. When we added support for CCE and built Hipfort, there was a bug that required us to manually add its modules directory (the one containing .mod files) to our include path. I did this using CMAKE_Fortran_MODULE_DIRECTORY without realizing the full scope of its effects, and applied this on all platforms and all targets (no matter whether Hipfort was needed or not). Had CMAKE_Fortran_MODULE_DIRECTORY done what I thought it did, I could have gotten away with this.

In reality, CMAKE_Fortran_MODULE_DIRECTORY sets the directory where the compiler will create .mod files. So, no matter the target (pre_process, simulation, or post_process) and no matter the build flags (--mpi, --gpu, --debug, --case-optimization, ...), the SAME directory (build/staging/hipfort/include/hipfort/amdgcn) would be used to keep these files - which is, let's say, somewhat undesirable and not something CMake encourages.

What was happening in your case when you built post_process after simulation is that the modules whose names are shared between the two (like m_global_parameters containing the startx symbol..), would overwrite the .mod files from simulation with those from post_process since they share the same module directory and the same module name. Then, when you modified m_weno, CMake regenerates the f90 source using Fypp and then tries to build it. In doing so, to process use directives in that file, the compiler would leverage the .mod files already present in the folder, without knowing they were actually from the wrong program! Hence, the very strange error messages, and the requirement to build simulation, then post_process, then simulation to trigger the bug.

I found this quickly because post_process changing how simulation (doesn't) build is something that should never happen and hints towards them sharing something they shouldn't be. I made copies of my build folder until I reproduced the error, and simply diff'ed one where it worked and one where it didn't. From there, the Hipfort issue was obvious:

$ diff --recursive --brief build-good-space/ build-fail-sim/
Only in build-fail-sim/install: 03b34a2688
Files build-good-space/install/hipfort/include/hipfort/amdgcn/m_compile_specific.mod and >build-fail-sim/install/hipfort/include/hipfort/amdgcn/m_compile_specific.mod differ
Only in build-fail-sim/install/hipfort/include/hipfort/amdgcn: m_data_input.mod
Files build-good-space/install/hipfort/include/hipfort/amdgcn/m_data_output.mod and build->fail-sim/install/hipfort/include/hipfort/amdgcn/m_data_output.mod differ
Files build-good-space/install/hipfort/include/hipfort/amdgcn/m_derived_variables.mod and >build-fail-sim/install/hipfort/include/hipfort/amdgcn/m_derived_variables.mod differ
Files build-good-space/install/hipfort/include/hipfort/amdgcn/m_finite_differences.mod and >build-fail-sim/install/hipfort/include/hipfort/amdgcn/m_finite_differences.mod differ
Files build-good-space/install/hipfort/include/hipfort/amdgcn/m_global_parameters.mod and >build-fail-sim/install/hipfort/include/hipfort/amdgcn/m_global_parameters.mod differ
Files build-good-space/install/hipfort/include/hipfort/amdgcn/m_helper.mod and build-fail->sim/install/hipfort/include/hipfort/amdgcn/m_helper.mod differ
Files build-good-space/install/hipfort/include/hipfort/amdgcn/m_mpi_common.mod and build->fail-sim/install/hipfort/include/hipfort/amdgcn/m_mpi_common.mod differ
Files build-good-space/install/hipfort/include/hipfort/amdgcn/m_mpi_proxy.mod and build->fail-sim/install/hipfort/include/hipfort/amdgcn/m_mpi_proxy.mod differ
Files build-good-space/install/hipfort/include/hipfort/amdgcn/m_phase_change.mod and build->fail-sim/install/hipfort/include/hipfort/amdgcn/m_phase_change.mod differ
Files build-good-space/install/hipfort/include/hipfort/amdgcn/m_start_up.mod and build-fail->sim/install/hipfort/include/hipfort/amdgcn/m_start_up.mod differ
Files build-good-space/install/hipfort/include/hipfort/amdgcn/m_variables_conversion.mod and >build-fail-sim/install/hipfort/include/hipfort/amdgcn/m_variables_conversion.mod differ
Only in build-good-space/install/hipfort/include/hipfort/amdgcn: m_weno.mod
Only in build-fail-sim/staging: 03b34a2688
Only in build-fail-sim/staging/98998883b5/CMakeFiles: Progress
Only in build-good-space/staging/98998883b5/CMakeFiles/simulation.dir/fypp/simulation: >m_weno.fpp.f90.o
Files build-good-space/staging/98998883b5/fypp/simulation/m_weno.fpp.f90 and build-fail->sim/staging/98998883b5/fypp/simulation/m_weno.fpp.f90 differ
Only in build-fail-sim/staging: hdf5
Only in build-fail-sim/staging: silo

Fortunately, logging into Frontier, I saw that removing this hack no longer breaks builds.

@henryleberre henryleberre mentioned this pull request Oct 3, 2024
22 tasks
@sbryngelson
Copy link
Member

merge when ci passes

@codecov
Copy link

codecov bot commented Oct 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.38%. Comparing base (07fd919) to head (58ad244).
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #640   +/-   ##
=======================================
  Coverage   54.38%   54.38%           
=======================================
  Files          61       61           
  Lines       13751    13751           
  Branches     1720     1720           
=======================================
  Hits         7478     7478           
  Misses       5817     5817           
  Partials      456      456           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sbryngelson sbryngelson merged commit 75f5e3b into MFlowCode:master Oct 4, 2024
23 checks passed
@sbryngelson sbryngelson deleted the fix-build-reg branch October 4, 2024 12:08
okBrian pushed a commit to okBrian/MFC that referenced this pull request Oct 18, 2024
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