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

Add CREFL rayleigh corrector for ABI data #317

Merged
merged 42 commits into from
Aug 27, 2018

Conversation

wroberts4
Copy link
Contributor

@wroberts4 wroberts4 commented Jun 4, 2018

I added the rayleigh correction to also work for abi data in order to incorporate a wider range of satpy functionality. The code I wrote was re-purposed from work done by Ralph Kuehn at SSEC.

Note: Ralph's code uses a different algorithm than that which is used by satpy. A research paper covering the algorithm that Ralph's code uses can be found here: https://www.atmos-meas-tech.net/6/2989/2013/

A couple of things to keep in mind:

  1. Added modifiers in abi/viirs.yaml may need be re-purposed/removed in the future. Same goes for composites in abi.yaml.
  2. The last 5 elements of bH2O in crefl_utils.py are actually aO2 values (in Ralph's code). Currently aO2 is not used in the satpy code since it would require restructuring of multiple features, so they are tagged at the end of bH2O.
  • Tests added
  • Tests passed
  • Passes git diff origin/master **/*py | flake8 --diff
  • Fully documented

aH2O = np.array(
[0.000406601, 0.0015933, 0, 1.78644e-05, 0.00296457, 0.000617252,
0.000996563, 0.00222253, 0.00094005, 0.000563288, 0, 0, 0, 0, 0, 0])
0.000996563, 0.00222253, 0.00094005, 0.000563288, 0, 0, 0, 0, 0, 0,
2.4111e-003, 7.8454e-003*rg_fudge,7.9258e-3, 9.3392e-003, 2.53e-2])
Copy link
Contributor

Choose a reason for hiding this comment

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

E231 missing whitespace after ','

0.78812, 0.791204, 0.900564, 0.942907, 0, 0, 0, 0, 0, 0,
# These are actually aO2 values for abi calculations
1.2360e-003, 3.7296e-003, 177.7161e-006, 10.4899e-003, 1.63e-2])
# /*const float aO3[Nbands]={ 0.0711, 0.00313, 0.0104, 0.0930, 0, 0, 0, 0.00244, 0.00383, 0.0225, 0.0663, 0.0836, 0.0485, 0.0395, 0.0119, 0.00263};*/
Copy link
Contributor

Choose a reason for hiding this comment

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

E501 line too long (162 > 120 characters)

#/*const float taur0[Nbands] = { 0.0507, 0.0164, 0.1915, 0.0948, 0.0036, 0.0012, 0.0004, 0.3109, 0.2375, 0.1596, 0.1131, 0.0994, 0.0446, 0.0416, 0.0286, 0.0155};*/
0, 0, 0.0663, 0.0836, 0.0485, 0.0395, 0.0119, 0.00263,
4.2869e-003, 25.6509e-003*rg_fudge, 802.4319e-006, 0.0000e+000, 2e-5])
# /*const float taur0[Nbands] = { 0.0507, 0.0164, 0.1915, 0.0948, 0.0036, 0.0012, 0.0004, 0.3109, 0.2375, 0.1596, 0.1131, 0.0994, 0.0446, 0.0416, 0.0286, 0.0155};*/
Copy link
Contributor

Choose a reason for hiding this comment

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

E501 line too long (175 > 120 characters)

def get_atm_variables_abi(mus, muv, phi, height, coeffs, G_O3, G_H2O, G_O2):
# coeffs = (ah2o, aO2, ao3, tau)
(ah2o, ao2, ao3, tau) = coeffs
missing_value = -999.0
Copy link
Contributor

Choose a reason for hiding this comment

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

F841 local variable 'missing_value' is assigned to but never used

xfd = 0.958725775
xbeta2 = 0.5
as0 = [0.33243832, 0.16285370, -0.30924818, -0.10324388, 0.11493334,
-6.777104e-02, 1.577425e-03, -1.240906e-02, 3.241678e-02, -3.503695e-02]
Copy link
Contributor

Choose a reason for hiding this comment

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

E128 continuation line under-indented for visual indent

fs2 = as2[0] + xlntaur * as2[1]
del xlntaur
trdown = np.exp(-taur / mus)
trup= np.exp(-taur / muv)
Copy link
Contributor

Choose a reason for hiding this comment

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

E225 missing whitespace around operator

Ttotrayu = ((2 / 3. + muv) + (2 / 3. - muv) * trup) / (4 / 3. + taur)
Ttotrayd = ((2 / 3. + mus) + (2 / 3. - mus) * trdown) / (4 / 3. + taur)
if ao3 != 0:
tO3 = np.exp(G_O3 * ao3)
Copy link
Contributor

