Remove -O0 flag from LDFLAGS
#197
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 task
In `build3.sh`, the `-O0` flag (which disables optimisations) is included in the `LDFLAGS` variable. This change removes the `-O0` flag from `LDFLAGS` so that: 1. Compiler specific flags (non `ld` flags) are removed from `LDFLAGS` 2. `-O0` flag is not applied for only a subset of source files. 3. The Makefile rules can be simplified. See #196 for more details on why the flag should be removed. Regression tests (bitwise comparison of model output via `nccmp`) show that model output is bitwise identical between the current branch and the main branch for serial and MPI model runs. Fixes #196
SeanBryan51
force-pushed
the
196-O0-flag
branch
from
310265a
to
6a76f9e
Compare
2 tasks
SeanBryan51
added a commit
that referenced
this pull request
Feb 28, 2024
`ld` flags are currently specified incorrectly. For example: 1. `-lnetcdf` is not required since it is a dependency of the netCDF Fortran library - only `-lnetcdff` is needed. 2. `ld` flags are being split across two variables: `LDFLAGS` and `LD` when there should only be one (`LD` typically refers to the path to the `ld` executable). Having the flags split across two variables allows for the flags to be passed to the compiler in an inconsistent way. See #198 for more details. This change removes the `LD` variable and any hard-coded linker flags or include flags in the Makefile or build script. Linker flags and include flags are replaced with a query to `pkg-config` so that the correct flags are used. Using `pkg-config` also insulates the build system from changes to library versions and dependencies. ~~This PR should ideally be done after #197 is merged so that we can verify compiled object files have not been altered as a result of this change.~~ Compiled object files are identical between this branch and the main branch. However, the executables differ between the two branches due to changes in the compiler arguments at the linking step. Regression tests using [benchcab](https://github.com/CABLE-LSM/benchcab)* (bitwise comparison of model output via nccmp) show that model output is bitwise identical between the current branch and the main branch for serial and MPI model runs. *executables were built manually for the spatial case (see this related issue: CABLE-LSM/benchcab#244). Fixes #198 <!-- readthedocs-preview cable start --> ---- 📚 Documentation preview 📚: https://cable--199.org.readthedocs.build/en/199/ <!-- readthedocs-preview cable end -->
SeanBryan51
added a commit
that referenced
this pull request
Mar 1, 2024
In preparation for building CABLE with [spack](https://spack.io/), @harshula, @ccarouge and I have found areas where the current build system for CABLE offline can be improved (see #188 for more details). This change adds a [CMake](https://cmake.org/) based build system to address these issues: - #190 Although this problem can be solved in the Makefile by specifying the relative paths to each source file, it requires updating all the Makefile Fortran module dependency rules which is a pain. CMake automatically handles Fortran module dependencies out of the box so doing this with CMake is much easier. - #191 When invoking `cmake`, custom options, compilers and compiler flags can be configurable by setting the appropriate variables via the [`-D` flag](https://cmake.org/cmake/help/latest/manual/cmake.1.html#cmdoption-cmake-D), for example: `cmake -DCMAKE_Fortran_COMPILER="mpif90" -DCMAKE_Fortran_FLAGS="-O2 -fp-model precise" -S . -B build`. These can be used for doing MPI only builds and enabling specific debug flags. - #192 As mentioned above, CMake automatically handles Fortran module dependencies out of the box. Parallelised builds can be done by specifying the `-j` flag to `cmake`. The following pull requests should be merged before this pull request so that we can demonstrate binary equivalence between the executables compiled by both Makefile and CMake builds. This ensures no unwanted changes are introduced by the transition. - [x] #197 - [x] #199 Edit: the above PR's have been merged - the serial and MPI binaries built by CMake (in release mode) are equivalent to the binaries built by the Makefile based build system in the main branch. Fixes #188, #190, #191, #192 <!-- readthedocs-preview cable start --> ---- 📚 Documentation preview 📚: https://cable--200.org.readthedocs.build/en/200/ <!-- readthedocs-preview cable end -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In
build3.sh, the-O0flag (which disables optimisations) is included in theLDFLAGSvariable. This change removes the-O0flag fromLDFLAGSso that:ldflags) are removed fromLDFLAGS-O0flag is not applied for only a subset of source files.See #196 for more details on why the flag should be removed.
Tools for testing binary equivalence (see
ldd,nmandobjdumptools) show that the following build artifacts differ when comparing against the main branch:cable(serial executable)cable-mpi(MPI executable)cable_mpicommon.ocable_mpimaster.ocable_mpiworker.opop_mpi.oRegression tests using benchcab (bitwise comparison of model output via
nccmp) show that model output is bitwise identical between the current branch and the main branch for serial and MPI model runs.Fixes #196
Type of change
📚 Documentation preview 📚: https://cable--197.org.readthedocs.build/en/197/