-
Notifications
You must be signed in to change notification settings - Fork 0
Convert prsn data from mm to cm #113
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,10 @@ | |
| pr_freezing_from_units, create_prsn_netcdf_from_source, \ | ||
| create_filepath_from_source, has_required_vars, matching_datasets, \ | ||
| check_pr_units, process_to_prsn, convert_temperature_units | ||
| from pint import UnitRegistry | ||
|
|
||
| ureg = UnitRegistry() | ||
| Q_ = ureg.Quantity | ||
|
|
||
| @pytest.mark.parametrize('arrays', [ | ||
| ({'shape1': np.arange(10).reshape(2, 5), 'shape2': np.arange(10).reshape(2, 5)}), | ||
|
|
@@ -67,7 +70,7 @@ def test_create_prsn_netcdf_from_source(tiny_dataset, fake_dataset): | |
| assert np.shape(fake_dataset.variables['prsn']) == (11688, 4, 2) | ||
| assert fake_dataset.variables['prsn'].standard_name == 'snowfall_flux' | ||
| assert fake_dataset.variables['prsn'].long_name == 'Precipitation as Snow' | ||
|
|
||
| assert fake_dataset.variables['prsn'].units == 'g cm-2 s-1' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's not actually any check on the data values in this test, so it's hard to know whether it's doing the conversion correctly. Could you work out what the sum snowfall in this data should be an add that to the test?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe the dataset used for result = fake_dataset.variables['prsn'][:]
result = np.where(result != 0)
for array in result:
assert len(array) == 0Is there possibly another dataset I could use to test the conversion?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh right, you had mentioned that. Well that's not a great testing dataset, then is it?! :) Maybe you could throw a few strategic low temperature values into the test data set... it might throw some other tests out of whack, but you could adjust as necessary.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we're only concerned about the conversion, would it be ok if I store the |
||
|
|
||
| @pytest.mark.parametrize('tiny_dataset, new_var, expected', [ | ||
| ('downscaled_pr', 'prsn', 'prsn_day_BCCAQ2_ACCESS1-0_ACCESS1-0+historical+rcp45+r1i1p1_r1i1p1_19600101-19911231.nc'), | ||
|
|
@@ -151,6 +154,13 @@ def test_process_to_prsn(pr, tasmin, tasmax, fake_dataset): | |
| for array in result: | ||
| assert len(array) == 0 | ||
|
|
||
| # Test conversion from mm to cm | ||
| pr_mm = pr_dataset.variables['pr'][:] | ||
| pr_units_from = ureg.parse_units('kg/m^2/s') | ||
| pr_units_to = ureg.parse_units('g/cm^2/s') | ||
| pr_cm = pr_mm * Q_(1.0, pr_units_from).to(pr_units_to).magnitude | ||
| assert pr_cm == pytest.approx(pr_mm/10.0) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize('units_from, units_to, expected', [ | ||
| ('degC', 'K', 273.15), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you tell me more about using this as the target unit? You've changed both the mass (kg -> g) and the area (per square meter to per square centimeter). Is that definitely what we want? And is it definitely correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Converting from kg -> g multiplies the data by 1000 and converting from /m^2 -> /cm^2 divides the data by 10000, giving the net effect of dividing the data by 10. Since converting from mm to cm also divides by 10, I think this will work. I can check with Lee when he's back.