Skip to content

Make MDI smaller#18

Open
DJDavies2 wants to merge 1 commit intodevelopfrom
feature/1
Open

Make MDI smaller#18
DJDavies2 wants to merge 1 commit intodevelopfrom
feature/1

Conversation

@DJDavies2
Copy link

Description

I was getting runtime failures in gsw_poly_check ctest for some compilers. Looking at the output it looks like there are some very large values in the arrays in gsw_mod_check_data that are clearly supposed to be missing data indicators. However the code makes no distinction between these values and the actual data values, and attempts to perform mathematical calculations on the missing data indicators. These calculations are overflowing, trigger floating point exceptions for some compilers. To fix I have modified these MDI values to be something smaller so that they survive these calculations. This is probably not the best solution, but I am not sure it is appropriate for me to do something better.

Issue(s) addressed

I don't seem to be able to open issues in the repository? I opened an issue in the Met Office equivalent, but was told to close it. Is it the intention that we cannot open issues in this repo?

Dependencies

None.

Impact

None.

Manual Testing Instructions (optional)

None.

Checklist

  • [y ] I have performed a self-review of my own code
  • [ y] I have made corresponding changes to the documentation
  • [y ] I have run the unit tests before creating the PR

@twsearle
Copy link

I think this one might be a fork? Can you open a similar issue on https://github.com/TEOS-10/GSW-Fortran ?

@DJDavies2
Copy link
Author

I could (in fact I think there is one there already) but:

  1. That repo hasn't been updated in about 3 years
  2. This fork is very much out of date (many commits behind and ahead at the same time)
  3. This fork has been modified in some fairly significant ways, perhaps to do with running in a bundle

Copy link

@srherbener srherbener left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem is that the original values (9e90_r8) do not match the JEDI internal missing values which are defined in oops: https://github.com/JCSDA-internal/oops/blob/develop/src/oops/util/missingValues.cc, and made available to Fortran here: https://github.com/JCSDA-internal/oops/blob/develop/src/oops/util/missing_values_mod.F90.

I can't speak on behalf of the GSW developers, but I think one possible solution could be to restore the 9e90_r8 values in this repo, and instead on the JEDI side where these tables are being read in, check for the 9e90_r8 values (or against some obviously too large of magnitude, like 1e10) and make the substitutions to the corresponding JEDI missing value. Then the JEDI code should properly ignore the 9e90_r8 values from these tables.

This is how the ioda reader and converters process missing values that come from obs providers where those providers use their own customized schemes for marking missing values.

@DJDavies2
Copy link
Author

I don't think that is the issue here though. gsw_poly_check is one of the ctests for the GSW-Fortran repository, as such it has no dependency on oops or any other Jedi repository. So changes on the Jedi side aren't going to help here.

Copy link
Collaborator

@travissluka travissluka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems harmless.

Ideally, this should be done in the upstream repo. The changes to our repo are just to compile as part of a bundle. Someday we should stop maintaining this repo, instead add the upstream gsw (fortran and C) as part of spack stack..... someday.

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.

4 participants