IvS-KULeuven / IvSPythonRepository

Python Repository of the Institute of Astronomy @ KU Leuven
15 stars 22 forks source link

Bug when using the function "iterative_prewhitening" #21

Open stefano-rgc opened 4 years ago

stefano-rgc commented 4 years ago

After I made a fresh installation of the IvSPythonRepository and the corresponding Conda environment, I get a Python TypeError when running the following code:

>>>from ivs.timeseries.freqanalyse import iterative_prewhitening
>>>from ivs.timeseries.freqanalyse import stopcrit_scargle_snr
>>>import numpy as np
>>>
>>>t = np.arange(100)
>>>s = np.sin(t)
>>>
>>>iterative_prewhitening(t, s)
Frequency precision not reached with stepsize 6.464646e-08 , breaking loop
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/lhome/stefano/Python/packages/ivs/timeseries/freqanalyse.py", line 379, in iterative_prewhitening
    prewhiteningorder_snr_window=prewhiteningorder_snr_window,**kwargs)
  File "/lhome/stefano/Python/packages/ivs/timeseries/freqanalyse.py", line 326, in find_frequency
    params = numpy_ext.recarr_join(params,errors)
  File "/lhome/stefano/Python/packages/ivs/aux/numpy_ext.py", line 374, in recarr_join
    arr1 = append_fields(arr1,field,arr2[field],asrecarray=False)
  File "/lhome/stefano/anaconda3/envs/ivs_repo_3.6/lib/python3.6/site-packages/numpy/lib/recfunctions.py", line 651, in append_fields
    base = merge_arrays(base, usemask=usemask, fill_value=fill_value)
  File "/lhome/stefano/anaconda3/envs/ivs_repo_3.6/lib/python3.6/site-packages/numpy/lib/recfunctions.py", line 382, in merge_arrays
    seqarrays = seqarrays.ravel()
  File "/lhome/stefano/anaconda3/envs/ivs_repo_3.6/lib/python3.6/site-packages/numpy/ma/core.py", line 4517, in ravel
    r = ndarray.ravel(self._data, order=order).view(type(self))
TypeError: descriptor 'ravel' requires a 'numpy.ndarray' object but received a 'numpy.void'

Timothy identified the problem in the function recarr_join within the file ivs/aux/numpy_ext.py.

In the version I am using, such function reads (note the commented line):

def recarr_join(arr1,arr2):
    """
    Join to record arrays column wise.
    """
    arr1 = arr1.copy()
    for field in arr2.dtype.names:
        # arr1 = pl.mlab.rec_append_fields(arr1,field,arr2[field])
        arr1 = append_fields(arr1,field,arr2[field],asrecarray=False)
    return arr1

While in the working version of Timothy, the same function reads:

def recarr_join(arr1,arr2):
    """
    Join to record arrays column wise.
    """
    arr1 = arr1.copy()
    for field in arr2.dtype.names:
        arr1 = pl.mlab.rec_append_fields(arr1,field,arr2[field])
    return arr1

I have now changed my local version of the IvS repository but I think the change should be done in the official version too.

karandis commented 4 years ago

Hi Stefano,

Thanks for bringing this up. I double-checked, and I had commented out that line because the function rec_append_fields() is depreciated and has been removed in matplotlib 3.1.

https://matplotlib.org/3.0.0/api/mlab_api.html#matplotlib.mlab.rec_append_fields

I will try to look for an alternative :)

Cheers, Karan

LucIJspeert commented 4 years ago

Hi Karan,

I have had trouble with this functionality as well: the problem is that recarrays have been moved from mpl to numpy, but it's only in numpy 1.15 and up (which is a higher version than the repository environment has).

The functionality can now be found under: np.lib.recfunctions.rec_append_fields() https://numpy.org/devdocs/user/basics.rec.html - (search for 'rec_append_fields')

As a side note, depending on the functionality you need, structured arrays might be a good alternative.

Cheers, Luc

karandis commented 4 years ago

Hi Luc,

Awesome! have you tested this function on your local repository? If it works in the way that it's intended to, then I'll update the version of numpy and implement it :)

Cheers, Karan

LucIJspeert commented 4 years ago

Yeah it works, I use that in my local version of the repository, along with al the updated packages. :) In fact, you can find the change here in my fork: https://github.com/LucIJspeert/IvSPythonRepository/commit/abec847ea13be5f94be8daa385a34d3b5dbab210

karandis commented 4 years ago

Hi everyone, I have updated the repository to reflect the changes. Could you please check if it works now @stefano-rgc?

Cheers, Karan