search

BoldingBruggeman / getm-rewrite

Rewrite of the General Estuarine Transport Model (GETM) in Python (mostly) and Fortran (performance-critical sections)
https://pygetm.readthedocs.io/
Other
7 stars 3 forks source link

compiler error for MODULE PROCEDURE uv_momentum_2d #4

Closed knutaros closed 3 years ago

knutaros commented 3 years ago

[ 86%] Building Fortran object CMakeFiles/getm_dynamics.dir/src/dynamics/momentum_2d.F90.o /media/work/kklingbe/tools/bb-getm/code/src/dynamics/momentum_2d.F90:24:0:

MODULE PROCEDURE uv_momentum_2d

internal compiler error: in gfc_get_symbol_decl, at fortran/trans-decl.c:1423

this is with GNU Fortran (Debian 6.3.0-18+deb9u1) 6.3.0 20170516

any idea what can cause trouble (despite the old compiler)?

bolding commented 3 years ago

In submodules you can either use 'MODULE PROCEDURE' or 'MODULE SUBROUTINE'. In the former case you don't specify interface information it is picked up from the parent module - in the latter case you need to copy the interface information from the parent module.

Maybe that is the problem - in principle submodules should be supported in gfortran 6.3.0.

knutaros commented 3 years ago

On 10/28/20 1:14 PM, Karsten Bolding wrote:

In submodules you can either use 'MODULE PROCEDURE' or 'MODULE SUBROUTINE'. In the former case you don't specify interface information it is picked up from the parent module - in the latter case you need to copy the interface information from the parent module.

Maybe that is the problem - in principle submodules should be supported in gfortran 6.3.0.

they also worked fine with the advection code, but I do not see a difference in the code. adding IMPLICIT NONE to the submodule procedure also did not help.

knutaros commented 3 years ago

I am wondering about the combination of dimension and T (same for dpdx and dpdy): real(real64), dimension(:,:), intent(in) :: tausx(T),tausy(T) shouldn't it read either real(real64), intent(in) :: tausx(T),tausy(T) or real(real64), dimension(:,:), intent(in) :: tausx,tausy

knutaros commented 3 years ago

in the last post the underbars around T were removed

bolding commented 3 years ago

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70864 mentions a backport done in 2018

I don't have gfortran-6 on my computer but I've checked with:

kb@orca:~/source$ gfortran-7 --version GNU Fortran (Ubuntu 7.5.0-6ubuntu2) 7.5.0

kb@orca:~/source$ gfortran-8 --version GNU Fortran (Ubuntu 8.4.0-3ubuntu2) 8.4.0

kb@orca:~/source$ gfortran-9 --version GNU Fortran (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0

kb@orca:~/source$ gfortran-10 --version GNU Fortran (Ubuntu 10.2.0-5ubuntu1~20.04) 10.2.0

and they all work - so I'll close the issue now.

bolding commented 3 years ago

I did not see your new comment before I closed - sorry - two _ turns into italic in markdown :-)

I don't know if having dimension specified when you give the actual size matters - the compilers did not complain - but it is not necessary

and now I close

knutaros commented 3 years ago

I still suggest to change similar to uv_momentum_3d... (at least then it compiles)

diff --git a/src/dynamics/momentum.F90 b/src/dynamics/momentum.F90 index 07455b2..a44c9e1 100644 --- a/src/dynamics/momentum.F90 +++ b/src/dynamics/momentum.F90 @@ -89,9 +89,9 @@ MODULE getm_momentum        procedure :: configuration => momentum_configuration        procedure :: initialize => momentum_initialize        procedure :: register => momentum_register -      procedure :: uv_momentum_2d => uv_momentum_2d +      procedure :: do_2d => uv_momentum_2d        procedure :: advection_2d => uv_advection_2d -      procedure :: uv_momentum_3d => uv_momentum_3d +      procedure :: do_3d => uv_momentum_3d        procedure :: w_momentum_3d => w_momentum_3d        procedure :: advection_3d => uv_advection_3d        procedure :: vel_2d => velocities_2d @@ -109,9 +109,9 @@ MODULE getm_momentum           real(real64), intent(in) :: dt              !! timestep [s]  #define T self%domain%T%l(1):,self%domain%T%l(2): -         real(real64), dimension(:,:), intent(in) :: tausx(T),tausy(T) +         real(real64), dimension(:,:), intent(in) :: tausx, tausy              !! surface stresses -         real(real64), dimension(:,:), intent(in) :: dpdx(T), dpdy(T) +         real(real64), dimension(:,:), intent(in) :: dpdx, dpdy              !! surface pressure gradient - including air pressure  #undef T        end subroutine uv_momentum_2d

On 10/28/20 5:46 PM, Karsten Bolding wrote:

I did not see your new comment before I closed - sorry - two _ turns into italic in markdown :-)

I don't know if having dimension specified when you give the actual size matters - the compilers did not complain - but it is not necessary

and now I close

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/BoldingBruggeman/getm-rewrite/issues/4#issuecomment-718064369, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2RV6VNDRHJSYDRYRUOLKLSNBDGTANCNFSM4TCGXORA.

bolding commented 3 years ago

we need to specify the array sizes - and we do know them from the grids - as they are not 1-based

the 3D version would give wrong results

bolding commented 3 years ago

I've pushed a consistent treatment of arrays in subroutines