Add GK-2A AMI L1B Reader #911
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
djhoese
added
enhancement
djhoese
changed the title
Codecov Report
@@ Coverage Diff @@
## master #911 +/- ##
==========================================
+ Coverage 85.28% 85.92% +0.64%
==========================================
Files 174 174
Lines 25822 26452 +630
==========================================
+ Hits 22022 22730 +708
+ Misses 3800 3722 -78
Continue to review full report at Codecov.
|
…h between pyspectral and AMI brightness temperature conversion
…h between pyspectral and AMI brightness temperature conversion
|
This is neat. Just checked out this branch and testing. Let me know if there are specific tasks i can help. |
Use internal calibration coefficients if available
|
@songhan89 Thanks. I just made some updates and got some changes from @simonrp84. If you have time to make sure things look ok that'd be great. I think the only thing remaining for this PR is making sure that AMI has the same (or similar) RGB composites configured like AHI. I think I only added true color iirc. |
|
Hi @djhoese . The 9 Oct commit worked fine for me but the latest merge into the feature branch on 12 Oct somehow broke it. Running python 3.6 on Mac here.
|
|
Hi, |
|
@simonrp84 , thanks. I'm using the sample data from KMA website. The download link is here |
|
@simonrp84 I would not update the reader to work with the sample data. The latest sample data .zip file is corrupt and the sample data before that does not represent what the files look like now (ex. all bands used 14 bits of the 16-bit unsigned integer space even though it is supposed to vary by channel). |
|
Thanks @songhan89. I agree with @djhoese, if it's only the sample data that is causing problems then I don't think that the reader should be corrected. But please let us know if any other data has a problem! |
|
Sure I will test with live data.
Regards,
Songhan
… On 14 Oct 2019, at 4:17 PM, Simon Proud ***@***.***> wrote:
Thanks @songhan89. I agree with @djhoese, if it's only the sample data that is causing problems then I don't think that the reader should be corrected. But please let us know if any other data has a problem!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
Tested and works fine with the data. Thanks. |
|
Seemed to have run into issues with I turned on the from satpy import available_readers
from satpy import Scene
import glob
from satpy.utils import debug_on
debug_on()filenames = glob.glob('/Users/songhanwong/Google Drive/NEA/GK2A/gk2a_ami_le1b_*_201909180900.nc')
scn = Scene(reader="ami_l1b", filenames=filenames)
scn.available_dataset_names()scn.available_composite_names()scn.load(['convection'])from pyresample import create_area_def
custom_area = create_area_def('my_area', {'proj': 'merc', 'lon_0': 0.0},
width=1000, height=1000,
area_extent=[90, 15, 130, 40], units='degrees')
new_scn = scn.resample(custom_area)%matplotlib notebook
import matplotlib.pyplot as plt
from satpy.writers import get_enhanced_image
plt.figure()
img = get_enhanced_image(new_scn['convection'])
# get DataArray out of `XRImage` object
img_data = img.data
img_data.plot.imshow(rgb='bands', vmin=0, vmax=1)new_scn.load(['natural_color'])
scaled_scn = new_scn.resample(resampler='native')scaled_scn.load(['natural_color']) |
|
You can't call |
|
Sorry for the spam. Seems that the problem lies with computing sunz correction. Tested and it only affects those composites with [sunz_corrected] in
|
|
This is a bug in xarray 0.13.0. Either downgrade xarray or wait for the next release (hopefully this week they said). |
|
@songhan89 Just FYI, the bug that @djhoese mentioned has been fixed now - if you upgrade to xarray 0.14.0 then it should be OK. |
|
@simonrp84 @djhoese did one of you check the geolocation in the end ? |
|
@mraspaud I've checked against coastline overlays and it looks fine - but I can't find an alternative source of comparison. |
|
Good enough for me 👍 |
mraspaud
reviewed
Oct 21, 2019
Comment on lines
+57
to
+66
| wv063: | ||
| file_reader: !!python/name:satpy.readers.ami_l1b.AMIL1bNetCDF | ||
| file_patterns: ['{platform_shortname:4s}_{sensor:3s}_le1b_wv063_{sector_info:s}_{start_time:%Y%m%d%H%M}.nc'] | ||
| wv069: | ||
| file_reader: !!python/name:satpy.readers.ami_l1b.AMIL1bNetCDF | ||
| file_patterns: ['{platform_shortname:4s}_{sensor:3s}_le1b_wv069_{sector_info:s}_{start_time:%Y%m%d%H%M}.nc'] | ||
| wv073: | ||
| file_reader: !!python/name:satpy.readers.ami_l1b.AMIL1bNetCDF | ||
| file_patterns: ['{platform_shortname:4s}_{sensor:3s}_le1b_wv073_{sector_info:s}_{start_time:%Y%m%d%H%M}.nc'] | ||
|
|
There was a problem hiding this comment.
can't these be compacted in one generic filetype instead ?
There was a problem hiding this comment.
No. Not unless we implement what I talked about on slack or in another PR where the file handler updates its own file type once it parses the file. Otherwise when you ask for "VI006" then Satpy (yaml reader) is going to ask each file handler for this data. In my opinion this is unnecessarily wasteful computation.
There was a problem hiding this comment.
hmmm, ok. the viirs_sdr reader proceedes in this way, do you think it should be changed back ?
There was a problem hiding this comment.
I don't think you can do it with the viirs_sdr reader since you have to look through all possible combinations of the various file types (and in any order).
Comment on lines
+87
to
+97
| cfac = self.nc.attrs['cfac'] | ||
| coff = self.nc.attrs['coff'] | ||
| lfac = self.nc.attrs['lfac'] | ||
| loff = self.nc.attrs['loff'] | ||
| bit_shift = 2**16 | ||
| area_extent = ( | ||
| h * np.deg2rad((0 - coff - 0.5) * bit_shift / cfac), | ||
| -h * np.deg2rad((0 - loff - 0.5) * bit_shift / lfac), | ||
| h * np.deg2rad((cols - coff + 0.5) * bit_shift / cfac), | ||
| -h * np.deg2rad((rows - loff + 0.5) * bit_shift / lfac), | ||
| ) |
There was a problem hiding this comment.
This looks really similar to the hrit code, any possibility to factorize these ?
https://github.com/pytroll/satpy/blob/master/satpy/readers/hrit_base.py#L262-L277
There was a problem hiding this comment.
It does look pretty similar doesn't it. That's probably because these L1b files are based off of "uhrit" files which are probably similar in data structure to hrit files. I'm going to say that I don't want to make this refactor in this PR. We can make it an issue if you'd like, but there is a lot in that module that has nothing to do with these AMI files. We could move the method to another module, but it is also pretty specific to hrit, right?
There was a problem hiding this comment.
Yes, it's hrit specific, but I don't see any reason not to factorize. An issue would be the least :)
There was a problem hiding this comment.
Just a note, it's not HRIT specific but Geostationary specific. The AGRI reader uses similar code. Perhaps this type of thing could be added to a geo_utils.py or similar file?
There was a problem hiding this comment.
@simonrp84 But the 2**16 part isn't geostationary specific, right? We don't need these types of calculations for ABI because we have the X/Y radian coordinates already.
There was a problem hiding this comment.
Yes, @djhoese that's right. But ABI is a bit of an exception, we could do it the same way as the other geosats - but why bother with the extra computation :)
To my knowledge all the other geosats need this calculation.
There was a problem hiding this comment.
ABI is CF compliant. Hopefully future satellite data files (at least for higher level processing output) can also be CF compliant.
|
@djhoese do you still want to tick the first item in the check list or should with just merge this ? |
|
I think it is the best we're going to get and follows the sample code provided by KMA. I'm sure when it comes down to it we may want to be more precise like my updates to the ABI reader allowed, but that was being nit-picky. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add GEO-KOMPSAT-2A AMI Level 1B reader.
flake8 satpy