Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data type mismatch in physics/module_sf_noahmplsm.f90 #750

Closed
uturuncoglu opened this issue Oct 5, 2021 · 15 comments · Fixed by #752 or #766
Closed

Data type mismatch in physics/module_sf_noahmplsm.f90 #750

uturuncoglu opened this issue Oct 5, 2021 · 15 comments · Fixed by #752 or #766
Labels

Comments

@uturuncoglu
Copy link
Contributor

Description

There are couple of data type inconsistency in the module_sf_noahmplsm.f90 file, which is raised when I compile that file outside of the CCPP since I was trying to define NoahMP as a separate land component under UFS. One of the compiler error is something like following,

/glade/work/turuncu/NOAHMP/ufs-weather-model/FV3/ccpp/physics/physics/module_sf_noahmplsm.f90(774): error #6633: The type of the actual argument differs from the type of the dummy argument.   [LAISUN]
                 tauss  ,laisun ,laisha ,rb , errmsg ,errflg ,    & !inout
-------------------------^
/glade/work/turuncu/NOAHMP/ufs-weather-model/FV3/ccpp/physics/physics/module_sf_noahmplsm.f90(774): error #6633: The type of the actual argument differs from the type of the dummy argument.   [LAISHA]
                 tauss  ,laisun ,laisha ,rb , errmsg ,errflg ,    & !inout
  1. Specifically, the subroutine energy() call in physics/module_sf_noahmplsm.f90 uses real data type for following variables (around line 1601, tauss ,laisun ,laisha ,rb ,errmsg ,errflg,) but if you trace it to the actual call call energy around line 756, those variables are defined as real(kind=kind_phys).

  2. I also need to change alog function calls to dlog (the double precision one) in the following statement (around line 5871, subroutine frh2o) to be able to compile.

          df = dlog ( ( parameters%psisat(isoil) * grav / hfus ) * ( ( 1. + ck * swl )**2.) * &
               ( parameters%smcmax(isoil) / (smc - swl) )** bx) - dlog ( - (               &
               tkelv - tfrz)/ tkelv)

Steps to Reproduce

Try to compile module_sf_noahmplsm.f90 outside of CCPP.

Additional Context

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

  • Machine: NCAR's Cheyenne
  • Compiler: Intel Compiler (module load intel/2021.2)
  • Suite Definition File or Scheme: N/A
  • Reference other issues or PRs in other repositories that this is related to, and how they are related.

Output

Please include any relevant log files, screenshots or other output here.

@uturuncoglu uturuncoglu added the bug label Oct 5, 2021
@climbfuji
Copy link
Collaborator

Thanks for the bug report.

Regarding 1. This is one file of many that still needs to be cleaned up. Everything should be real(kind_phys), which is hardcoded to double precision (until we start working on compiling the physics in single precision, which will require identifying computations that need to be in double precision). You can probably get around this by defining kind_phys as double precision if it isn't already, and promoting all other reals to double precision (as it is the case in the UFS and the SCM).

Regarding 2. Please do not replace one stone age piece of code (alog) with another one (dlog). Use log and it will work for whatever type the argument is.

@uturuncoglu
Copy link
Contributor Author

@climbfuji Yes, as you already suggested, i solved the issue in my side by adding real(kind_phys) for those arguments. For the issue (2), I'll follow your suggestion. Thanks for your help.

@uturuncoglu
Copy link
Contributor Author

@climbfuji I am also getting error from following statements in ccpp/physics/physics/set_soilveg.f,

      NAMELIST /SOIL_VEG/ SLOPE_DATA, RSMTBL, RGLTBL, HSTBL, SNUPX,
     &  BB, DRYSMC, F11, MAXSMC, REFSMC, SATPSI, SATDK, SATDW,
     &  WLTSMC, QTZ, LPARAM, ZBOT_DATA, SALP_DATA, CFACTR_DATA,
     &  CMCMAX_DATA, SBETA_DATA, RSMAX_DATA, TOPT_DATA,
     &  REFDK_DATA, FRZK_DATA, BARE, DEFINED_VEG, DEFINED_SOIL,
     &  DEFINED_SLOPE, FXEXP_DATA, NROOT_DATA, REFKDT_DATA, Z0_DATA,
     &  CZIL_DATA, LAI_DATA, CSOIL_DATA

I think those needs to be fixed by adding &at the end of each line. There are also more of them like initialization of slope_data, rsmtbl etc.

@climbfuji
Copy link
Collaborator

@climbfuji I am also getting error from following statements in ccpp/physics/physics/set_soilveg.f,

      NAMELIST /SOIL_VEG/ SLOPE_DATA, RSMTBL, RGLTBL, HSTBL, SNUPX,
     &  BB, DRYSMC, F11, MAXSMC, REFSMC, SATPSI, SATDK, SATDW,
     &  WLTSMC, QTZ, LPARAM, ZBOT_DATA, SALP_DATA, CFACTR_DATA,
     &  CMCMAX_DATA, SBETA_DATA, RSMAX_DATA, TOPT_DATA,
     &  REFDK_DATA, FRZK_DATA, BARE, DEFINED_VEG, DEFINED_SOIL,
     &  DEFINED_SLOPE, FXEXP_DATA, NROOT_DATA, REFKDT_DATA, Z0_DATA,
     &  CZIL_DATA, LAI_DATA, CSOIL_DATA

I think those needs to be fixed by adding &at the end of each line. There are also more of them like initialization of slope_data, rsmtbl etc.

I don't think that is needed. This is Fortran fixed form (indicated by .f) and line continuations are made by putting a non-blank character (usually &) in the fifth column. No ending & needed, but if there is one, then past column 72. I guess you are passing a compiler argument that says "compile in free form", because ususally the compiler picks the right form (fixed vs free) based on the file ending (.f/.F vs .f90/.F90).

@uturuncoglu
Copy link
Contributor Author

@climbfuji It found that my NOAHMP CMake interface was passing -free. So, I removed it but this part pf code still gives error and all the lines are in the range of 72. So, I don't think the line length is an issue in here. so, maybe &is still needed.

@uturuncoglu
Copy link
Contributor Author

@climbfuji okay, my fault. it seems it solved the issue. Sorry for confusion.

@climbfuji
Copy link
Collaborator

@climbfuji okay, my fault. it seems it solved the issue. Sorry for confusion.

I am sorry that there is still so much legacy code in the repository. All of this should be converted to "modern" Fortran, following formatting and coding standards, declaring the real kinds correctly etc. But glad to hear it is working (for now).

@uturuncoglu
Copy link
Contributor Author

@climbfuji sorry but just for your record. ccpp/physics/physics/sflx.f has also alog issue.

@uturuncoglu
Copy link
Contributor Author

@climbfuji BTW, it seems that there is something wrong with the subroutine penman in ccpp/physics/physics/sflx.f and its argument definition. For example, ccpp/physics/physics/sfc_noahmp_drv.F90 calls penman with lots of arguments but the actual definition has no arguments. They are all start with !, is this file somehow preprocessed under CCPP?

@climbfuji
Copy link
Collaborator

@climbfuji BTW, it seems that there is something wrong with the subroutine penman in ccpp/physics/physics/sflx.f and its argument definition. For example, ccpp/physics/physics/sfc_noahmp_drv.F90 calls penman with lots of arguments but the actual definition has no arguments. They are all start with !, is this file somehow preprocessed under CCPP?

Now I am confused. I see in that file (line 1024) that it is called with 17 arguments:

      call penman (temperature_forcing, air_pressure_forcing , ch_noahmp            , &
                   virtual_temperature, potential_temperature, precipitation_forcing, &
                   penman_radiation   , ground_heat_total    , spec_humidity_forcing, &
                   spec_humidity_sat  , potential_evaporation, is_snowing           , &
                   is_freeze_rain     , precip_freeze_frac_in, dqsdt                , &
                   emissivity_total   , snow_cover_fraction  )

Later, in line 1461, the definition of penman also has 17 arguments:

      subroutine penman (sfctmp,sfcprs,ch,t2v,th2,prcp,fdown,ssoil,     &
     &                   q2,q2sat,etp,snowng,frzgra,ffrozp,             &
     &                   dqsdt2,emissi_in,sncovr)

@uturuncoglu
Copy link
Contributor Author

@climbfuji Okay. I think I understand the issue in here. Both ccpp/physics/physics/sflx.f and ccpp/physics/physics/sfc_noahmp_drv.F90 has penman method and they have different arguments.

@uturuncoglu
Copy link
Contributor Author

@climbfuji The arguments of subroutine penman in ccpp/physics/physics/sfc_noahmp_drv.F90 requires also real(kind_phys)

@climbfuji
Copy link
Collaborator

@uturuncoglu If you are making changes to the code to fix any of the issues that you have come across, please create a PR so that we also get those bug fixes in. Thanks a lot!

@uturuncoglu
Copy link
Contributor Author

@climbfuji Sure. Let me work on it. BTW, It is just a part of the CCPP/physics that I am using for external NoahMP land component but it is better than nothing.

@uturuncoglu
Copy link
Contributor Author

@climbfuji I created a PR: #755

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment