Skip to content

Conversation

@xylar
Copy link
Collaborator

@xylar xylar commented Sep 29, 2016

This merge removes an unnecessary shift by 360 degrees in the script fix_features_at_prime_meridian.py. This behavior was added for compatibility with the 0-360 degree longitude coordinate used in MPAS-O but is no longer needed because the mask creator supports features between -180 and 180 degrees longitude by default.

@xylar
Copy link
Collaborator Author

xylar commented Sep 29, 2016

@pwolfram, this should be merged after #52. It is a relatively simple bug fix.

@xylar
Copy link
Collaborator Author

xylar commented Sep 29, 2016

I have tested both fix_features*.py scripts on the following features file to make sure it still works as expected.

{"type": "FeatureCollection",
 "features":
    [
        {"type": "Feature",
         "properties": {
            "author": "Xylar Asay-Davis",
            "component": "ocean",
            "name": "temp",
            "object": "region",
            "tags": ""
         },
         "geometry":
            {"type": "Polygon",
             "coordinates":
                [
                    [
                        [ 180.000000, -85.000000],
                        [ 90.000000, -85.000000],
                        [ 0.000000, -85.000000],
                        [ -90.000000, -85.000000],
                        [ -180.000000, -85.000000],
                        [ 180.000000, -85.000000]
                    ]
                ]
            }
        }
    ]
}

The result of the following commands:

./fix_features_at_antimeridian.py -f 85S.geojson -o valid_85S.geojson
./fix_features_at_prime_meridian.py -f valid_85S.geojson -o valid_85S_split_at_prime.geojson 

is a shape split at both meridians, as expected:

{
    "type": "FeatureCollection", 
    "features": [
        {
            "type": "Feature", 
            "properties": {
                "author": "Xylar Asay-Davis", 
                "object": "region", 
                "component": "ocean", 
                "name": "temp", 
                "tags": ""
            }, 
            "geometry": {
                "type": "MultiPolygon", 
                "coordinates": [
                    [
                        [
                            [
                                -180.000000, 
                                -85.000000
                            ], 
                            [
                                -90.000000, 
                                -85.000000
                            ], 
                            [
                                0.000000, 
                                -85.000000
                            ], 
                            [
                                0.000000, 
                                -90.000000
                            ], 
                            [
                                -180.000000, 
                                -90.000000
                            ], 
                            [
                                -180.000000, 
                                -85.000000
                            ]
                        ]
                    ], 
                    [
                        [
                            [
                                0.000000, 
                                -85.000000
                            ], 
                            [
                                90.000000, 
                                -85.000000
                            ], 
                            [
                                180.000000, 
                                -85.000000
                            ], 
                            [
                                180.000000, 
                                -90.000000
                            ], 
                            [
                                0.000000, 
                                -90.000000
                            ], 
                            [
                                0.000000, 
                                -85.000000
                            ]
                        ]
                    ]
                ]
            }
        }
    ]
}

@xylar xylar force-pushed the fix_fix_prime_meridian_scrip branch from 4352a92 to 9db8c66 Compare September 29, 2016 14:47
@pwolfram
Copy link
Contributor

@xylar, can you please rebase this branch and then I'll review it?

@xylar xylar force-pushed the fix_fix_prime_meridian_scrip branch from 9db8c66 to 6e12e7f Compare September 29, 2016 15:52
@xylar
Copy link
Collaborator Author

xylar commented Sep 29, 2016

@pwolfram, in the end, I don't think the rebase was necessary because I think a merge would have detected the redundant commits. But I just did the rebase in any case. Removing the don't merge flag.

@xylar xylar removed the Don't Merge label Sep 29, 2016
This merge removes a command to translate the western portin of
a feature by 360 degrees after splitting along the prime meridian.
Instead, longitudes should remain between -180 and 180 degrees.

Updated the docstrings for both fix_features*.py scripts to reflect
the fact that the output file may be specified by the -o flag.
@pwolfram
Copy link
Contributor

@xylar, this looks good. Some thoughts:

  1. The functionality between both of these scripts is very similar. Do we need two scripts?
  2. The script fix_features_at_antimeridian.py never appears to be used in the workflow:
┌─[pwolfram][shapiro][~/Documents/MPAS_pull_requests/fix_feature_prime_meridian_shift][11:26][±][fix_fix_prime_meridian_scrip ✗]
└─▪ git grep fix_features
driver_scripts/setup_antarctic_basins.py:41:args = ['./fix_features_at_prime_meridian.py', '-f', outName]
driver_scripts/setup_ocean_land_coverage.py:46:#args = ['./fix_features_at_prime_meridian.py', '-f', outName]
fix_features_at_prime_meridian.py:7:call fix_features_at_antimeridian.py first if necessary before using this

I'm happy to merge this but it may be useful to consider these potential cleanup steps, which are obviously not high-priority.

@pwolfram pwolfram merged commit 9e469eb into MPAS-Dev:master Sep 29, 2016
@xylar xylar deleted the fix_fix_prime_meridian_scrip branch September 29, 2016 17:30
@xylar
Copy link
Collaborator Author

xylar commented Sep 29, 2016

@pwolfram, I think there is a logical difference between the two scripts and we had rather different uses for them. In fact, we may not need fix_features_at_prime_meridian.py anymore even though it's currently used on the Antarctic basins.

fix_features_at_antimeridian.py is used to fix features that are not valid because they contain the North and/or South poles or because they cross the antimeridian and thus have segments that erroneously connect points near -180 with those near 180. Features like these arose when making Antarctic basins on polar stereographic grids and then remapping the points onto lon/lat coordinates. One basin included the South pole and several crossed the -180/180 divide. This script was applied manually to all Antarctic features before committing them to the repo. If we had provenance for those features, this script would appear there. Unfortunately, we don't and we'll need to recreate it. I'll add that to my to-do list.

fix_features_at_prime_meridian.py was used to make features that the MPAS mask creator could read with longitude between 0 and 360 degrees. This is no longer needed because the mask creator assumes it's getting input between -180 and 180 by default now.

@pwolfram
Copy link
Contributor

@xylar, should we remove fix_features_at_prime_meridian.py then? I'm ok leaving it for now but want to keep the repository as simple / clean as possible (hence the suggestion to potentially remove it).

@xylar
Copy link
Collaborator Author

xylar commented Sep 29, 2016

@pwolfram, you could make an issue for this. For now, I'm not entirely sure it's not needed so it should stay.

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.

2 participants