Unidata / netcdf-java

The Unidata netcdf-java library
https://docs.unidata.ucar.edu/netcdf-java/current/userguide/index.html
BSD 3-Clause "New" or "Revised" License
146 stars 71 forks source link

GEOSTransform better test if past limb #1147

Closed rschmunk closed 1 year ago

rschmunk commented 1 year ago

As previously coded, GEOSTransform was providing non-null results for locations that were just past the visible limb of the planet but not more than 90° away from the below-satellite point.

Description of Changes

I have added a better test to determine if such lon-lat points should be ignored based on the "GOES R SERIES PRODUCT DEFINITION AND USERS’ GUIDE (PUG)", sects. 5.1.2.8.1-5.1.2.8.2 (pp. 21-24) which can be dowloaded at https://www.goes-r.gov/resources/docs.html

I have tested this for the GOES orientation but not the GEOS, as I do not have sample datasets for the latter.

PR Checklist

rschmunk commented 1 year ago

More specifically, the added test is based on the first equation of section 5.1.2.8.2 in the referenced document.

haileyajohnson commented 1 year ago

Awesome, thanks @rschmunk ! Could you provide the sample dataset that you do have so we could add a unit test for this scenario and change?

rschmunk commented 1 year ago

@haileyajohnson, The sample file that a user sent me to look at is located here. This is a GOES-16 dataset apparently acquired earlier this month, and the variable of interest is CMI. Not sure but the original might be buried in the Google Cloud&prefix=&forceOnObjectsSortingFiltering=false).

After seeing the bad plotting effect that the user pointed out, I poked around in my sample datasets folder and found several more GOES-16 files that showed the same problem, which was what led me to believe it was a code problem rather than a data gridding problem.

It was a good thing the user also sent me the GOES user's guide so that I could try coding things up and then compare my code to that in GEOSTransform, as I found that many/most of the URLs in NJ's ucar.nc2.dataset.transform.Geostationary and ucar.unidata.geoloc.projection.sat.Geostationary classes had rotted and gone 404.

Even so, I'd like to get a sample GEOS dataset so that I can test handling of its alternative scan orientation.

rschmunk commented 1 year ago

Any idea when this pull might be approved? I'm wondering if/when I can drop using an NJ fork from my app build process.

rschmunk commented 1 year ago

Not to complain. Dennis's pull request re: DAP4 support also matches up with an issue I'm dealing with, and it looks like serious stuff.

tdrwenski commented 1 year ago

Sorry for the delay in approving this. We will try to add a test and approve it as soon as possible!

tdrwenski commented 1 year ago

Hi @rschmunk, I added a few tests. The only one that directly checks your change is shouldReturnNanForNotVisibleLatLonPoints but I added a few others while I was at it.