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

Add support for mom (supergrid) grid format #993

Open
wants to merge 34 commits into
base: main
Choose a base branch
from

Conversation

anton-seaice
Copy link
Contributor

@anton-seaice anton-seaice commented Nov 7, 2024

This changes adds grid_format = 'mom_nc' support to directly load the grid from a netcdf file which uses the mom (supergrid) definition, and calculates all fields to follow the method MOM6 uses. i.e.

  • all lats & lons are read from the file
  • all cell x/y lengths are read from the file
  • A grid and B grid areas are read from the file, C grid areas are calculated internally (per Initialising areaC NOAA-GFDL/MOM6#740)
  • angles are calculated internally (even though they exist in the mosaic file)

There are a few limitations:

  • If the CHANNEL_CONFIG option is used in MOM6, this can change some cell sizes at runtime, however we are loading these from the file.
  • MOM6 generates a land-sea mask by making land where the bathymetry<0.5m , however we are still loading from a kmt file which needs to be generated "offline".

I haven't tested every permutation of boundary conditions and grid type, I focussed on on tripolar grids and did best-effots checks on the other boundary conditions.

I found exact matches (to single point precision) between the MOM6 & CICE6 saved grids, see https://github.com/anton-seaice/sandbox/blob/main/new_cice_grid/mom_mosaic_in_cice.ipynb

The NUOPC driver checks for consistency in areas, and lon/lats between the model and the mediator. It applies a correction for differences in area between the model and the mediator. Even if the cell area field is populated in the mediator mesh file, this is not used and the area calculated by ESMF is used instead. Therefore depending on how the mosaic file was created (and how the areas are calculated for that process), the correction between model and mediator areas may not be 1 everywhere.

This change doesn't include any new tests, and I suggest deferring adding tests as it requires new forcing data

I also added kmt_type = 'none' - This is only for testing / idealised configurations, so it's not in the docs. It's not essential for this PR.

PR checklist

  • Short (1 sentence) summary of your PR:
    Add support for creating the CICE grid from a MOM mosaic (supergrid) format
  • Developer(s):
    @anton-seaice
  • Suggest PR reviewers from list in the column to the right.
    @dabail10
  • Please copy the PR test results link or provide a summary of testing completed below.
    To-do, if the change & scope looks ok, I will run the cice standalone tests
  • How much do the PR code changes differ from the unmodified code?
    • bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?
    • Yes
    • No
  • Does this PR update the Icepack submodule? If so, the Icepack submodule must point to a hash on Icepack's main branch.
    • Yes
    • No
  • Does this PR add any new test cases?
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/. A test build of the technical docs will be performed as part of the PR testing.)
    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please document the changes in detail, including why the changes are made. This will become part of the PR commit log.
    This changes adds grid_format = 'mom_nc' support to directly load the grid from a netcdf file which uses the mom mosaic (supergrid) definition, and calculates all fields to follow the method MOM6 uses. Contributes to MOM supergrid and other grid types #807

anton-seaice and others added 28 commits October 14, 2024 13:24
Tidy compiler flags

extend a few tests

extend one more

tidy

fix gadi test submission

Tidy compiler flags

extend a few tests

longer again
commit 9555346
Author: Anton Steketee <79179784+anton-seaice@users.noreply.github.com>
Date:   Mon Oct 28 09:26:14 2024 +1100

    Update cicecore/cicedyn/general/ice_init.F90

commit 388612d
Author: anton-seaice <anton.steketee@anu.edu.au>
Date:   Tue Oct 22 11:38:46 2024 +1100

    fix gadi test submission

    Tidy compiler flags

    extend a few tests

    extend one more

    tidy

    fix gadi test submission

    Tidy compiler flags

    extend a few tests

    longer again

commit 3d9566a
Author: anton-seaice <anton.steketee@anu.edu.au>
Date:   Tue Oct 22 13:43:42 2024 +1100

    deprecate cpom orca grid
@eclare108213
Copy link
Contributor

@anton-seaice, it looks like 'mosaic' in this context has nothing to do with the field campaign. Is that correct? If so, it could be confusing for other users interested in the MOSAiC data. Let's figure out how to document both to reduce confusion. Certainly explain what they are in the readthedocs, but also perhaps add some comments to the diagnostic output?

@dabail10
Copy link
Contributor

dabail10 commented Nov 7, 2024

@anton-seaice, it looks like 'mosaic' in this context has nothing to do with the field campaign. Is that correct? If so, it could be confusing for other users interested in the MOSAiC data. Let's figure out how to document both to reduce confusion. Certainly explain what they are in the readthedocs, but also perhaps add some comments to the diagnostic output?

I agree. Maybe mosaic is not the right term to use here. Maybe mom_supergrid or just 'mom'. Also, I would add that grid_type has the following options currently: 'displaced_pole', 'tripole', 'default', 'regional', and 'latlon'. So, perhaps we should do something like 'mom_tripole'?

@anton-seaice
Copy link
Contributor Author

Thanks both

Yes definitely unrelated to MOSAiC voyages.

The term mosaic and supergrid are used a little bit interchangeably, although most of the MOM6 source code and the ESMF docs refer to a mosaic specification, so I used that. Happy to change to calling it supergrid if that is clearer. I didn't want to only refer to it as the MOM format because there are is an earlier mom format which is different.

@dabail10 - apologies - there was an error in the description, which i've just amended. This changes adds grid_format = mom_mosaic and it supports the existing grid_type options of regional, tripole and displaced_pole. The only one it doesn't support is tripoleT and aborts if that configuration is set.

@dabail10
Copy link
Contributor

dabail10 commented Nov 7, 2024

Thanks both

Yes definitely unrelated to MOSAiC voyages.

The term mosaic and supergrid are used a little bit interchangeably, although most of the MOM6 source code and the ESMF docs refer to a mosaic specification, so I used that. Happy to change to calling it supergrid if that is clearer. I didn't want to only refer to it as the MOM format because there are is an earlier mom format which is different.

@dabail10 - apologies - there was an error in the description, which i've just amended. This changes adds grid_format = mom_mosaic and it supports the existing grid_type options of regional, tripole and displaced_pole. The only one it doesn't support is tripoleT and aborts if that configuration is set.

I understand now. However isn't grid format either nc or bin? Maybe this should be mom_nc?

@anton-seaice
Copy link
Contributor Author

I understand now. However isn't grid format either nc or bin? Maybe this should be mom_nc?

I figured there would never be a mom_mosaic grid not in a netcdf format, so i didn't think we needed to included nc.

If it is clearer we could use mom_nc or leave grid_type alone and add a new namelist variable (e.g. grid_specification and it could be type pop or mom_mosaic)

@dabail10
Copy link
Contributor

dabail10 commented Nov 7, 2024

I understand now. However isn't grid format either nc or bin? Maybe this should be mom_nc?

I figured there would never be a mom_mosaic grid not in a netcdf format, so i didn't think we needed to included nc.

If it is clearer we could use mom_nc or leave grid_type alone and add a new namelist variable (e.g. grid_specification and it could be type pop or mom_mosaic)

This might be better. However, I think this is overkill. I think grid_format = 'pop_nc' or 'mom_nc' is fine. I also really think you should not have "mosaic" in the name. This is going to be confusing.

@anton-seaice
Copy link
Contributor Author

anton-seaice commented Nov 7, 2024

This might be better. However, I think this is overkill. I think grid_format = 'pop_nc' or 'mom_nc' is fine. I also really think you should not have "mosaic" in the name. This is going to be confusing.

I guess if we just cover that a mom_grid refers to a mom mosaic / supergrid format in the docs then its sufficiently clear that is the format being referred to ? Its unlikely that configs using the latest CICE will want to use an old MOM grid format ?

@dabail10
Copy link
Contributor

dabail10 commented Nov 7, 2024

I understand now. However isn't grid format either nc or bin? Maybe this should be mom_nc?

I figured there would never be a mom_mosaic grid not in a netcdf format, so i didn't think we needed to included nc.
If it is clearer we could use mom_nc or leave grid_type alone and add a new namelist variable (e.g. grid_specification and it could be type pop or mom_mosaic)

This might be better. However, I think this is overkill. I think grid_format = 'pop_nc' or 'mom_nc' is fine. I also really think you should not have "mosaic" in the name. This is going to be confusing.

Before I get more into this, I am so grateful that you've done the work on this!

I just talked to one of our MOM6 developers here. Here is what I think we should do:

For POP binary and netcdf files: grid_format = 'bin','nc' , kmt_type = 'file'. This maintains backward compatibility.

For MOM6 supergrid files: grid_format = 'mom6_nc' and kmt_type = 'none'. Then key off grid_format to see if we need to read in the MOM6 bathymetry file.

The "mosaic" file was an older MOM file. We will never have to support this. Our MOM developer has a script called mosaic_to_supergrid.py. I agree that we should stop using 'kmt' but again for backward compatibility, I think we leave it as kmt for now.

@anton-seaice
Copy link
Contributor Author

Hopefully it saves us generating many CICE grid files offline :)

Ok great - those namelist options are ok with me.

I haven't tried loading a bathymetry file yet, is the format of the CICE bathymetry file different to the MOM6 format ?

I was planning to, in a later PR, generate the CICE land/sea mask from the bathymetry file. In the interim, we would keep using the current kmt_type=file for the land/sea mask - is that consistent with your thinking ?

@dabail10
Copy link
Contributor

dabail10 commented Nov 7, 2024

Hopefully it saves us generating many CICE grid files offline :)

Ok great - those namelist options are ok with me.

I haven't tried loading a bathymetry file yet, is the format of the CICE bathymetry file different to the MOM6 format ?

I was planning to, in a later PR, generate the CICE land/sea mask from the bathymetry file. In the interim, we would keep using the current kmt_type=file for the land/sea mask - is that consistent with your thinking ?

I think the MOM6 bathymetry is very different from the one CICE expects. So we will also have to use grid_format here. I definitely agree with keeping kmt_file around for the near future.

@anton-seaice
Copy link
Contributor Author

For POP binary and netcdf files: grid_format = 'bin','nc' , kmt_type = 'file'. This maintains backward compatibility.

For MOM6 supergrid files: grid_format = 'mom6_nc' and kmt_type = 'none'. Then key off grid_format to see if we need to read in the MOM6 bathymetry file.

@apcraig - are these option names ok ? implementing the MOM6 bathymetry would be a future PR

@apcraig
Copy link
Contributor

apcraig commented Nov 8, 2024

I think the current plan is fine. We should maintain the current namelist and backwards compatibility whenever possible. I also agree that using "mosaic" could be confusing.

Would having a kmt_type='grid' make sense? That would mean the kmt info is on the grid file? Or maybe even kmt_type='grid' and kmt_type='grid_bathy'. The former means the mask is read off the grid file, the latter means the bathymetry is read off the grid file then a kmt mask is created. And how does that interact with the bathymetry namelist values? I haven't looked at all the current and possible permutations of grid/kmt/bathy options, but maybe we should clarify all the existing and potential options and make sure we have a clear set of namelist values that make implementation of them clear (without breaking backwards compatibility).

@anton-seaice
Copy link
Contributor Author

Thanks - I change mom_mosaic to mom_nc and updated the comments / names etc as needed

@anton-seaice anton-seaice changed the title Add support for mom mosaic (supergrid) grid format Add support for mom (supergrid) grid format Nov 10, 2024

! check tripole flags here
! can't check in init_data because ns_boundary_type is not yet read
! can't check in init_domain_blocks because grid_type is not accessible due to circular logic

if (grid_type == 'tripole' .and. ns_boundary_type /= 'tripole' .and. &
if (trim(grid_type) == 'tripole' .and. ns_boundary_type /= 'tripole' .and. &
Copy link
Contributor

Choose a reason for hiding this comment

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

I never understand the use of trim. However, should this not be consistent here?

@apcraig apcraig added the Grids label Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants