NCAR / DART

Data Assimilation Research Testbed
https://dart.ucar.edu/
Apache License 2.0
184 stars 139 forks source link

MITgcm/N-BLING with Compressed Staggered Grids #640

Closed mgharamti closed 4 months ago

mgharamti commented 4 months ago

Description:

The MITgcm code now supports compressed grids, especially suited for areas like the Red Sea where land occupies more than 90% of the domain. Thanks to Ed Liu (SIParCS intern, 2022) and Helen Kershaw for developing this approach.

Fixes issue

The updated code allows writing the bgc fields into MITgcm's pickup files. It also allows for different compression of the regular and staggered grids. These changes have been tested at KAUST for the Arabian Gulf in cycling DA runs.

Types of changes

Documentation changes needed?

Tests

Please describe any tests you ran to verify your changes.

Checklist for merging

Checklist for release

Testing Datasets

hkershaw-brown commented 4 months ago

Hi Moha, great work on this and thanks to the KAUST folks for testing it thoroughly. It is a really good improvement on the current code. One question for you: There is a program expand_netcdf which was used to uncompress the netcdf file into a non-compressed netcdf file - this was for MITgcm novices (like me) taking a look at the data with ncview while writing the compression code. It does not uncompress for the different grids, and so its not complete at the movement. Are you ok removing this (assuming it is not what regular DA user would need?)

I do want check on the netcdf_utilies_mod 64 bit offset change to see if we need to be careful/paranoid about breaking netcdf writes: reviewers see notes https://github.com/NCAR/DART/pull/455/files#r1101999108, https://github.com/NCAR/DART/issues/287

Then a couple of notes for whoever does the release: check with Ed how he wants his name, and here is his AMS presentation and siparcs page to add to the release notes. From work by Jiachen Liu which he presented at AMS 2023 The work was done as part of SIParCS 2022 under the Red Sea Initiative project

mgharamti commented 4 months ago

Helen, thanks for taking time to review this PR. To your first question, I have no problem removing the expand_netcdf program. I understand the reason behind it yet I didn't run it myself. Siva, used it a couple of times before extending the compression capability but as you noted it's not needed for operating filter. Good observation regarding the 64 bit netcdf thing, I am not sure if this causes any issues with other nc files. I guess I'm not experienced enough on this front to give a useful suggestion. I fully support acknowledging Ed in the best possible way for this release.