Choose a reason for hiding this comment

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

E222 multiple spaces after operator

if ah2o != 0:
tH2O = np.exp(G_H2O * ah2o)

tO2 = np.exp(G_O2 * ao2)
Copy link
Contributor

Choose a reason for hiding this comment

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

E222 multiple spaces after operator

sensor_aa, sensor_za, solar_aa, solar_za = self.get_angles(vis)
else:
vis, sensor_aa, sensor_za, solar_aa, solar_za = self.check_areas(
datasets + optional_datasets)
Copy link
Contributor

Choose a reason for hiding this comment

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

E122 continuation line missing indentation or outdented

solar_aa = solar_aa.data
solar_za = solar_za.data
# angles must be xarrays
sensor_aa = xr.DataArray(sensor_aa, dims=['y','x'])
Copy link
Contributor

Choose a reason for hiding this comment

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

E231 missing whitespace after ','

@codecov
Copy link

codecov bot commented Jun 4, 2018

Codecov Report

Merging #317 into master will increase coverage by 1.53%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #317      +/-   ##
=========================================
+ Coverage   69.27%   70.8%   +1.53%     
=========================================
  Files         123     125       +2     
  Lines       15833   16270     +437     
=========================================
+ Hits        10968   11520     +552     
+ Misses       4865    4750     -115
Impacted Files Coverage Δ
satpy/tests/test_crefl_utils.py 0% <0%> (ø)
satpy/enhancements/__init__.py 77.18% <100%> (+2.18%) ⬆️
satpy/tests/test_enhancements.py 98.55% <100%> (+0.21%) ⬆️
satpy/composites/crefl_utils.py 83.62% <77.94%> (+83.62%) ⬆️
satpy/readers/hdfeos_l1b.py 14.71% <9.67%> (-0.7%) ⬇️
satpy/composites/viirs.py 84.09% <90%> (+5.39%) ⬆️
satpy/tests/compositor_tests/test_viirs.py 99.28% <99.32%> (+0.04%) ⬆️
satpy/tests/test_writers.py 97.68% <0%> (-0.03%) ⬇️
satpy/tests/reader_tests/test_generic_image.py 97.76% <0%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78314dc...cf5640b. Read the comment docs.

@coveralls
Copy link

coveralls commented Jun 4, 2018

Coverage Status

Coverage increased (+1.5%) to 70.805% when pulling cf5640b on wroberts4:feature_abi_rayleigh into 78314dc on pytroll:master.

xph3 = xfd * xbeta2 * 0.375 * (1.0 - mus * mus) * (1.0 - muv * muv)

fs01 = as0[0] + (mus + muv)*as0[1] + (mus * muv)*as0[2] + (mus * mus + muv * muv)*as0[3] + \
(mus * mus * muv * muv)*as0[4]
Copy link
Contributor

Choose a reason for hiding this comment

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

E127 continuation line over-indented for visual indent

fs01 = as0[0] + (mus + muv)*as0[1] + (mus * muv)*as0[2] + (mus * mus + muv * muv)*as0[3] + \
(mus * mus * muv * muv)*as0[4]
fs02 = as0[5] + (mus + muv)*as0[6] + (mus * muv)*as0[7] + (mus * mus + muv * muv)*as0[8] + \
(mus * mus * muv * muv)*as0[9]
Copy link
Contributor

Choose a reason for hiding this comment

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

E127 continuation line over-indented for visual indent

@djhoese
Copy link
Member

djhoese commented Jun 4, 2018

If I recall correctly @wroberts4 this still produces different results that Ralph's original code. Maybe you could try running the same data through this satpy code and Ralph's old code and show us screenshots of the results? Or I guess screenshots once they are equivalent.

Then just unittests to get the coverage score higher and maybe @adybbroe and @mraspaud would agree that this should maybe be moved to composites/__init__.py? Or maybe make the full jump to adding it to pyspectral?

@mraspaud mraspaud added enhancement code enhancements, features, improvements component:compositors labels Jun 5, 2018
@mraspaud
Copy link
Member

mraspaud commented Jun 8, 2018

About moving to composites/__init__.py I don't know, I was just looking at the file the other day, and thought it was starting to get big. Should we maybe start splitting it instead ?

@djhoese
Copy link
Member

djhoese commented Jun 8, 2018

@mraspaud Sounds good to me. Wondering if we should have _rayleigh.py or maybe a atmospheric_correctors.py or something like that?

@mraspaud
Copy link
Member

mraspaud commented Jun 8, 2018

atmospheric_correctors.py sounds good

@@ -38,6 +39,14 @@
LOG = logging.getLogger(__name__)


class IncompatibleAreas(Exception):
Copy link
Member

Choose a reason for hiding this comment

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

This should have been imported from composites/__init__.py

xph3 = xfd * xbeta2 * 0.375 * (1.0 - mus * mus) * (1.0 - muv * muv)

fs01 = as0[0] + (mus + muv)*as0[1] + (mus * muv)*as0[2] + (mus * mus + muv * muv)*as0[3] + \
(mus * mus * muv * muv)*as0[4]
Copy link
Contributor

Choose a reason for hiding this comment

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

E127 continuation line over-indented for visual indent

import xarray.ufuncs as xu

from satpy.composites import CompositeBase, GenericCompositor
from satpy.config import get_environ_ancpath
from satpy.dataset import combine_metadata

LOG = logging.getLogger(__name__)
from satpy.composites import IncompatibleAreas
Copy link
Contributor

Choose a reason for hiding this comment

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

E402 module level import not at top of file
F401 'satpy.composites.IncompatibleAreas' imported but unused

# FIXME: if/when dask can support lazy index arrays then remove this
return sphalb0[index_arr]
sphalb_delayed = dask.delayed(_sphalb_index)(sphalb0, (taur / TAUSTEP4SPHALB + 0.5).astype(np.int32))
sphalb = da.from_delayed(sphalb_delayed, taur.shape, dtype=sphalb0.dtype)
Copy link
Member

Choose a reason for hiding this comment

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

I thought about this while working on a separate feature. I think you could get this to work with map_blocks if you provide index_arr as the first argument, specify the chunks=taur.chunks, and most importantly before passing the sphalb0 array do sphalb0 = sphalb0.rechunk(sphalb0.shape). We can talk about this in person later.

@@ -224,6 +224,214 @@ def test_hncc_dnb(self):
unique, [3.484797e-04, 9.507845e-03, 4.500016e+03])


def test_ReflectanceCorrector_abi(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

E303 too many blank lines (2)

from pyresample.geometry import AreaDefinition
from satpy.composites.viirs import ReflectanceCorrector
from satpy import DatasetID
ref_cor = ReflectanceCorrector(dem_filename= '_fake.hdf', optional_prerequisites= [
Copy link
Contributor

Choose a reason for hiding this comment

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

E251 unexpected spaces around keyword / parameter equals

DatasetID(name='solar_azimuth_angle', wavelength=None, resolution=None, polarization=None, calibration=None,
level=None, modifiers=None),
DatasetID(name='solar_zenith_angle', wavelength=None, resolution=None, polarization=None, calibration=None,
level=None, modifiers=None)], name= 'C01', prerequisites= [],
Copy link
Contributor

Choose a reason for hiding this comment

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

E251 unexpected spaces around keyword / parameter equals

level=None, modifiers=None),
DatasetID(name='solar_zenith_angle', wavelength=None, resolution=None, polarization=None, calibration=None,
level=None, modifiers=None)], name= 'C01', prerequisites= [],
wavelength= (0.45, 0.47, 0.49), resolution= 1000, calibration= 'reflectance',
Copy link
Contributor

Choose a reason for hiding this comment

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

E251 unexpected spaces around keyword / parameter equals

DatasetID(name='solar_zenith_angle', wavelength=None, resolution=None, polarization=None, calibration=None,
level=None, modifiers=None)], name= 'C01', prerequisites= [],
wavelength= (0.45, 0.47, 0.49), resolution= 1000, calibration= 'reflectance',
modifiers= ('sunz_corrected', 'rayleigh_corrected_crefl',), sensor= 'abi')
Copy link
Contributor

Choose a reason for hiding this comment

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

E131 continuation line unaligned for hanging indent
E251 unexpected spaces around keyword / parameter equals

'polarization': None,
'sensor': 'viirs',
'units': units,
'start_time': datetime.datetime(2012,2,25,18,1,24,570942),
Copy link
Contributor

Choose a reason for hiding this comment

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

E231 missing whitespace after ','

'sensor': 'viirs',
'units': units,
'start_time': datetime.datetime(2012,2,25,18,1,24,570942),
'end_time': datetime.datetime(2012,2,25,18,11,21,175760),
Copy link
Contributor

Choose a reason for hiding this comment

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

E231 missing whitespace after ','

'solar_azimuth_angle', 'solar_azimuth_angle', 'degrees')
c05 = make_xarray(None, (), None, ['gitco', 'gimgo'], 'All_Data/{file_group}_All/SolarZenithAngle',
'solar_zenith_angle', 'solar_zenith_angle', 'degrees')
res = ref_cor([c01], [c02,c03,c04,c05])
Copy link
Contributor

Choose a reason for hiding this comment

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

E231 missing whitespace after ','

self.assertEqual(res.attrs['platform_name'], 'Suomi-NPP')
self.assertEqual(res.attrs['sensor'], 'viirs')
self.assertEqual(res.attrs['units'], '%')
self.assertEqual(res.attrs['start_time'], datetime.datetime(2012,2,25,18,1,24,570942))
Copy link
Contributor

Choose a reason for hiding this comment

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

E231 missing whitespace after ','

self.assertEqual(res.attrs['sensor'], 'viirs')
self.assertEqual(res.attrs['units'], '%')
self.assertEqual(res.attrs['start_time'], datetime.datetime(2012,2,25,18,1,24,570942))
self.assertEqual(res.attrs['end_time'], datetime.datetime(2012,2,25,18,11,21,175760))
Copy link
Contributor

Choose a reason for hiding this comment

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

E231 missing whitespace after ','

DatasetID(name='solar_zenith_angle', wavelength=None, resolution=None, polarization=None, calibration=None,
level=None, modifiers=None)], name='C01', prerequisites=[],
wavelength=(0.45, 0.47, 0.49), resolution=1000, calibration='reflectance',
modifiers=('sunz_corrected', 'rayleigh_corrected_crefl',), sensor='abi')
Copy link
Contributor

Choose a reason for hiding this comment

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

E131 continuation line unaligned for hanging indent

