Add a class to help manage and interpolate static datasets by MiCurry · Pull Request #637 · MPAS-Dev/MPAS-Model

MiCurry · 2020-07-22T03:33:03Z

This merge adds a new module to the init_atmosphere core, mpas_geotile_manager, which contains a class, mpas_geotile_mgr_type, which can be used to help read, store and interpolate static fields. Currently, it is only being compiled, but in the future will be used to parallelize the interpolation of static fields.

src/core_init_atmosphere/mpas_geotile_manager.F

MiCurry · 2020-07-22T16:12:05Z

src/core_init_atmosphere/mpas_geotile_manager.F

When testing this function, I was a bit confused by the range of latitudes that it produced. If I grabbed the most north east tile mgr % get_tile(5. * pi, 2.0 * pi, tile) some of the range of latitudes produced by tile_to_latlon were greater than .5 * pi, and I'm not sure exactly why that would be.

A first thought is that this is because known_lat is not equal to exactly 5. * -pi. @mgduda, is that correct?

Typically, the known_lat and known_lon values for a dataset give the coordinates of the center of a pixel. So if the boundary of the pixel extends to, e.g., the South Pole, then the pixel's center coordinates will be greater than -Pi/2.

I just left a comment for lines 642 and 643 that may be related.

src/core_init_atmosphere/mpas_geotile_manager.F

MiCurry · 2020-07-27T16:34:56Z

@mgduda thanks for the primarily review. I just pushed an update fixing the warnings and other issues you found.

src/core_init_atmosphere/mpas_geotile_manager.F

MiCurry · 2020-07-29T17:30:39Z

@mgduda, I have just pushed an update to this branch that includes the changes above. I have pulled them out of 469c271 and moved them into 9111cfb and I have added the convert_lx routine in fe0709a.

src/core_init_atmosphere/mpas_geotile_manager.F

mgduda · 2020-07-30T19:33:10Z

src/core_init_atmosphere/mpas_geotile_manager.F

In lines 692 and 693, I think int should be nint.

Trying out nint (as well as floor and ceiling) will cause the geotile_manager to generate tile names that don't exist for the dataset. Although, I'm not sure if this behavior is correct or not. For instance the geotile_manager will generate the tile 42001-43200.21601-22800 for the point: (1.57080, 0.00000). The calculation is:

* lat = 90.0 * (lat - known_lat) / dx = 21599.5059 * nint(21599.5059) = 21600

However, I don't think this is correct, as the very last tile contains the 21600 pixel: 42001-43200.20401-21600.

So perhaps the tile filename generation is incorrect? Perhaps off by one?

Are the pixel coordinates that are returned by mpas_geotile_mgr_latlon_to_pixel supposed to be 0-based or 1-based?

Let's assume that every latitude value in the interval [-Pi/2, Pi/2] is uniquely mapped to a j-index. Then for the center latitude, lat, of a given pixel, we can either say that all latitudes in the interval [lat - Pi/2, lat + Pi/2) map to that pixel, or that all latitudes in the interval (lat - Pi/2, lat + Pi/2] map to that pixel. Regardless of which we choose, there will always be a pole, i.e., -Pi/2 or Pi/2 that is not mapped to some j-index and that therefore needs special treatment.

src/core_init_atmosphere/mpas_geotile_manager.F

MiCurry · 2020-08-03T20:15:33Z

Okay @mgduda, I just pushed an update that addresses your above comments, except for changes on line 692 and 693, which I left a comment on!

MiCurry · 2020-08-10T15:29:34Z

@mgduda I have just pushed an update to this branch which adds a default value to tile_bdr and uses nint on the latlon_to_pixel calculation and solves problems at the poles (as we discussed above). It also adds a new commit which adds a mpas_geotile_manager.F target to the Makefile.

mgduda · 2020-08-10T22:17:41Z

It looks like the changes to mpas_kd_tree.F are being included in this PR as commit 04e88f0, while these were previously included in the develop branch as ddd137d . Could you rework the commit history of this PR so the tie-breaking commit doesn't appear as a new commit?

mgduda · 2020-08-10T22:19:20Z

src/core_init_atmosphere/mpas_geotile_manager.F

Since pixel_y is declared as an integer, should 0.0 be changed to 0?

src/core_init_atmosphere/mpas_geotile_manager.F

MiCurry · 2020-08-11T22:01:13Z

@mgduda okay, I have just pushed an update that resolves rounding pixel_y values, changes 0.0 -> 0 and reworked the commit history to remove commit 04e88f0.

Let me know if you see anything else.

mgduda · 2020-08-14T21:41:52Z

src/core_init_atmosphere/mpas_geotile_manager.F

Should pixel_y be mgr % pixel_ny on the r.h.s.?

I think you're right. I'll push an update.

To help with future efforts in parallelizing the interpolation of static fields, a class has been added, mpas_geotile_type, which helps with reading and storing static datasets. As well, it contains routines for converting pixel coordinates into latitude and longitude. Currently, the mpas_geotile_type is not being used anywhere within the init_atmosphere core, it is only being compiled.

The pixel_to_latlon function of the mpas_geotile_manager is currently not planned to be used in the parallelization interpolation of static fields, so it is being removed.

This commit adds a function to the mpas_geotile_manager with a function to convert latitude, longitude coordinates into Cartesian coordinates, given a radius.

In previous commits mpas_geotile_manager.F was being compiled, but there was not a target for it in the Makefile. This meant that there was no dependencies for mpas_geotile_manager.F which would cause its compilation to fail occasionally when the mpas_geotile_manager.F was compiled before mpas_parse_geoindex.F.

MiCurry · 2020-08-14T22:01:43Z

@mgduda Okay, I just pushed an update!

mgduda

I think we're ready to merge!

Update MPAS-Source: COMPASS and Documentation files This PRi brings in a new mpas-source submdoule with changes only to the ocean core. It updates files unrelated to the E3SM forward model, including: * Move load_compass_env.sh to the ocean dir (MPAS-Dev/MPAS-Model#618); * Update ISOMIP+ docs, including tutorial for Ocean0 on LANL IC (MPAS-Dev/MPAS-Model#653); * Fix path to docs in Travis CI (MPAS-Dev/MPAS-Model#644); * atmosphere core init changes (MPAS-Dev/MPAS-Model#637); * New Mesh: WC14to60kmL60E3SMv2r03 (MPAS-Dev/MPAS-Model#628); * Fix ISOMIP+ viz scripts (MPAS-Dev/MPAS-Model#642); * Add support for custom critical passages and land blockages (MPAS-Dev/MPAS-Model#586); and * New Mesh: WC12to60kmL60E3SMv2r01 (MPAS-Dev/MPAS-Model#555) [BFB]

Update MPAS-Source: COMPASS and Documentation files This PR brings in a new mpas-source submdoule with changes only to the ocean core. It updates files unrelated to the E3SM forward model, including: * Move load_compass_env.sh to the ocean dir (MPAS-Dev/MPAS-Model#618); * Update ISOMIP+ docs, including tutorial for Ocean0 on LANL IC (MPAS-Dev/MPAS-Model#653); * Fix path to docs in Travis CI (MPAS-Dev/MPAS-Model#644); * atmosphere core init changes (MPAS-Dev/MPAS-Model#637); * New Mesh: WC14to60kmL60E3SMv2r03 (MPAS-Dev/MPAS-Model#628); * Fix ISOMIP+ viz scripts (MPAS-Dev/MPAS-Model#642); * Add support for custom critical passages and land blockages (MPAS-Dev/MPAS-Model#586); and * New Mesh: WC12to60kmL60E3SMv2r01 (MPAS-Dev/MPAS-Model#555) [BFB]

mgduda self-requested a review July 22, 2020 15:28

mgduda added Atmosphere feature labels Jul 22, 2020

mgduda reviewed Jul 22, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

mgduda reviewed Jul 22, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

mgduda reviewed Jul 22, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

MiCurry commented Jul 22, 2020

View reviewed changes

mgduda reviewed Jul 22, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

mgduda reviewed Jul 22, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

MiCurry force-pushed the init_atmosphere/geotile_manager branch from 3b49edf to cd6417d Compare July 27, 2020 16:34

MiCurry commented Jul 27, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

MiCurry commented Jul 27, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

MiCurry force-pushed the init_atmosphere/geotile_manager branch 4 times, most recently from e0b1a3b to fe0709a Compare July 29, 2020 17:27

MiCurry force-pushed the init_atmosphere/geotile_manager branch from fe0709a to 40b0e09 Compare July 29, 2020 23:53

MiCurry commented Jul 30, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

mgduda reviewed Jul 30, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

mgduda reviewed Jul 30, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

mgduda reviewed Jul 30, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

mgduda reviewed Jul 30, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

mgduda reviewed Jul 30, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

MiCurry force-pushed the init_atmosphere/geotile_manager branch from 40b0e09 to 5c48f9f Compare August 3, 2020 20:11

MiCurry force-pushed the init_atmosphere/geotile_manager branch from 5c48f9f to bab82e2 Compare August 9, 2020 03:13

MiCurry force-pushed the init_atmosphere/geotile_manager branch from bab82e2 to fc4248b Compare August 10, 2020 16:12

mgduda reviewed Aug 10, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated

Copy link

Contributor

mgduda Aug 10, 2020

Choose a reason for hiding this comment

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

Since pixel_y is declared as an integer, should 0.0 be changed to 0?

mgduda reviewed Aug 10, 2020

View reviewed changes

src/core_init_atmosphere/mpas_geotile_manager.F Outdated Show resolved Hide resolved

MiCurry force-pushed the init_atmosphere/geotile_manager branch from fc4248b to a16d043 Compare August 11, 2020 21:57

mgduda self-requested a review August 14, 2020 21:24

mgduda reviewed Aug 14, 2020

View reviewed changes

MiCurry force-pushed the init_atmosphere/geotile_manager branch from a16d043 to d0cdeb3 Compare August 14, 2020 21:57

MiCurry added 4 commits August 14, 2020 21:58

Remove function to convert pixel location to lat, lon in geotile manager

278a42e

The pixel_to_latlon function of the mpas_geotile_manager is currently not planned to be used in the parallelization interpolation of static fields, so it is being removed.

Add function to convert lat,lon to a Cartesian coordinate

29065e9

This commit adds a function to the mpas_geotile_manager with a function to convert latitude, longitude coordinates into Cartesian coordinates, given a radius.

MiCurry force-pushed the init_atmosphere/geotile_manager branch from d0cdeb3 to a1160b8 Compare August 14, 2020 21:59

mgduda self-requested a review August 14, 2020 22:34

mgduda approved these changes Aug 14, 2020

View reviewed changes

mgduda merged commit 515afd7 into MPAS-Dev:develop Aug 14, 2020

mark-petersen mentioned this pull request Aug 17, 2020

Update MPAS-Source: COMPASS and Documentation files E3SM-Project/E3SM#3776

Merged

matthewhoffman added Atmosphere feature labels Mar 17, 2021

Conversation

MiCurry commented Jul 22, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MiCurry commented Jul 27, 2020

Uh oh!

Uh oh!

Uh oh!

MiCurry commented Jul 29, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MiCurry commented Aug 3, 2020

Uh oh!

MiCurry commented Aug 10, 2020

Uh oh!

mgduda commented Aug 10, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MiCurry commented Aug 11, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MiCurry commented Aug 14, 2020

Uh oh!

mgduda left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants