dioph / periodicity

Useful tools for periodicity analysis in time series data.
https://periodicity.readthedocs.io
MIT License
34 stars 5 forks source link

Weird multiplication with a TimeSeries behaviour #4

Open nmicauxmellisim opened 3 years ago

nmicauxmellisim commented 3 years ago

While trying to multiply a Periodogram with coefficients y, I get different results : Context :

gls = GLS(fmin=fmin, fmax=fmax)
pg = gls(x)
z= pg.val * y # y is an np array

I get results fine, but cannot do z.find_peaks() because z is a np array

So I switched to z= pg.val * Timeseries(time=periods, val=y) This works (return a result), BUT z here is not equal to z in the first test.

I would assume it to gave same result, because of :

    def __mul__(self, other):
        result = self.copy()
        if isinstance(other, Signal):
            if len(self) != len(other):
                raise ValueError("Cannot multiply two Signals with different lengths.")
            result.val = self.val * other.val
        else:
            result.val = self.val * other
        return result

Am I missing something here ?

Some more infos :
pg.val and y are of size (7215,)

nmicauxmellisim commented 3 years ago

I've more info on this : the bad result is the one from z= pg.val * Timeseries(time=periods, val=y), the other one is correct. y Out[6]: array([3.14300736e-52, 1.86213031e-11, 5.74780622e-06, ..., 1.33338891e-05, 1.33331659e-05, 1.33324430e-05])

pg.val Out[7]: array([0.01228954, 0.00296232, 0.00016234, ..., 0.00051971, 0.00030851, 0.00014104])

pg.val * period_likely Out[3]: array([3.86261247e-54, 5.51622778e-14, 9.33122670e-10, ..., 6.92970103e-09, 4.11347977e-09, 1.88042697e-09])

z= pg.val * Timeseries(time=periods, val=y) Out[2]: array([1.63849634e-07, 3.94971180e-08, 2.16467878e-09, ..., 2.98716890e-09, 5.74494862e-15, 4.43294285e-56])

dioph commented 2 years ago

Hi @nmicauxmellisim! I'm terribly sorry for the delay on this. Thank you for bringing this to my attention.

I've been working on a more general framework for TimeSeries and Periodogram objects by wrapping a xarray.DataArray object, so that will be coming soon, together (unfortunately) with some changes to the API. This will probably also be the first proper periodicity release out of beta, so keep an eye out for that.

I'll leave this issue open until then, so that we can come back and check if it is solved with the new changes.

dioph commented 2 years ago

Hi again, it's been a bit. I've just released v1.0b4, so you can check if any discrepancies persist in your example (unfortunately I can't reproduce your issue without knowledge of x, y etc., but if you could provide a minimal reproducible example it would be helpful!).

Also, as I mentioned on #3, I assume what you want to do is actually z = pg * y, where z will have the same type as pg.