search

NOAA-EMC / WW3

WAVEWATCH III
Other
255 stars 504 forks source link

SEC1 switch causes GOTO jump into a loop #99

Open ukmo-ccbunney opened 4 years ago

ukmo-ccbunney commented 4 years ago

Compiling WW3 with the SEC1 switch (sub-second cycling) generates code where a GOTO statement jumps into a DO loop from outside the loop. This produces warnings on GNU and IFORT compilers, but is a fatal error on the Cray compiler. Either way, it seems like a bad idea.

In w3wavemd.ftn, the GOTO statement on line 1466 https://github.com/NOAA-EMC/WW3/blob/92cb41f70e91da97be9e5cd8bfee1de0a569f676/model/ftn/w3wavemd.ftn#L1466

jumps into the ISEC1 DO loop defined between lines 1563 to 2100: https://github.com/NOAA-EMC/WW3/blob/92cb41f70e91da97be9e5cd8bfee1de0a569f676/model/ftn/w3wavemd.ftn#L1563 https://github.com/NOAA-EMC/WW3/blob/92cb41f70e91da97be9e5cd8bfee1de0a569f676/model/ftn/w3wavemd.ftn#L2100

Essentially, the code is doing the equivalent of this:

      GOTO 370
      DO i = 1,10
        <DO  STUFF>
370   CONTINUE
      ENDDO

which looks a bit alarming!

I don't have a solution to this at the moment - it might require a bit of code refactoring...

ukmo-ccbunney commented 4 years ago

Update: I have found a Cray compiler flag that downgrades this from an error to a warning; this at least allows me to compile using the /SEC1 switch.

I will leave the issue open as I think it is worth some thought from the wider developer group.

aliabdolali commented 4 years ago

Hi Chis, Thanks for raising this issue. Computed GOTOs must be removed from operational codes. In addition, compilers will soon stop supporting computed GOTOs. Please make sure your bugfix leads to GOTO removal. It is applicable for any future development. Regards AA

| ALI ABDOLALI, Ph.D. ali.abdolali(at)noaa(dot)gov ali.abdolali@noaa.gov |

| Scientist abdolali(at)ucar(dot)edu abdolali@ucar.edu |

| Marine Modeling and Analysis Branch tell: +1 (302) 299 2378 |

| NWS/NCEP/Environmental Modeling Center office: +1 (301) 684 3733 |

| National Oceanic and Atmospheric Administration |

5830 University Research Court https://www.google.com/maps/place/5830+University+Research+Ct,+College+Park,+MD+20740/@38.972028,-76.9265727,17z/data=!3m1!4b1!4m5!3m4!1s0x89b7c6c96739b5a5:0xb7b4f4e67115a75e!8m2!3d38.972028!4d-76.924384
College Park, Maryland 20740, USA https://www.google.com/maps/place/5830+University+Research+Ct,+College+Park,+MD+20740/@38.972028,-76.9265727,17z/data=!3m1!4b1!4m5!3m4!1s0x89b7c6c96739b5a5:0xb7b4f4e67115a75e!8m2!3d38.972028!4d-76.924384
Personal webpage link http://aliabdolalilab.wix.com/ali-abdolali-cacr

On Wed, Sep 18, 2019 at 6:54 AM Chris Bunney notifications@github.com wrote:

Update: I have found a Cray compiler flag that downgrades this from an error to a warning; this at least allows me to compile using the /SEC1 switch.

I will leave the issue open as I think it is worth some thought from the wider developer group.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/NOAA-EMC/WW3/issues/99?email_source=notifications&email_token=AI43PDHZYUA7UUNTZQPEFQTQKIJILA5CNFSM4IX43CG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD67ZVPI#issuecomment-532650685, or mute the thread https://github.com/notifications/unsubscribe-auth/AI43PDB5OES57JPMNBW23Z3QKIJILANCNFSM4IX43CGQ .

ajhenrique commented 4 years ago

Hi @JessicaMeixner-NOAA please have a look at this issue and let us know if you clean-up of GOTOs will tackle the GOTO referenced above. Thanks!

JessicaMeixner-NOAA commented 4 years ago

This is not a computed GOTO that NCO requires us to remove. We do need to make sure we are not adding to the overall number of GOTOs though. I will not be addressing this.

On Mon, Sep 23, 2019 at 10:43 AM Jose-Henrique Alves < notifications@github.com> wrote:

Hi @JessicaMeixner-NOAA https://github.com/JessicaMeixner-NOAA please have a look at this issue and let us know if you clean-up of GOTOs will tackle the GOTO referenced above. Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NOAA-EMC/WW3/issues/99?email_source=notifications&email_token=AFMBF3ZJ5DH7JWQEBGUV4L3QLDPZZA5CNFSM4IX43CG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7LJZTQ#issuecomment-534158542, or mute the thread https://github.com/notifications/unsubscribe-auth/AFMBF35V2Y44YVBPOPZULIDQLDPZZANCNFSM4IX43CGQ .

-- Jessica Meixner NCEP/EMC NOAA Center for Weather and Climate Prediction