From 4667563a13934d59d0a667a9c1d88eb79caba700 Mon Sep 17 00:00:00 2001 From: Jorn Bruggeman Date: Wed, 17 Jul 2024 13:54:43 +0100 Subject: [PATCH] add flang support (#79) * test with new flang * test flang on windows * try to ensure we use conda python version * tests: set cmake platform via environment variable * drop cmake platform spec (experiment) * add msbuild to PATH * try conda package vs2022_win-64 for finding rc * use vs developer prompt * simplify rc detection with cmd shell * support test_host exe directly in build dir * drop CMAKE_GENERATOR_PLATFORM if Ninja * drop CMAKE_GENERATOR_TOOLSET * ensure required arguments are always provided to yaml_settings * drop CMAKE_GENERATOR_PLATFORM and CMAKE_GENERATOR_TOOLSET for pyfabm * do not use /lib:static for flang * correct units metadata for bb/passive * simplify run_all_testcases.py * drop DLLEXPORT for parameters (not working anyway with ifort, and tripping up flang) https://community.intel.com/t5/Intel-Fortran-Compiler/Parameter-defined-in-DLL-not-exported/m-p/810478 * work around flang 19 bug that breaks continuation of compiler directives * add CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=TRUE * Revert "correct units metadata for bb/passive" This reverts commit c01455940ec78c680cf99f4e082233deaf3d790a. * bump version to 2.1.2 --- .github/workflows/fabm.yml | 32 +++++++++++ CMakeLists.txt | 2 +- pyproject.toml | 2 +- src/c/fabm_c.F90 | 2 - src/c/variable.F90 | 2 +- src/drivers/python/CMakeLists.txt | 5 +- src/fabm_types.F90 | 81 +++++++++++++--------------- util/developers/run_all_testcases.py | 12 +++-- 8 files changed, 81 insertions(+), 57 deletions(-) diff --git a/.github/workflows/fabm.yml b/.github/workflows/fabm.yml index 3e71639b..78053ab2 100644 --- a/.github/workflows/fabm.yml +++ b/.github/workflows/fabm.yml @@ -217,6 +217,38 @@ jobs: run: | source /home/runner/setenv_AOCC.sh python3 util/developers/run_all_testcases.py pyfabm --show_logs --compiler flang ${{ matrix.pyfabm_cmake_args }} + flang-new: + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.repository + strategy: + matrix: + os: [windows-latest] + fail-fast: false + runs-on: ${{ matrix.os }} + defaults: + run: + shell: cmd /C call {0} + env: + CMAKE_GENERATOR: Ninja + steps: + - uses: conda-incubator/setup-miniconda@v3 + with: + auto-update-conda: true + - name: Install Python dependencies + run: conda install -c conda-forge/label/llvm_dev -c conda-forge pyyaml flang ninja c-compiler + - name: Clone repository + uses: actions/checkout@v4 + with: + submodules: recursive + - name: Run all test cases with host emulators + run: | + set CMAKE_GENERATOR_PLATFORM= + set CMAKE_GENERATOR_TOOLSET= + python util/developers/run_all_testcases.py harness --show_logs --compiler flang-new + - name: Run all test cases with pyfabm + run: | + set CMAKE_GENERATOR_PLATFORM= + set CMAKE_GENERATOR_TOOLSET= + python util/developers/run_all_testcases.py pyfabm --show_logs --compiler flang-new -DCMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=TRUE pyfabm: if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.repository runs-on: ubuntu-latest diff --git a/CMakeLists.txt b/CMakeLists.txt index 0f17ef8f..bf989fb5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5,7 +5,7 @@ cmake_minimum_required(VERSION 3.13) project(fabm - VERSION 2.1.1 + VERSION 2.1.2 DESCRIPTION "Framework for Aquatic Biogeochemical Models" HOMEPAGE_URL https://fabm.net LANGUAGES Fortran diff --git a/pyproject.toml b/pyproject.toml index cddc5855..ff9408b3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "pyfabm" -version = "2.1.1" +version = "2.1.2" authors = [ {name = "Jorn Bruggeman", email = "jorn@bolding-bruggeman.com"}, {name = "Karsten Bolding", email = "karsten@bolding-bruggeman.com"} diff --git a/src/c/fabm_c.F90 b/src/c/fabm_c.F90 index 2e82346a..49f47702 100644 --- a/src/c/fabm_c.F90 +++ b/src/c/fabm_c.F90 @@ -5,8 +5,6 @@ module fabm_c use iso_c_binding, only: c_int, c_char, C_NULL_CHAR, c_f_pointer, c_loc, c_ptr, c_null_ptr, c_funptr, c_f_procpointer - !DIR$ ATTRIBUTES DLLEXPORT :: STATE_VARIABLE,DIAGNOSTIC_VARIABLE,CONSERVED_QUANTITY - use fabm, only: type_fabm_model, type_fabm_variable, fabm_get_version, status_start_done, fabm_create_model use fabm_types, only: rke, attribute_length, type_model_list_node, type_base_model, & factory, type_link, type_link_list, type_internal_variable, type_variable_list, type_variable_node, & diff --git a/src/c/variable.F90 b/src/c/variable.F90 index 750e70be..97285472 100644 --- a/src/c/variable.F90 +++ b/src/c/variable.F90 @@ -105,7 +105,7 @@ function variable_get_no_river_dilution(pvariable) bind(c) result(value) end function variable_get_no_river_dilution function variable_get_no_precipitation_dilution(pvariable) bind(c) result(value) - !DIR$ ATTRIBUTES DLLEXPORT :: variable_get_no_precipitation_dilution + !DIR$ ATTRIBUTES DLLEXPORT :: variable_get_no_precipitation_dilution ! reduced indentation to work around flang bug type (c_ptr), value, intent(in) :: pvariable integer(kind=c_int) :: value diff --git a/src/drivers/python/CMakeLists.txt b/src/drivers/python/CMakeLists.txt index 3ff475e8..8beaf57d 100644 --- a/src/drivers/python/CMakeLists.txt +++ b/src/drivers/python/CMakeLists.txt @@ -12,16 +12,13 @@ project(python_fabm Fortran) if("${CMAKE_Fortran_COMPILER_ID}" STREQUAL "Intel") if(WIN32) set(CMAKE_Fortran_FLAGS_DEBUG "${CMAKE_Fortran_FLAGS_DEBUG} /Od") + add_compile_options("/libs:static") else() # Do not warn about Windows-specific export directives set (CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -diag-disable 7841") endif() endif() -if(MSVC) - add_compile_options("/libs:static") -endif() - set(PYFABM_NAME "fabm" CACHE STRING "Base name of library") set(PYFABM_DIR ${CMAKE_INSTALL_PREFIX} CACHE PATH "Directory to create library in") set(PYFABM_DEFINITIONS "" CACHE STRING "FABM host preprocessor definitions") diff --git a/src/fabm_types.F90 b/src/fabm_types.F90 index 33fac367..42cbc3da 100644 --- a/src/fabm_types.F90 +++ b/src/fabm_types.F90 @@ -1918,7 +1918,7 @@ subroutine add_variable(self, variable, name, units, long_name, missing_value, m ! Ensure that initial value falls within prescribed valid range. if (variable%initial_value < variable%minimum .or. variable%initial_value > variable%maximum) then write (text,*) 'Initial value', variable%initial_value, 'for variable "' // trim(name) // '" lies& - &outside allowed range', variable%minimum, 'to', variable%maximum + & outside allowed range', variable%minimum, 'to', variable%maximum call self%fatal_error('fill_internal_variable', text) end if @@ -2557,6 +2557,32 @@ recursive subroutine register_expression(self, expression) if (associated(self%parent)) call register_expression(self%parent, expression) end subroutine + function get_effective_string(value, default) result(value_) + character(len=*), intent(in), optional :: value + character(len=*), intent(in) :: default + character(len=:), allocatable :: value_ + + if (present(value)) then + value_ = value + else + value_ = default + end if + end function + + function get_effective_display(display, user_created) result(display_) + integer, intent(in), optional :: display + logical, intent(in) :: user_created + integer :: display_ + + if (present(display)) then + display_ = display + elseif (user_created) then + display_ = display_normal + else + display_ = display_inherit + end if + end function + subroutine get_real_parameter(self, value, name, units, long_name, default, scale_factor, minimum, maximum, display) class (type_base_model), intent(inout), target :: self real(rk), intent(inout), target :: value @@ -2565,19 +2591,12 @@ subroutine get_real_parameter(self, value, name, units, long_name, default, scal real(rk), intent(in), optional :: default, scale_factor, minimum, maximum integer, intent(in), optional :: display - integer :: display_ - - if (present(display)) then - display_ = display - elseif (self%user_created) then - display_ = display_normal - else - display_ = display_inherit - end if if (fabm_parameter_pointers) then - call self%parameters%get(value, name, long_name, units, default, minimum, maximum, scale_factor, display=display_) + call self%parameters%get(value, name, get_effective_string(long_name, name), get_effective_string(units, ''), & + default, minimum, maximum, scale_factor, display=get_effective_display(display, self%user_created)) else - value = self%parameters%get_real(name, long_name, units, default, minimum, maximum, scale_factor, display=display_) + value = self%parameters%get_real(name, get_effective_string(long_name, name), get_effective_string(units, ''), & + default, minimum, maximum, scale_factor, display=get_effective_display(display, self%user_created)) end if end subroutine get_real_parameter @@ -2589,17 +2608,9 @@ subroutine get_integer_parameter(self, value, name, units, long_name, default, m integer, intent(in), optional :: default, minimum, maximum type (type_option), intent(in), optional :: options(:) integer, intent(in), optional :: display - - integer :: display_ - - if (present(display)) then - display_ = display - elseif (self%user_created) then - display_ = display_normal - else - display_ = display_inherit - end if - value = self%parameters%get_integer(name, long_name, units, default, minimum, maximum, options, display=display_) + + value = self%parameters%get_integer(name, get_effective_string(long_name, name), units, & + default, minimum, maximum, options, display=get_effective_display(display, self%user_created)) end subroutine get_integer_parameter subroutine get_logical_parameter(self, value, name, units, long_name, default, display) @@ -2610,16 +2621,8 @@ subroutine get_logical_parameter(self, value, name, units, long_name, default, d logical, intent(in), optional :: default integer, intent(in), optional :: display - integer :: display_ - - if (present(display)) then - display_ = display - elseif (self%user_created) then - display_ = display_normal - else - display_ = display_inherit - end if - value = self%parameters%get_logical(name, long_name, default, display=display_) + value = self%parameters%get_logical(name, get_effective_string(long_name, name), & + default, display=get_effective_display(display, self%user_created)) end subroutine get_logical_parameter recursive subroutine get_string_parameter(self, value, name, units, long_name, default, display) @@ -2630,16 +2633,8 @@ recursive subroutine get_string_parameter(self, value, name, units, long_name, d character(len=*), intent(in), optional :: default integer, intent(in), optional :: display - integer :: display_ - - if (present(display)) then - display_ = display - elseif (self%user_created) then - display_ = display_normal - else - display_ = display_inherit - end if - value = self%parameters%get_string(name, long_name, units, default, display=display_) + value = self%parameters%get_string(name, get_effective_string(long_name, name), units, & + default, display=get_effective_display(display, self%user_created)) end subroutine get_string_parameter function find_object(self, name, recursive, exact) result(object) diff --git a/util/developers/run_all_testcases.py b/util/developers/run_all_testcases.py index 77ffa8e0..35860432 100755 --- a/util/developers/run_all_testcases.py +++ b/util/developers/run_all_testcases.py @@ -105,7 +105,7 @@ def cmake( shutil.rmtree(build_dir) os.mkdir(build_dir) - if os.name == "nt": + if os.name == "nt" and os.environ.get("CMAKE_GENERATOR", "") != "Ninja": x64 = sys.maxsize > 2**32 cmake_arguments = ["-A", "x64" if x64 else "Win32"] + cmake_arguments @@ -452,14 +452,16 @@ def test_harness(args, testcases: Mapping[str, str]): ) if not success: continue - exe = os.path.join( - build_dir, "Debug/test_host.exe" if os.name == "nt" else "test_host" - ) + exe = "test_host" + if os.name == "nt": + exe += ".exe" + if not os.path.isfile(os.path.join(build_dir, exe)): + exe = os.path.join("Debug", exe) for case, path in testcases.items(): shutil.copy(path, os.path.join(run_dir, "fabm.yaml")) run( f"test_harness/{host}/{case}", - [exe, "--simulate", "-n", "10"], + [os.path.join(build_dir, exe), "--simulate", "-n", "10"], cwd=run_dir, )