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 memory errors: uninitialized variables, and NetCDF writes to arrays that are too small #668

Merged
merged 38 commits into from
Jul 10, 2023

Conversation

SamuelTrahanNOAA
Copy link
Contributor

@SamuelTrahanNOAA SamuelTrahanNOAA commented Jun 15, 2023

Description

This fixes two memory errors detected with valgrind, and one missing error check (errmsg, errflg). See the "issues addressed" for details.

Issue(s) addressed

Testing

How were these changes tested?

A 32-bit physics version of the RRFS conus13km regression tests, using valgrind.

What compilers / HPCs was it tested with?

Hera gnu and intel

Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)

There are new regression tests

Have the ufs-weather-model regression test been run? On what platform?

Yes. Hera.

Will the code updates change regression test baseline? If yes, why? Please show the baseline directory below.

No, but there are new tests.

Please commit the regression test log files in your ufs-weather-model branch

Will do.

Dependencies

@junwang-noaa
Copy link
Collaborator

@SamuelTrahanNOAA It looks to me the error-check in ufs-community/ccpp-physics#83 is to prevent the error of read(4) array from reading real(8) data. I am wondering what the solution is, so you will need a new netcdf file with real(4) data in order to run the 32bit physics?

@SamuelTrahanNOAA
Copy link
Contributor Author

@SamuelTrahanNOAA It looks to me the error-check in ufs-community/ccpp-physics#83 is to prevent the error of read(4) array from reading real(8) data. I am wondering what the solution is, so you will need a new netcdf file with real(4) data in order to run the 32bit physics?

My fix for the type conversion error was to use the NetCDF Fortran 90 interface. The Fortran 90 interface does automatic type conversion. Previously, that code used the Fortran 77 NetCDF interface.

@junwang-noaa
Copy link
Collaborator

Got it, thanks

@SamuelTrahanNOAA
Copy link
Contributor Author

Quoting the documentation of NF90_GET_VAR,

Data values are converted from the external type of the variable, if necessary.

They define "external type of a variable" as the type it has in the file. An exception to this rule is string arrays, which have no automatic type conversion.

@SamuelTrahanNOAA
Copy link
Contributor Author

The ccpp-physics PR has been merged. I have update the submodule pointer and reverted the .gitmodules. We're waiting for this PR to be merged so we can finish ufs-community/ufs-weather-model#1797

@zach1221
Copy link
Collaborator

@jkbk2004 can you merge this PR, please? I'm not authorized in this case.

@jkbk2004 jkbk2004 merged commit 67e146d into NOAA-EMC:develop Jul 10, 2023
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.

Uninitialized Grid variables in GFS_typedefs.F90 grid_create
5 participants