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

Toposlib - creating an ndarray from ragged nested sequences is deprecated #536

Closed
JeremyTurpin opened this issue Jan 25, 2021 · 5 comments
Closed

Comments

@JeremyTurpin
Copy link

JeremyTurpin commented Jan 25, 2021

I revisited some code I had written last year with a fresh python environment (clean Python 3.7 installation, installed all dependencies into a virtualenv), and started getting a very large number of warnings when creating Topos objects.

'env/lib/python3.7/site-packages/skyfield/toposlib.py:21: VisibleDeprecationWarning: Creating an ndarray from ragged nested sequences (which is a list-or-tuple of lists-or-tuples-or ndarrays with different lengths or shapes) is deprecated. If you meant to do this, you must specify 'dtype=object' when creating the ndarray'

The code still functioned, but there were so many warnings that it was hard to see the results in the notebook...I looked at the source, and followed the recommendation in the warning by changing:

self._velocity_au_per_d = ANGVEL * DAY_S * array((-y, x, 0.0))
to
self._velocity_au_per_d = ANGVEL * DAY_S * array((-y, x, 0.0),dtype=object)

in line 19 of toposlib.py.

This got rid of the warning, but it turned out this led to other problems.

When running:

ts = load.timescale(builtin=True);
t = ts.utc(0,0,0,0,45+np.arange(0,60,1),0)
Topos('0N','0E').at(t)

I received the following exception:

TypeError                                 Traceback (most recent call last)
<ipython-input-4-67832433ab80> in <module>
      1 ts = load.timescale(builtin=True);
      2 t = ts.utc(0,0,0,0,45+np.arange(0,60,1),0)
----> 3 Topos(0.,0.).at(t)

env/lib/python3.7/site-packages/skyfield/vectorlib.py in at(self, t)
     90                              ' instance as its argument, instead of the'
     91                              ' value {0!r}'.format(t))
---> 92         p, v, gcrs_position, message = self._at(t)
     93         center = self.center
     94         position = build_position(p, v, t, center, self.target)

env/lib/python3.7/site-packages/skyfield/toposlib.py in _at(self, t)
     39         RT = _T(itrs.rotation_at(t))
     40         r = mxv(RT, r)
---> 41         v = mxv(RT, v)
     42         return r, v, r, None
     43 

env/lib/python3.7/site-packages/skyfield/functions.py in mxv(M, v)
     26 def mxv(M, v):
     27     """Matrix times vector: multiply an NxN matrix by a vector."""
---> 28     return einsum('ij...,j...->i...', M, v)
     29 
     30 def mxm(M1, M2):

<__array_function__ internals> in einsum(*args, **kwargs)

env/lib/python3.7/site-packages/numpy/core/einsumfunc.py in einsum(out, optimize, *operands, **kwargs)
   1348         if specified_out:
   1349             kwargs['out'] = out
-> 1350         return c_einsum(*operands, **kwargs)
   1351 
   1352     # Check the kwargs to avoid a more cryptic error later, without having to

TypeError: invalid data type for einsum

Some looking around indicated that the exception is due to the dtype=object modification, and I found an open numpy issue that seems to relate to this issue, as well: numpy/numpy#17837

I have since done the easy thing to get my own code to work again, at the risk of numpy (presumably) changing further and breaking things more in the future, by editing the toposlib.py file again to simply ignore the warnings from that line.

#self._velocity_au_per_d = ANGVEL * DAY_S * array((-y, x, 0.0),dtype=object) 
#self._velocity_au_per_d = ANGVEL * DAY_S * array((-y, x, 0.0))                                                         
with warnings.catch_warnings():                                                                                             
    warnings.simplefilter("ignore")                                                                                         
    self._velocity_au_per_d = ANGVEL * DAY_S * array((-y, x, 0.0)) 

This is a fragile 'fix', but I wanted to check if there was a more robust correction.

I'm using pip versions of numpy and skyfield on python 3.7.6; relevant package versions listed below.

astropy 4.2
Cartopy 0.18.0
matplotlib 3.3.3
numpy 1.19.5
scipy 1.6.0
sgp4 2.15
skyfield 1.35

Thanks for creating and supporting this great library!

@JoshPaterson
Copy link
Contributor

This seems related to #506, I'm not sure if the discussion there is helpful to you or not.

@brandon-rhodes
Copy link
Member

@JeremyTurpin — An experimental fix for that was made a few weeks ago, and is scheduled to appear in the next version. Could you try installing the most recent development version, and checking whether it fixes the problem?

pip install -U https://github.com/skyfielders/python-skyfield/archive/master.zip

Thanks!

As you'll see from the code, the solution is to make all three (x, y, z) components have the same length, instead of trying to paste together a long x and y array with a single number 0.0 for z.

@JeremyTurpin
Copy link
Author

@brandon-rhodes Thanks for the tip - installed the latest version from git as you mentioned, and the warning goes away. Thanks for the input!

@brandon-rhodes
Copy link
Member

Great, I'm glad it worked! I hadn't known the bug was so obvious from a notebook, which could affect lots of users. I'll see about getting a new version out before adding any new features, so it can come out soon. Thanks for the report!

@brandon-rhodes
Copy link
Member

I've just released 1.36. Let me know if you catch any regressions in the future!

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