IMPRO-1917: Boolean output are now int8 from these plugins/CLIs: #1362
Conversation
- CorrectLandSeaMask; generate-landmask-ancillary - PrecipPhaseProbability; precipphase - SignificantPhaseMask; phase-mask - calculate_sleet_prob - if inputs are int8
Codecov Report
@@ Coverage Diff @@
## master #1362 +/- ##
==========================================
- Coverage 96.18% 96.18% -0.01%
==========================================
Files 85 85
Lines 7285 7284 -1
==========================================
- Hits 7007 7006 -1
Misses 278 278
Continue to review full report at Codecov.
|
| ones = np.ones((prob_of_snow.shape), dtype="float32") | ||
| sleet_prob = ones - (prob_of_snow.data + prob_of_rain.data) | ||
| if np.any(sleet_prob < 0.0): | ||
| sleet_prob = 1 - (prob_of_snow.data + prob_of_rain.data) |
There was a problem hiding this comment.
Why is it a problem to cast probabilities to float32?
There was a problem hiding this comment.
I don't think there's any reason to cast the ones here to float32 as int - float32 = float32 (it's also about twice as fast to do it this way)
There was a problem hiding this comment.
Chris is right. By casting the ones as float32, the code was forcing the output to be at least float32 precision. Now it is determined by Numpy based on the highest precision of the snow and rain inputs. Other changes in this PR mean they will be int8, so the output here is int8 too. If they are float32 fractions, then the output here would be a float32 fraction too.
There was a problem hiding this comment.
Ah OK, I misunderstood. I thought the data coming in were float32 probabilities.
Could you update the docstring please to clarify that these are actually integer / boolean masks, not probabilities? Otherwise this is fine.
| self.assertIsInstance(result, iris.cube.Cube) | ||
| self.assertEqual(result.name(), "probability_of_rain_at_surface") | ||
| self.assertEqual(result.units, Unit("1")) | ||
| self.assertArrayAlmostEqual(result.data, expected) | ||
| self.assertTrue(result.dtype == np.int8) | ||
|
|
||
| def test_unit_conversion(self): |
There was a problem hiding this comment.
I think there are two more tests in this file whose datatypes need converting.
There was a problem hiding this comment.
I've moved these meta-data tests into a function that is called from each test instead. This avoids repetition.
- Moves meta-data tests into a common function and uses it everywhere that the output are tested. - Moves creation of expected data array template into setup function.
…phase-change-radius * remotes/origin/master: IMPRO-1917: Boolean output are now int8 from these plugins/CLIs: (metoppv#1362) IMPRO-1815: reject float64 data in combine plugin (metoppv#1363)
…oppv#1362) * Boolean output are now int8 from these plugins/CLIs: - CorrectLandSeaMask; generate-landmask-ancillary - PrecipPhaseProbability; precipphase - SignificantPhaseMask; phase-mask - calculate_sleet_prob - if inputs are int8 * Blackify * Refactors test_PrecipPhaseProbability.py - Moves meta-data tests into a common function and uses it everywhere that the output are tested. - Moves creation of expected data array template into setup function. * Makes doc-string clearer in calculate_sleet_prob
Testing: