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

Enable solar zenith angle caching for the DayNightCompositor #2105

Merged
merged 7 commits into from
May 6, 2022

Conversation

simonrp84
Copy link
Member

At present, the DayNightCompositor does not use the new lat/lon cacheing feature, which slows down processing of this type of composite.
This PR switches the compositor to use the ready-made cache features in angles.py. It requires a small modification to the get_cos_sza in that file to deal with chunking of RGB composites.

  • Tests added

satpy/modifiers/angles.py Outdated Show resolved Hide resolved
@simonrp84 simonrp84 force-pushed the chunkify_daynightcomposite branch from 7ef5511 to e58657d Compare May 5, 2022 14:19
satpy/composites/__init__.py Outdated Show resolved Hide resolved
satpy/modifiers/angles.py Outdated Show resolved Hide resolved
@djhoese djhoese changed the title Enable caching for the DayNightCompositor Enable solar zenith angle caching for the DayNightCompositor May 5, 2022
@djhoese djhoese added enhancement code enhancements, features, improvements component:compositors optimization cleanup Code cleanup but otherwise no change in functionality labels May 5, 2022
@djhoese djhoese added this to In progress in PCW Spring 2022 via automation May 5, 2022
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.

A couple changes...and what do you think about tests? Maybe add an RGB case to the existing angles tests?

satpy/composites/__init__.py Show resolved Hide resolved
satpy/composites/__init__.py Show resolved Hide resolved
satpy/modifiers/angles.py Outdated Show resolved Hide resolved
@simonrp84
Copy link
Member Author

Yeah, this probably needs some new tests (at least the new helper function). I don't have a huge amount of time right now but can try to add some next week.

@djhoese
Copy link
Member

djhoese commented May 5, 2022

Ok, I think this is done. It didn't actually work the way you had it (something I didn't notice either). The test cases don't have dims defined so I add a default that assumes (y, x). I added tests with RGB with no dims and RGB with dims.

@pnuu pnuu moved this from In progress to Ready for review in PCW Spring 2022 May 6, 2022
Copy link
Member

@pnuu pnuu left a comment

Choose a reason for hiding this comment

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

LGTM

@pnuu
Copy link
Member

pnuu commented May 6, 2022

The TestDayNightCompositor.test_daynight_area test fails with

self = {'name': 'dn_test', 'optional_prerequisites': [], 'prerequisites': []}
projectables = [<xarray.DataArray 'array-4105b65f4b41e351f1402df7e0f94aca' (bands: 3, y: 2, x: 2)>
dask.array<array, shape=(3, 2, 2),...
Attributes:
    test:        b
    start_time:  2018-01-01 18:00:00
    area:        <MagicMock id='139787571969568'>]
kwargs = {}
foreground_data = <xarray.DataArray 'array-4105b65f4b41e351f1402df7e0f94aca' (bands: 3, y: 2, x: 2)>
dask.array<array, shape=(3, 2, 2), ...x
Attributes:
    test:        a
    start_time:  2018-01-01 18:00:00
    area:        <MagicMock id='139787571969568'>
lim_low = 0.08715574274765814, lim_high = 0.03489949670250108
get_cos_sza = <function get_cos_sza at 0x7f22fdc6c790>

    def __call__(self, projectables, **kwargs):
        """Generate the composite."""
        projectables = self.match_data_arrays(projectables)
        # At least one composite is requested.
        foreground_data = projectables[0]
    
        lim_low = np.cos(np.deg2rad(self.lim_low))
        lim_high = np.cos(np.deg2rad(self.lim_high))
        try:
>           coszen = np.cos(np.deg2rad(projectables[2 if self.day_night == "day_night" else 1]))
E           IndexError: list index out of range

satpy/composites/__init__.py:632: IndexError

During handling of the above exception, another exception occurred:

self = <satpy.tests.test_composites.TestDayNightCompositor testMethod=test_daynight_area>

    def test_daynight_area(self):
        """Test compositor both day and night portions when SZA data is not provided."""
        from satpy.composites import DayNightCompositor
        comp = DayNightCompositor(name='dn_test', day_night="day_night")
>       res = comp((self.data_a, self.data_b))

satpy/tests/test_composites.py:351: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
satpy/composites/__init__.py:637: in __call__
    coszen = get_cos_sza(foreground_data)
satpy/modifiers/angles.py:369: in get_cos_sza
    lons, lats = _get_valid_lonlats(data_arr.attrs["area"], chunks)
satpy/modifiers/angles.py:142: in __call__
    arg_hash = _hash_args(*new_args, unhashable_types=self._uncacheable_arg_types)
satpy/modifiers/angles.py:258: in _hash_args
    arg_hash.update(json.dumps(tuple(hashable_args)).encode('utf8'))
/usr/share/miniconda3/envs/test-environment/lib/python3.9/json/__init__.py:231: in dumps
    return _default_encoder.encode(obj)
/usr/share/miniconda3/envs/test-environment/lib/python3.9/json/encoder.py:199: in encode
    chunks = self.iterencode(o, _one_shot=True)
/usr/share/miniconda3/envs/test-environment/lib/python3.9/json/encoder.py:257: in iterencode
    return _iterencode(o, 0)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <json.encoder.JSONEncoder object at 0x7f231941a700>
o = <MagicMock id='139787571969568'>

    def default(self, o):
        """Implement this method in a subclass such that it returns
        a serializable object for ``o``, or calls the base implementation
        (to raise a ``TypeError``).
    
        For example, to support arbitrary iterators, you could
        implement default like this::
    
            def default(self, o):
                try:
                    iterable = iter(o)
                except TypeError:
                    pass
                else:
                    return list(iterable)
                # Let the base class default method raise the TypeError
                return JSONEncoder.default(self, o)
    
        """
>       raise TypeError(f'Object of type {o.__class__.__name__} '
                        f'is not JSON serializable')
E       TypeError: Object of type MagicMock is not JSON serializable

@djhoese
Copy link
Member

djhoese commented May 6, 2022

MOCKING!?!?!?!

Edit: ...I'll see what I can do.

@codecov
Copy link

codecov bot commented May 6, 2022

Codecov Report

Merging #2105 (d503619) into main (56eb21b) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2105      +/-   ##
==========================================
+ Coverage   93.92%   93.95%   +0.02%     
==========================================
  Files         283      283              
  Lines       42715    42877     +162     
==========================================
+ Hits        40120    40285     +165     
+ Misses       2595     2592       -3     
Flag Coverage Δ
behaviourtests 4.75% <6.45%> (+0.04%) ⬆️
unittests 94.51% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
satpy/composites/__init__.py 90.14% <100.00%> (+0.19%) ⬆️
satpy/modifiers/angles.py 99.14% <100.00%> (+0.05%) ⬆️
satpy/tests/test_composites.py 100.00% <100.00%> (ø)
satpy/tests/test_modifiers.py 100.00% <100.00%> (ø)
satpy/readers/fci_l2_nc.py 99.45% <0.00%> (-0.01%) ⬇️
satpy/tests/reader_tests/test_seviri_l1b_icare.py 100.00% <0.00%> (ø)
satpy/readers/fci_l1c_nc.py 98.05% <0.00%> (+0.02%) ⬆️
satpy/tests/test_yaml_reader.py 99.60% <0.00%> (+0.03%) ⬆️
satpy/readers/yaml_reader.py 97.27% <0.00%> (+0.44%) ⬆️

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 56eb21b...d503619. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 94.45% when pulling d503619 on simonrp84:chunkify_daynightcomposite into 56eb21b on pytroll:main.

@djhoese
Copy link
Member

djhoese commented May 6, 2022

Ok with Panu's approval and the simple fix I've made since then, I think this is good to go. Merging...

@djhoese djhoese merged commit d034ebb into pytroll:main May 6, 2022
PCW Spring 2022 automation moved this from Ready for review to Done May 6, 2022
@simonrp84
Copy link
Member Author

Thanks for your reviews / coding on this @pnuu and @djhoese !

@simonrp84 simonrp84 deleted the chunkify_daynightcomposite branch May 6, 2022 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code cleanup but otherwise no change in functionality component:compositors enhancement code enhancements, features, improvements optimization
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants