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

Problems with radsstat #171

Closed
leuliett opened this issue Dec 28, 2021 · 4 comments
Closed

Problems with radsstat #171

leuliett opened this issue Dec 28, 2021 · 4 comments

Comments

@leuliett
Copy link
Collaborator

radsstat has two errors: 1) The maximum value is always nan and 2) the option --no-stddev suppresses the output from --minmax.

  1. The commit
    57794bc sets z(:,1:) = nan, which causes the max value to always be nan:
swift:~$ radsstat -Sj3 -l -C1 -c
# Statistics of RADS variables (box weight)
# Created: 2021-12-28 16:34:39 UTC: radsstat -Sj3 -l -C1 -c
#
# Satellite : j3/a
# Cycles    :    1 -    1
# Passes    :    1 -  254
#
# Output columns:
#    ( 1) cycle
#    ( 2) date at beginning of cycle [YYMMDD]
#    ( 3) nr of measurements
#    ( 4) mean time [seconds since 1985-01-01 00:00:00 UTC]
# ( 5- 8) mean, stddev, min and max of sea level anomaly [m]
  1 160217   540421  982747942.   0.04301   0.11024  -1.93600       NaN
  1. The parameter --no-stddev introduced in commit 95a7f35 also suppresses the output from --minmax.
swift:~$ radsstat -Sj3 -l -C1 -c --no-stddev
# Statistics of RADS variables (box weight)
# Created: 2021-12-28 16:36:33 UTC: radsstat -Sj3 -l -C1 -c --no-stddev
#
# Satellite : j3/a
# Cycles    :    1 -    1
# Passes    :    1 -  254
#
# Output columns:
#    ( 1) cycle
#    ( 2) date at beginning of cycle [YYMMDD]
#    ( 3) nr of measurements
#    ( 4) mean time [seconds since 1985-01-01 00:00:00 UTC]
#    ( 5) mean of sea level anomaly [m]
  1 160217   540421  982747942.   0.04301
@remkos
Copy link
Owner

remkos commented Dec 29, 2021

I can confirm that both these bugs are present.

@remkos
Copy link
Owner

remkos commented Dec 29, 2021

I think number 1 is not coming from the change referred to, but rather from the fact that the IEEE fortran standard, nor gfortran, clearly defines what the result of max(x,y) is when either or both x and y are Nan.

My experiments with gfortran shows that

  • max(NaN,x) returns NaN
  • max(x,NaN) returns x

In most RADS programs, xmin and xmax are first initialised as NaN and then updated using

xmin = min(xmin, x)
xmax = max(xmax, x)

This would mean that both xmin and xmax remain NaN. I do not understand why xmin does appear to be updated; potentially because of the undefined nature of the routine. Using the apparent implementation of min and max in gfortran, this could then be replaced by

xmin = min(x, xmin)
xmax = max(x, xmax)

But maybe better is to avoid the compiler-dependent implementation and go for:

if (.not.(x > xmin)) xmin = x
if (.not.(x < xmax)) xmax = x

remkos added a commit that referenced this issue Dec 29, 2021
remkos added a commit that referenced this issue Dec 29, 2021
remkos added a commit that referenced this issue Dec 29, 2021
@remkos remkos assigned leuliett and unassigned remkos Dec 29, 2021
@remkos
Copy link
Owner

remkos commented Dec 29, 2021

Returning assignment to @leuliett to check implementation as per commits above and close this issue.

@leuliett
Copy link
Collaborator Author

Confirm that both issues are fixed by these commits.

remkos added a commit that referenced this issue Jan 10, 2022
* master:
  rads_add_common: reintroduce geoid_eigen6
  radsstat: fixing issue #171 part 2
  radsstat: fixing issue #171 part 1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants