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

DeprecationWarnings #4574

Closed
1 of 2 tasks
rcomer opened this issue Feb 9, 2022 · 11 comments
Closed
1 of 2 tasks

DeprecationWarnings #4574

rcomer opened this issue Feb 9, 2022 · 11 comments

Comments

@rcomer
Copy link
Member

rcomer commented Feb 9, 2022

📰 Custom Issue

A couple of DeprecationWarnings I've spotted that should probably be addressed at some point. They can be reproduced by running test modules, but don't seem to show up in the CI.

Steps to reproduce

  • Check out a current branch
  • Activate an up-to-date iris-dev environment
[git-path]/iris/lib/iris/analysis/_area_weighted.py:606: DeprecationWarning: Out of bound index found. This was previously ignored when the indexing result contained no elements. In the future the index error will be raised. This error occurs either due to an empty slice, or if an array has zero elements even before indexing.
  (Use `warnings.simplefilter('error')` to turn this DeprecationWarning into an error and get more details on the invalid index.)
    src_area_datas_square = src_data[
[conda-path]/iris-dev/lib/python3.8/site-packages/nc_time_axis/__init__.py:41: DeprecationWarning: CalendarDateTime is obsolete and will be deprecated in nc_time_axis version 1.5.  Please consider switching to plotting instances or subclasses of cftime.datetime directly.
@pp-mo
Copy link
Member

pp-mo commented Feb 9, 2022

can be reproduced by running test modules, but don't seem to show up in the CI.

Yes, I've been seeing some of these.
I expect the "only shows a warning once" is why you don't see them in the CI.

I'm also getting a persistent error in my envs at present from numpy :

lib/ugrid_checks/nc_dataset_scan.py:136: in scan_dataset
    import netCDF4 as nc
/home/h05/itpp/.conda/envs/iris_new/lib/python3.8/site-packages/netCDF4/__init__.py:3: in <module>
    from ._netCDF4 import *
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

>   ???
E   RuntimeWarning: numpy.ndarray size changed, may indicate binary incompatibility. Expected 16 from C header, got 88 from PyObject

src/netCDF4/_netCDF4.pyx:1: RuntimeWarning

That looks to me like a compatibilty problem between numpy + netCDF4 versions, but I haven't managed to fix it.
Have you seen this one ?

@rcomer
Copy link
Member Author

rcomer commented Feb 9, 2022

@pp-mo I'm not seeing that from the Iris tests, but when testing my user code with the v3.2 release candidate I got some like this:

<frozen importlib._bootstrap>:228: RuntimeWarning: numpy.ndarray size changed, may indicate binary incompatibility. Expected 16 from C header, got 88 from PyObject
<frozen importlib._bootstrap>:241: RuntimeWarning: numpy.ndarray size changed, may indicate binary incompatibility. Expected 16 from C header, got 96 from PyObject

I can't say I even attempted to understand them!

@pp-mo
Copy link
Member

pp-mo commented Feb 10, 2022

I can't say I even attempted to understand them!

😄
I haven't seen it in our tests, either.
I'd like to chase it, but initially it seems very hard to work out what is responsible .
My usage was based on PyTest,. There you can use a magic "-W" setting to cause the warning to error+stackdump. From which, it seems to occur when netCDF4 is first imported. Beyond that I ran out of time / patience / enthusiasm ...

Presumably it is a clib / numpy / netCDF4 version or compatibility problem. So may not apply to the CI envs after all

@rcomer
Copy link
Member Author

rcomer commented Feb 12, 2022

@pp-mo do you have a theory about why

pytest -W error::DeprecationWarning lib/iris/tests/integration/analysis/test_area_weighted.py

does not turn the warning into an error?

Incidentally running that now is giving me the RuntimeWarning...

@wjbenfold
Copy link
Contributor

[ ] Regridding Issue

I think what's happening here is map_blocks throwing a 0d array through the regridder, see https://docs.dask.org/en/latest/generated/dask.array.Array.map_blocks.html near the top of the page.

The result of this would be an oddly suppressed failure (maybe Dask suppresses errors but not warnings?)

So I'm not sure I'm worried about this - though a little more investigation to confirm my assessment is correct would be reassuring.

@rcomer
Copy link
Member Author

rcomer commented Feb 16, 2022

Thanks @wjbenfold I don’t think I would ever have figured that one out! So would this mean we just need to pass the meta keyword when we call map_blocks?

@wjbenfold
Copy link
Contributor

Yeah, ideally we'd be handling it properly so that the regridding tells dask what type of array to expect out, but I think I remember it being more complicated than that (not that I can remember what made it more complicated).

The offending map_blocks call is

return da.map_blocks(elementwise_op, lazy_array, dtype=dtype)

@pp-mo
Copy link
Member

pp-mo commented Feb 17, 2022

I think what's happening here is map_blocks throwing a 0d array through the regridder

I for one hadn't realised it did that.
Given the 'meta' keyword you point out -- just like the one in 'from_array' -- I think you must be right though !

Just for background ...

We fixed this for our 'from_array' usage in #4135 and it went into v3.0.2
In that case IIRC they started doing it at dask v2.0.
It caused a serious real performance hit.

I suspect this usage in map_blocks may actually go back further,
but they only more recently added the 'meta' so you can avoid it.
So we should be able to do that ok.
-- and I expect that the warning problem came in with more recent versions of numpy

@pp-mo
Copy link
Member

pp-mo commented Feb 17, 2022

I think I remember it being more complicated than that (not that I can remember what made it more complicated)

Interesting.
I would expect that meta = np.darray, as seen in #4135 should be good enough.
Possibly we might need to take masked arrays into account though.

@rcomer
Copy link
Member Author

rcomer commented Feb 24, 2022

Point 1 in the OP is now covered by #4598 and point 2 is fixed in #4584. So I think we can close this, unless someone wants it open as a reminder to look at the RuntimeWarning?

@wjbenfold
Copy link
Contributor

I'd be happy for that to be raised separately I think - seems good to close to me

@rcomer rcomer closed this as completed Feb 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants