CFMIP / COSPv2.0

COSP - The CFMIP (Cloud Feedbacks Model Intercomparison Project) Observation Simulator Package
42 stars 40 forks source link

UM global snapshot #51

Closed alejandrobodas closed 4 years ago

alejandrobodas commented 4 years ago

This addresses #49 . It implements a new test that uses a global field of inputs from the UM. The original model resolution is N96 (96x72), with 85 levels. That required the use of Large Files Support, but I believe we are restricted to 1GB storage and 1GB of bandwith per month in LFS, which is not enough. In the end, I have bypassed this by subsampling every other row&column in the model's inputs, and using only the bottom 53 levels. This, and gzip compression of the global input and KGO files makes the data volumes manageable without the need of large file support. The global test is only run for the standard version of the gfortran compiler. When ifort is used, it fails with a segmentation fault in the github servers. It runs fine in my local machine using ifort, so I couldn't investigate this further. I have also added a step that, when the global test fails (i.e. it doesn't strictly compares with the KGO), it runs a python script that produces plots of the majority of diagnostics. The pdf attached includes all the plots produced in the github servers for the current global KGO. The CloudSat PIA doesn't look right to me, but since this pull request doesn't change the fortran code, apart from a minor change to pass the input namelist as an argument, I think that should be looked into in a different issue.

um_global.issue49.pdf

klein21 commented 4 years ago

Great progress, Alejandro!

Looking through the plots, I was confused why some of the plots had the same title (page 3) and also reappear with more missing value (page 6), but I assume that you can take of that later.

alejandrobodas commented 4 years ago

Thanks @klein21 Each plot shows a different variable. The titles are picked up from the long_name attribute in the output file, which is not always correct (legacy of copying&pasting bits of code to write new variables, I guess). I put all the plots in a single pdf for convenience, but you can look at the individual plots if you download the artifact created by: https://github.com/CFMIP/COSPv2.0/actions/runs/141022134

There, the filenames contain the variable name, and therefore there is no ambiguity.

alejandrobodas commented 4 years ago

@klein21 @dustinswales @RobertPincus if none of you have any objections to this change, then I propose to go ahead and merge it. Thanks.

RobertPincus commented 4 years ago

@alejandrobodas: thanks for motivating the global snapshot. This is another big step forward in making the testing more robust. I have two suggestions the current implementation.

First, it would be better to store large files, like those required for the global snapshot testing, outside the source code repository. This is partly conceptual — source code is different than testing data — and partly polite to users who might not find it convenient to have lots of data pulled alongside a few kb of source code. What’s important, I think, is that the data we refer to be persistent and trackable. The Github large file service is one possibility. Careful working practices with file sharing services like Dropbox or Owncloud would also work. But we shouldn’t merge until this question is resolved.

Second, it’s important that the global tests (thinned if need be) pass for all compilers. The point of the continuous integration is to stress-test the code with a range of data and a range of compilers.

alejandrobodas commented 4 years ago

@RobertPincus thanks for your feedback, I'll think about solutions for the points you raise.

alejandrobodas commented 4 years ago

@RobertPincus I've made some changes that address the issues you raised. The global files are now stored in Google Drive, and downloaded on the fly by the integration tests. I have stored md5 sums in the repository so that developers can check that the are using the correct files if they use it locally. I've also added a simple script that can be used to download the global files, should any user/developer want to do that. I've also found the origin of the segmentation fault with ifort, and the global test is now run with all four compilers. By default, ifort uses the stack for automatic arrays, whereas gfortran uses the heap. There are many places where COSP uses temporary arrays, making ifort prone segmentation faults when large files are used. Now heap arrays are also used with ifort. I think these changes are now ready for merging.

RobertPincus commented 4 years ago

@alejandrobodas This is neat, thanks for taking the trouble. I very much like the checking of the MD5 sum against a known value. Perhaps it’s worth noting somewhere that developers have to update those .md5 files when answers change.

One last ideas: as long as you’re revisiting polynomial evaluation in COSP optics why not use https://en.wikipedia.org/wiki/Horner%27s_method?

RobertPincus commented 4 years ago

@alejandrobodas … but I should have said that I otherwise think this is fine to merge.

alejandrobodas commented 4 years ago

@RobertPincus Thanks for this, it has reminded me of the need to update the working practices. 've slightly modified the README to make users aware of the new global test. I have also expanded the working practices (in cfmip.github.io) to explain the use of the new CI, and the need to update the MD5 sums when results change. Regarding that polynomial, I've tested Horner's method, and it produces slightly different results, without making any significant impact in overall speed. Given that it doesn't seem to show any obvious benefit and it changes the results, I've decided not to include that change. This should probably be revisited within the context of more widespread optimisations.