search

NOAA-OWP / snow17

Other
4 stars 10 forks source link

Remove goto statements from source code #30

Open SnowHydrology opened 1 year ago

SnowHydrology commented 1 year ago

Work on the current NWM identified Fortran goto statements as a potential issue. Modern Fortran has moved away from goto statements for myriad reasons, some of which are discussed here.

There is one module from which the gotos should be removed (https://github.com/NOAA-OWP/snow17/search?q=goto):

Alternative solutions include do loops, if statements, cycle, exit, and case.

This issue also needs fixing in Sac-SMA and Noah-OWP-Modular.

nmizukami commented 1 year ago

Definitely goto statement is obsolete and could be error prone, but there may be other old formats that still remain especially if the programs are originally written earlier. Here are a list. https://fortranwiki.org/fortran/show/Modernizing+Old+Fortran, or this is good summary too.

andywood commented 1 year ago

FWIW, we have always tried to leave the original fortran for the NWS routines as untouched as possible to make sure they remain consistent with the legacy code. That said, they have goto statements which are not ideal (especially in snow17). The GOTOs are so dense in snow17 that any effort to replace them should be very careful to validate the new version. I tried rewriting snow17 in R once and got so bogged down that I eventually gave up. And there are R versions out there, but after my own experience I don't trust them!

The one FORTRAN construction that has to be changed when it's found is the statement function, which is no longer supported by compilers, even in legacy mode. I rewrote a bunch of those in Noah-MP as we were converting it.

Cheers, Andy

On Wed, May 3, 2023 at 9:56 AM Naoki Mizukami @.***> wrote:

Definitely goto statement is obsolete and could be error prone, but there may be other old formats that still remain especially if the programs are originally written earlier. Here are a list. https://fortranwiki.org/fortran/show/Modernizing+Old+Fortran, or this https://alm.engr.colostate.edu/cb/wiki/16983 is good summary too.

— Reply to this email directly, view it on GitHub https://github.com/NOAA-OWP/snow17/issues/30#issuecomment-1533293616, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIKARNEOTTTNUXGLRBYCVDXEJ5ZNANCNFSM6AAAAAAXUKOMRY . You are receiving this because you were assigned.Message ID: @.***>