from satpy.composites.viirs import ReflectanceCorrector
from satpy import DatasetID
ref_cor = ReflectanceCorrector(dem_filename='_fake.hdf', optional_prerequisites=[
DatasetID(name='satellite_azimuth_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None),
Copy link
Contributor

Choose a reason for hiding this comment

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

E501 line too long (149 > 120 characters)

from satpy import DatasetID
ref_cor = ReflectanceCorrector(dem_filename='_fake.hdf', optional_prerequisites=[
DatasetID(name='satellite_azimuth_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None),
DatasetID(name='satellite_zenith_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None),
Copy link
Contributor

Choose a reason for hiding this comment

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

E501 line too long (148 > 120 characters)

ref_cor = ReflectanceCorrector(dem_filename='_fake.hdf', optional_prerequisites=[
DatasetID(name='satellite_azimuth_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None),
DatasetID(name='satellite_zenith_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None),
DatasetID(name='solar_azimuth_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None),
Copy link
Contributor

Choose a reason for hiding this comment

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

E501 line too long (145 > 120 characters)

DatasetID(name='satellite_azimuth_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None),
DatasetID(name='satellite_zenith_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None),
DatasetID(name='solar_azimuth_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None),
DatasetID(name='solar_zenith_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None)],
Copy link
Contributor

Choose a reason for hiding this comment

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

E501 line too long (145 > 120 characters)

DatasetID(name='satellite_zenith_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None),
DatasetID(name='solar_azimuth_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None),
DatasetID(name='solar_zenith_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None)],
name='I01', prerequisites=[], wavelength=(0.6, 0.64, 0.68), resolution=371, calibration='reflectance',
Copy link
Contributor

Choose a reason for hiding this comment

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

E122 continuation line missing indentation or outdented

DatasetID(name='solar_azimuth_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None),
DatasetID(name='solar_zenith_angle', wavelength=None, resolution=371, polarization=None, calibration=None, level=None, modifiers=None)],
name='I01', prerequisites=[], wavelength=(0.6, 0.64, 0.68), resolution=371, calibration='reflectance',
modifiers=('sunz_corrected_iband', 'rayleigh_corrected_crefl_iband'), sensor='viirs')
Copy link
Contributor

Choose a reason for hiding this comment

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

E122 continuation line missing indentation or outdented

def make_xarray(wavelength, modifiers, resolution, file_type, name, calibration):
return xr.DataArray(dnb,
dims=('y', 'x'),
attrs={ 'wavelength': wavelength, 'level': None, 'modifiers': modifiers,
Copy link
Contributor

Choose a reason for hiding this comment

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

E201 whitespace after '{'

@djhoese djhoese added this to the v0.10 milestone Jul 26, 2018
Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

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

Looks pretty good. I have a few things I'd like you to change and other things that I was just pointing out as reminders to myself to make issues for.

# otherwise make it the same size as the other arrays
sphalb = np.repeat(np.repeat(sphalb, factor, axis=0), factor, axis=1)
from satpy.composites import IncompatibleAreas
raise IncompatibleAreas("")
Copy link
Member

Choose a reason for hiding this comment

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

I would say remove this entire if block. This should theoretically not be possible when called from the modifier (because of check_areas).

avg_elevation = nc.variables[self.dem_sds][:]
# average elevation is stored as a 16-bit signed integer but with
# scale factor 1 and offset 0, convert it to float here
avg_elevation = nc.variables[self.dem_sds][:].astype(np.float)
Copy link
Member

Choose a reason for hiding this comment

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

I think in the long run we'll want this to be dask arrays, but for now let's not worry about it. It probably isn't a performance issue anyway.

@@ -110,6 +110,23 @@ def apply_enhancement(data, func, exclude=None, separate=False,
return data


# pointed to by generic.yaml
def crefl_scaling(img, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

As discussed on slack I think we'll have to make another issue to combine this with the existing crude stretch so that it allows for multiple "control" points.

@@ -1,6 +1,15 @@
sensor_name: visir/abi

modifiers:
# TODO: fix
Copy link
Member

Choose a reason for hiding this comment

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

Remove # TODO. Also future note, usually a todo of fixing something is just # FIXME: description 😉

@@ -198,7 +207,7 @@ enhancements:
operations:
- name: stretch
method: *stretchfun
kwargs: {stretch: linear}
kwargs: {stretch: crude, min_stretch: 0, max_stretch: 120}
Copy link
Member

Choose a reason for hiding this comment

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

@mraspaud @adybbroe @pnuu Thoughts on changing the default reflectance scaling to a static 0 to 120?

satpy/node.py Outdated
@@ -423,12 +423,10 @@ def _find_dependencies(self, dataset_key, **dfilter):
except KeyError:
# exact dataset isn't loaded, let's load it below
pass

Copy link
Member

Choose a reason for hiding this comment

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

Why were these lines removed? Automatic by some auto-correcting tool? Please add the blank lines back in unless they were style errors.

import unittest


class testCreflUtils(unittest.TestCase):
Copy link
Member

Choose a reason for hiding this comment

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

Since your my student @wroberts4 I'm going to ask you to fix the name of this class and the name of your test functions to be PEP8 compatible. TestCreflUtils and test_get_atm_variables_abi

Copy link
Member

Choose a reason for hiding this comment

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

Try to review other tests that you've added.

satpy/node.py Outdated
@@ -472,5 +472,5 @@ def find_dependencies(self, dataset_keys, **dfilter):
continue

self.add_child(self, n)

Copy link
Contributor

Choose a reason for hiding this comment

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

W293 blank line contains whitespace


loader = unittest.TestLoader()
mysuite = unittest.TestSuite()
mysuite.addTest(loader.loadTestsFromTestCase(testCreflUtils))
Copy link
Contributor

Choose a reason for hiding this comment

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

F821 undefined name 'testCreflUtils'

Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

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

Things left to decide:

  1. Should the default reflectance enhancement be changed? If not, then this has to be fixed before this is merged.
  2. The bug fix in resample.py should probably be included in a bug fix release of 0.9.x (cherry-picked if possible).

@djhoese djhoese changed the title [WIP] Add Rayleigh Correction to abi Data Add CREFL rayleigh corrector for ABI data Jul 31, 2018
djhoese added a commit to djhoese/satpy that referenced this pull request Aug 14, 2018
Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

just a comment

@@ -331,6 +363,73 @@ def get_atm_variables(mus, muv, phi, height, coeffs):
return sphalb, rhoray, TtotraytH2O, tOG


def get_atm_variables_abi(mus, muv, phi, height, coeffs, G_O3, G_H2O, G_O2):
Copy link
Member

Choose a reason for hiding this comment

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

there seems to be a lot of duplicated code in this function from existing function, can we factorise ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct! I cooked something up and will push it later today or Monday (after I test to make sure I didn't break it).

G_H2O = G_calc(solar_zenith, a_H2O) + G_calc(sensor_zenith, a_H2O)
G_O2 = G_calc(solar_zenith, a_O2) + G_calc(sensor_zenith, a_O2)
# Note: bh2o values are actually ao2 values for abi
sphalb, rhoray, TtotraytH2O, tOG = get_atm_variables_abi(mus, muv, phi, height, *coeffs, G_O3, G_H2O, G_O2)
Copy link
Contributor

Choose a reason for hiding this comment

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

E999 SyntaxError: only named arguments may follow *expression

calibration: reflectance
calibration:
reflectance:
units: "%"
Copy link
Member

Choose a reason for hiding this comment

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

Do we know that the units coming out of this are % (0-~120) and not 1 (0 - ~1.2)?

@@ -12,15 +21,15 @@ composites:
modifiers: [sunz_corrected]
standard_name: true_color

true_color:
true_color_crefl:
Copy link
Member

Choose a reason for hiding this comment

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

Please reset this to be true_color and to use rayleigh_corrected instead of rayleigh_corrected_crefl. Feel free to leave the rayleigh_corrected_crefl definition above. Make sure that this true color composite still loads successfully.

0.0043149700000000004, 0.0037296,
0.014107995000000002, 0.052349)
if abs(np.array(sphalb) - 0.045213532544630494) >= 1e-10:
raise AssertionError('{} is not within {} of {}'.format(np.array(sphalb), 1e-10, 0.045213532544630494))
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I should have noticed these earlier. These should be replaced with assert statements or self.assertX calls.

self.assertEqual(ref_cor.attrs['sensor'], 'abi')
self.assertEqual(ref_cor.attrs['prerequisites'], [])
self.assertEqual(ref_cor.attrs['optional_prerequisites'], [
DatasetID(name='satellite_azimuth_angle', wavelength=None, resolution=None, polarization=None,
Copy link
Member

Choose a reason for hiding this comment

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

The default for DatasetID is None so these can all be left empty (except for maybe modifiers which defaults to an empty tuple).

self.assertEqual(res.attrs['ancillary_variables'], [])
data = res.values
if abs(np.mean(data) - 29.907390988422513) >= 1e-10:
raise AssertionError('{} is not within {} of {}'.format(np.mean(data), 1e-10, 29.907390988422513))
Copy link
Member

Choose a reason for hiding this comment

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

Replace this with self.assertX call or regular assert statement at least.

from satpy.composites.viirs import ReflectanceCorrector
from satpy import DatasetID
ref_cor = ReflectanceCorrector(dem_filename='_fake.hdf', optional_prerequisites=[
DatasetID(name='satellite_azimuth_angle', wavelength=None, resolution=371, polarization=None, calibration=None,
Copy link
Member

Choose a reason for hiding this comment

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

No need to list each None element here.

self.assertEqual(ref_cor.attrs['prerequisites'], [])
self.assertEqual(ref_cor.attrs['optional_prerequisites'], [
DatasetID(name='satellite_azimuth_angle', wavelength=None, resolution=371, polarization=None,
calibration=None, level=None, modifiers=None),
Copy link
Member

Choose a reason for hiding this comment

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

Same here.


area, dnb = self.data_area_ref_corrector()

def make_xarray(wavelength, modifiers, calibration, file_type, file_key, name, standard_name, units):
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it is github's weird formatting but this looks like it needs a self as the first argument.

self.assertEqual(res.attrs['ancillary_variables'], [])
data = res.values
if abs(np.mean(data) - 40.7578684169142) >= 1e-10:
raise AssertionError('{} is not within {} of {}'.format(np.mean(data), 1e-10, 40.7578684169142))
Copy link
Member

Choose a reason for hiding this comment

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

Replace with self.assertX

import datetime
from satpy.composites.viirs import ReflectanceCorrector
from satpy import DatasetID
sataa_did = DatasetID(name='satellite_azimuth_angle', wavelength=None, resolution=None, polarization=None,
Copy link
Member

Choose a reason for hiding this comment

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

None parameters can be removed

self.assertEqual(ref_cor.attrs['sensor'], 'modis')
self.assertEqual(ref_cor.attrs['prerequisites'], [])
self.assertEqual(ref_cor.attrs['optional_prerequisites'], [
DatasetID(name='satellite_azimuth_angle', wavelength=None, resolution=None, polarization=None,
Copy link
Member

Choose a reason for hiding this comment

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

Same here

@djhoese djhoese merged commit fa72f94 into pytroll:master Aug 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:compositors enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants