Analysis percentile method - update applicability test for fast_percentile_method

[bayliffe]

Since Iris 2 was introduced all cube data arrays have become MaskedArray type, though by default no mask is set. The fast_percentile_method implemented in analysis uses the test isMaskedArray to determine if the input cube is masked, which precludes use of the fast percentile method, however this is overly restrictive. The now default MaskedArray type used for cubes does not by default have a mask set (instead it is simply mask=False). If no mask is set, the faster numpy method works fine and remains orders of magnitude faster that the scipy equivalent.

This PR changes the test to use np.ma.is_masked which differentiates between a MaskedArray with and without an applied mask. This enables the use of the fast_percentile_method once again for most cubes.

Tests have been added to cover the case of using a MaskedArray with no set mask. The functions used in the unit tests have also been modified for all tests to check that the returned cube.data type matches that which was passed in.

[rcomer]

Hi @bayliffe, at least some of your Travis test failures are things that have been fixed in the master branch since you submitted this. Can I suggest you try rebasing to pick up the changes?

[bjlittle]

Hey @bayliffe, thanks for this PR. Would you mind rebasing?

[bayliffe]

@bjlittle
PR rebased, and now travis is my friend. Thanks for the prompt (sorry I missed the previous one).

[pp-mo]

@bjlittle @rcomer from what I see this can be merged,
but perhaps we were all waiting for the others ??

[rcomer]

@pp-mo I have not read through in detail so, if you have, please go ahead. I’m looking forward to taking the workarounds out of my user code 😀

[bjlittle]

@pp-mo @rcomer There's a bigger question here, as to whether we deal with this in a systemic way across iris i.e., should all masked arrays with no mask points automatically be converted to ordinary arrays?

I'd pretty much say yes, given the poor performance of masked arrays, see SciTools/iris-agg-regrid#37

Anyways, regardlessly, I'm banking this... Awesome thanks @bayliffe 👍

[rcomer]

@bjlittle I guess I’d want to understand the rationale for the NetCDF4 folks making this change, and then decide if I agree with it!

I don’t know what iris-agg-regrid does, but we have a similar check in the area weighted regridder. Fortunately that one already uses is_masked.

[bjlittle]

@rcomer See https://github.com/Unidata/netcdf4-python and the associated news for release 1.4.0. There have been many breaking changes IMHO in minor releases which have caused issues to the community. I don't think we can reasonably expect netcdf4-python to change or back out certain behaviour that we take issue with... Well I don't think so.

[rcomer]

@bjlittle sorry, I wasn’t clear. I meant that if their rationale for this behaviour makes sense, then it might also make sense for Iris and therefore we shouldn’t override it. Wasn’t thinking we could ask them to revert it.

[rcomer]

Possibly related: #2046 #2856

[bjlittle]

@rcomer I'm assuming their rational is related to consistency of the type of array exposed to users i.e., no need to check if it's masked or not - everything is just masked. That sounds attractive, as you don't need to "if" and "but" within application code... however, this rational is independent to performance. If I'm guessing right, they bought in to the utility of consistency, but at the cost of perhaps being (4-5 times) slower, but surely unbeknownst to them.

Having such consistency at the cost of performance is a pretty hard sell... if only I got a penny from every scientist who wanted their Python code to run faster 🤪