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

Jbacon ims latlong #53

Merged
merged 18 commits into from
Mar 19, 2024
Merged

Jbacon ims latlong #53

merged 18 commits into from
Mar 19, 2024

Conversation

JuLieAlgebra
Copy link
Member

@JuLieAlgebra JuLieAlgebra commented Mar 17, 2024

Major features:

  • Creates lat/lon functions (based on polar convert py from Ludo) that works in meters
  • Adds tests for lat/lon functions
  • Added tests for geolocation utility function
  • Includes my notebook work and lat/lon conversion prototypes
  • Rotates the netcdf files during the cropping pipeline to have expected x,y coordinates
  • Minor visualization function that was utility during lat/lon and cropping verification work
  • Can specify the center of the window to crop with lat/lon coordinates
  • Updated window_size to be final shape of cropped window (instead of 2window_size, 2window_size originally)

Details on Grid Rotation Solution

We can't rotate the entire netcdf image due to its size, but we also can't select the right window without correcting grid with a rotation first. Temporary solution is to crop to a large, but much smaller than original image first, then rotate and crop to correct window. Throws an assert error if the first crop excludes the selected window.

Details on lat/lon conversion:

Removed northern hemisphere considerations and removes a set of lines in the original polar_xy_to_lonlat that causes the longitude to become invalid (>180) for certain lonlat pairs.
https://github.com/nsidc/polarstereo-lonlat-convert-py/blob/366069fc65e781c277adf6d2fb0dac60c521d77d/polar_convert/polar_convert.py#L60C1-L61C38

    lon = lon + np.less(lon, 0) * 360

Previously, the tests I wrote to check that the conversions should undo each other doesn't pass with that re-wrapping of the longitude value. Now it does.

The original functions from latlon to xy actually don't have any tests: https://github.com/nsidc/polarstereo-lonlat-convert-py/blob/main/polar_convert/test.py

BGory and others added 11 commits March 16, 2024 22:36
Delete icedyno/preprocess/preprocess_netcdf.ipynb

Incorrect formatting

Add files via upload

Delete icedyno/preprocess/preprocess_netcdf.ipynb

Improper formatting

Add files via upload
…st, more to come. Had to include init for tests to see the icedyno project to import
cropped pipeline: Parallelized workflow to crop original IMS netcdf files to centered window at variable window size with luigi
…reprocess/geolocation. Returning x, y in meters and pinning constant values to IMS specific ones. Updated test to reflect change
Copy link
Collaborator

@SooluThomas SooluThomas left a comment

Choose a reason for hiding this comment

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

Are the cells in ims_latlong.ipynb that's not run still WIP?

@JuLieAlgebra
Copy link
Member Author

Are the cells in ims_latlong.ipynb that's not run still WIP?

It's cleaned up now! And ready for review

@JuLieAlgebra JuLieAlgebra requested a review from BGory March 18, 2024 02:58
… found by manual work in colab. Corrects out of memory error with netcdf rotations with temporary solution to an initial crop before grid correction.
Copy link
Collaborator

@SooluThomas SooluThomas left a comment

Choose a reason for hiding this comment

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

Looks great. I have aa question

config/preprocess_netcdf.toml Outdated Show resolved Hide resolved
…ould look like with google maps, added descriptions
Copy link
Collaborator

@SooluThomas SooluThomas left a comment

Choose a reason for hiding this comment

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

This looks perfect! Thank you!

Copy link
Collaborator

@mthanos7 mthanos7 left a comment

Choose a reason for hiding this comment

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

Great call on the netcdf crop, this looks good! Thank you!

@JuLieAlgebra JuLieAlgebra merged commit 932bbd8 into main Mar 19, 2024
2 checks passed
@JuLieAlgebra JuLieAlgebra deleted the jbacon_ims_latlong branch March 19, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants