Skip to content
This repository has been archived by the owner on Aug 28, 2023. It is now read-only.

Refactor Time Aggregation #162

Merged
merged 13 commits into from
Dec 21, 2016
Merged

Refactor Time Aggregation #162

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
1140907
Refactor time aggregation into base Indicator class
Dec 8, 2016
f3a29c1
Update unit tests to specify time aggregation
Dec 12, 2016
650d3fc
Add time_aggregation and valid_aggregations to Indicator response
Dec 19, 2016
a046c74
Add documentation for time_aggregation parameter
Dec 19, 2016
3309c22
Remove references to implied aggregation from indicator descriptions
Dec 19, 2016
c7f7e25
Tweak TotalPrecipitation to give better monthly values
Dec 19, 2016
908e40e
Remove references to year from descriptions
Dec 20, 2016
78a6e7a
PR review changes
Dec 20, 2016
8b27963
Abstract unit conversion tests so all combinations are tested automat…
Dec 15, 2016
fa6423e
Refactor MonthRanges to simplify output format
Dec 20, 2016
df9faad
Simplify YearlySequenceIndicator logic
Dec 20, 2016
de9be08
Introduce static units for total precipitation
Dec 20, 2016
61a2a0d
Remove outdated boilerplate from indicator descriptions
Dec 20, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions django/climate_change_api/indicators/indicators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@

from .abstract_indicators import (Indicator, CountIndicator, BasetempIndicatorMixin,
YearlyMaxConsecutiveDaysIndicator, YearlySequenceIndicator)
from .unit_converters import (TemperatureUnitsMixin, PrecipUnitsMixin, DaysUnitsMixin,
CountUnitsMixin, TemperatureDeltaUnitsMixin)
from .unit_converters import (TemperatureUnitsMixin, PrecipUnitsMixin, PrecipRateUnitsMixin,
DaysUnitsMixin, CountUnitsMixin, TemperatureDeltaUnitsMixin,
SECONDS_PER_DAY)


##########################
Expand Down Expand Up @@ -54,8 +55,11 @@ class TotalPrecipitation(PrecipUnitsMixin, Indicator):
description = 'Total precipitation'
valid_aggregations = ('yearly', 'monthly',)
variables = ('pr',)
# Precipitation is stored per-second, and we want a total for all days in the aggregation,
# so we need to multiple each day's value by 86400 to get the total for that day and then
# sum the results
expression = F('pr') * SECONDS_PER_DAY
Copy link
Contributor

Choose a reason for hiding this comment

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

👍
about_the_right_amount_of_rain

agg_function = Sum
default_units = 'in/day'


class FrostDays(DaysUnitsMixin, CountIndicator):
Expand Down Expand Up @@ -210,7 +214,7 @@ class HighTemperature(TemperatureUnitsMixin, Indicator):
variables = ('tasmax',)


class Precipitation(PrecipUnitsMixin, Indicator):
class Precipitation(PrecipRateUnitsMixin, Indicator):
label = 'Precipitation'
description = ('Daily precipitation averaged across all requested models')
valid_aggregations = ('daily',)
Expand Down
40 changes: 20 additions & 20 deletions django/climate_change_api/indicators/tests/test_indicators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,17 +170,17 @@ class YearlyTotalPrecipitationTestCase(IndicatorTests, TestCase):
indicator_class = indicators.TotalPrecipitation
indicator_name = 'total_precipitation'
time_aggregation = 'yearly'
units = 'kg/m^2/s'
test_indicator_rcp85_equals = {2000: {'avg': 35.0, 'min': 30.0, 'max': 40.0}}
test_indicator_rcp45_equals = {2000: {'max': 20.0, 'avg': 15.0, 'min': 10.0},
2001: {'max': 20.0, 'avg': 15.0, 'min': 10.0},
2002: {'avg': 10.0, 'max': 10.0, 'min': 10.0},
units = 'kg/m^2'
test_indicator_rcp85_equals = {2000: {'avg': 3024000.0, 'min': 2592000, 'max': 3456000}}
test_indicator_rcp45_equals = {2000: {'max': 1728000, 'avg': 1296000, 'min': 864000},
2001: {'max': 1728000, 'avg': 1296000, 'min': 864000},
2002: {'avg': 864000, 'max': 864000, 'min': 864000},
2003: {'avg': 0.0, 'max': 0.0, 'min': 0.0}}
test_years_filter_equals = {2001: {'max': 20.0, 'avg': 15.0, 'min': 10.0},
2002: {'avg': 10.0, 'max': 10.0, 'min': 10.0}}
test_models_filter_equals = {2000: {'avg': 10.0, 'max': 10.0, 'min': 10.0},
2001: {'avg': 10.0, 'max': 10.0, 'min': 10.0},
2002: {'avg': 10.0, 'max': 10.0, 'min': 10.0},
test_years_filter_equals = {2001: {'max': 1728000, 'avg': 1296000, 'min': 864000},
2002: {'avg': 864000, 'max': 864000, 'min': 864000}}
test_models_filter_equals = {2000: {'avg': 864000, 'max': 864000, 'min': 864000},
2001: {'avg': 864000, 'max': 864000, 'min': 864000},
2002: {'avg': 864000, 'max': 864000, 'min': 864000},
2003: {'avg': 0.0, 'max': 0.0, 'min': 0.0}}


Expand Down Expand Up @@ -416,17 +416,17 @@ class MonthlyTotalPrecipitationTestCase(IndicatorTests, TestCase):
indicator_class = indicators.TotalPrecipitation
indicator_name = 'total_precipitation'
time_aggregation = 'monthly'
units = 'kg/m^2/s'
test_indicator_rcp85_equals = {'2000-01': {'avg': 35.0, 'min': 30.0, 'max': 40.0}}
test_indicator_rcp45_equals = {'2000-01': {'max': 20.0, 'avg': 15.0, 'min': 10.0},
'2001-01': {'max': 20.0, 'avg': 15.0, 'min': 10.0},
'2002-01': {'avg': 10.0, 'max': 10.0, 'min': 10.0},
units = 'kg/m^2'
test_indicator_rcp85_equals = {'2000-01': {'avg': 3024000, 'min': 2592000, 'max': 3456000}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did the test condition for the TotalPrecipitation indicators (both yearly/monthly) change? Does it make sense that both aggregation levels have the same totals and responses?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test condition changed because it was previously looking for kg/m^2/s, which isn't an option anymore... we're giving the total rainfall, which is the rainfall per day summed across the whole year, and since the raw data is in rainfall per second everything had to be scaled up by 84600.

And unfortunately upon inspection it does make sense that the monthly and yearly have the same values... every single one of our test data points is set for January 1st. We need a lot more test data to start to get diverging results.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha. Thanks for clarifying.

test_indicator_rcp45_equals = {'2000-01': {'max': 1728000, 'avg': 1296000.0, 'min': 864000},
'2001-01': {'max': 1728000, 'avg': 1296000.0, 'min': 864000},
'2002-01': {'avg': 864000, 'max': 864000, 'min': 864000},
'2003-01': {'avg': 0.0, 'max': 0.0, 'min': 0.0}}
test_years_filter_equals = {'2001-01': {'max': 20.0, 'avg': 15.0, 'min': 10.0},
'2002-01': {'avg': 10.0, 'max': 10.0, 'min': 10.0}}
test_models_filter_equals = {'2000-01': {'avg': 10.0, 'max': 10.0, 'min': 10.0},
'2001-01': {'avg': 10.0, 'max': 10.0, 'min': 10.0},
'2002-01': {'avg': 10.0, 'max': 10.0, 'min': 10.0},
test_years_filter_equals = {'2001-01': {'max': 1728000, 'avg': 1296000.0, 'min': 864000},
'2002-01': {'avg': 864000, 'max': 864000, 'min': 864000}}
test_models_filter_equals = {'2000-01': {'avg': 864000, 'max': 864000, 'min': 864000},
'2001-01': {'avg': 864000, 'max': 864000, 'min': 864000},
'2002-01': {'avg': 864000, 'max': 864000, 'min': 864000},
'2003-01': {'avg': 0.0, 'max': 0.0, 'min': 0.0}}


Expand Down
15 changes: 13 additions & 2 deletions django/climate_change_api/indicators/tests/test_unit_converters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django.test import TestCase
from indicators.tests.mixins import ConverterTestMixin
from indicators.unit_converters import TemperatureConverter, PrecipitationConverter
from indicators.unit_converters import (TemperatureConverter, PrecipitationConverter,
PrecipitationRateConverter)


class TemperatureConverterTestCase(ConverterTestMixin, TestCase):
Expand All @@ -18,8 +19,18 @@ class TemperatureConverterTestCase(ConverterTestMixin, TestCase):
]


class PrecipitationRateConverterTestCase(ConverterTestMixin, TestCase):
converter_class = PrecipitationRateConverter
cases = [
{'kg/m^2/s': 1, 'kg/m^2/day': 86400},
{'kg/m^2/s': 0.00001157407, 'kg/m^2/day': 1}
]


class PrecipitationConverterTestCase(ConverterTestMixin, TestCase):
converter_class = PrecipitationConverter
cases = [
{'kg/m^2/s': 1, 'kg/m^2/day': 86400}
{'kg/m^2': 1, 'mm': 1, 'in': 0.0393701},
{'kg/m^2': 25.4, 'mm': 25.4, 'in': 1},
{'kg/m^2': 254, 'mm': 254, 'in': 10}
]
34 changes: 24 additions & 10 deletions django/climate_change_api/indicators/unit_converters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,8 @@ class TemperatureDeltaConverter(LinearConverter):
}


class PrecipitationConverter(LinearConverter):
""" Define units for precipitation

The units are rates, so cumulative totals can be had either by averaging the rates then
converting to the desired interval (i.e. average kg/m^2/s -> kg/m^2/year) or by converting
to an interval and summing all values for that interval across the desired interval
(i.e. convert each day's rate to kg/m^2/day and sum across the days in the month or year)

The former is to be preferred, since the latter is basically doing part of this unit conversion
by hand and will put the values out of sync with what the object believes its units to be.
class PrecipitationRateConverter(LinearConverter):
""" Define units for rate of precipitation

To convert from mass/area/second to height, we're assuming 1kg water == .001 m^3 which makes
kg/m^2 equivalent to millimeters.
Expand All @@ -97,6 +89,22 @@ class PrecipitationConverter(LinearConverter):
}


class PrecipitationConverter(LinearConverter):
""" Define units for precipitation

Internally precipitation is stored as mass per area per time, but for total precipitation
we need to present the result as a fixed amount... either a mass per area or linear distance.

To convert from mass/area to height, we're assuming 1kg water == .001 m^3 which makes
kg/m^2 equivalent to millimeters.
"""
units = {
'kg/m^2': 1,
'mm': 1,
'in': INCHES_PER_MILLIMETER
}


##########################
# Mixin classes

Expand Down Expand Up @@ -127,6 +135,12 @@ class TemperatureDeltaUnitsMixin(TemperatureUnitsMixin):

class PrecipUnitsMixin(ConversionMixin):
converter_class = PrecipitationConverter
storage_units = 'kg/m^2'
default_units = 'in'


class PrecipRateUnitsMixin(ConversionMixin):
converter_class = PrecipitationRateConverter
storage_units = 'kg/m^2/s'
default_units = 'in/day'

Expand Down