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

fix: bug fix for obs_sort_module.F90. #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

greent12
Copy link

@greent12 greent12 commented Dec 9, 2023

This is a proposed fix for Issue #11

A conditional statement was added on line 3158 to check wether the pointer 'next' is associated or not before going into the following block of code. This bug fix was stated by the user "Alpha_su" in the obsgrid forum: https://forum.mmm.ucar.edu/threads/obsgrid-exe-fails-after-a-few-seconds.13474/

…rmerly 3159) was causing obsgrid to segfault situationally.

A conditional statement was added on line 3158 to check wether the pointer 'next' is associated or not before going into the following block of code. This seems to have remedied the problem for now.
This bug fix was stated by the user "Alpha_su" in the obsgrid forum: https://forum.mmm.ucar.edu/threads/obsgrid-exe-fails-after-a-few-seconds.13474/
@greent12 greent12 requested a review from a team as a code owner December 9, 2023 16:18
@brianreen
Copy link

Thanks for submitting this code to address issue #11. The original code inadvertently relies on short-circuiting of Boolean expressions, but since the Fortran standard does not require this, the code only works with certain compilers. While the fix you submitted would avoid the issue, it seems to me that it would be more straightforward to not add the ASSOCIATED check but rather to break the existing IF statement into two IF statements so that next is evaluated only when is_sounding is FALSE. Specifically, I would suggest:

        IF(.NOT. is_sounding) THEN
          IF(.NOT. eps_equal(next%meas%height%data, obs(i)%info%elevation, 1.)) THEN
            obs(i)%info%discard = .TRUE.
            PRINT '(A,A,A,A,F12.2,A,F12.2,A,A,A,A)','WARNING: Ob indicates is_sounding=.FALSE. ',&
             'but the ob height does ',&
             'not match the station elevation.  This ob will be omitted since Obsgrid assumes ',&
             'is_sounding=.FALSE. indicates a surface ob. Height = ', next%meas%height%data, &
             ', Elevation = ', obs(i)%info%elevation,', Ob ID = ',TRIM(ADJUSTL(obs(i)%location%id)),&
             ', Ob Time = ', obs(i)%valid_time%date_char
          ENDIF
        ENDIF

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants