Latest commits to irf_powerfft return incorrect "Specrec"

thomas-nilsson-irfu commented 4 years ago

Step 1: Latest code?

[x] This issue is present in the most recent code (i.e. after running git pull this is still a problem).

Step 2: Describe your environment

irfu-matlab branch (i.e. "master", "devel"): devel
Matlab version used (i.e. "Matlab R2017a"): Matlab R2020a
Operating system (ie "Windows 7", "Mac OS X 10.12.3", "Linux Ubuntu 16.04"): Linux Ubuntu 20.04
[x] A 64-bit installation of Matlab was used.

Step 3: Describe the problem

Expected behavior

3d data (in form of TSeries) should properly have its power spectra computed per component in irf_powerfft.m, now it is not.

Actual behavior

Returned output is single matrix.

Steps to reproduce:

load some 3d data into a TSeries
process it with latest devel version of irf_powerfft().
returned output now has a single cell entry in the "Specrec.p".

Relevant code:

irf;
MMS_CONST = mms_constants; % Some mission constants
dceObj = dataobj('/data/mms/mms1/edp/fast/ql/dce2d/2020/05/mms1_edp_fast_ql_dce2d_20200520000000_v1.9.3.cdf'); % load example file (on brain or spis)
dceTs = get_ts(dceObj, 'mms1_edp_dce_xyz_dsl'); % get a TSeries object of this 3d data
dslFastSpect = irf_powerfft(dceTs, 512, MMS_CONST.Samplerate.fast)

dslFastSpect = 

  struct with fields:

    f: [256×1 double]
    p: {[5340×256 double]}
    t: [5340×1 double]
% But it should have 3 cells in "p"...

I am assigning this to @ErikPGJ, since he was the one who changed in irf_powerfft last.

thomas-nilsson-irfu commented 4 years ago

Just to help highlight the difference, if changing back to the old irf_powerfft.m, and running the same code as above we get:

dslFastSpect = irf_powerfft(dceTs, 512, MMS_CONST.Samplerate.fast)

dslFastSpect = 

  struct with fields:

    f: [256×1 double]
    p: {[5340×256 double]  [5340×256 double]  [5340×256 double]}
    t: [5340×1 double]

which is more along with the expected result for the 3d data.

ErikPGJ commented 4 years ago

Your case concerns vector data (one 1D vector per timestamp), but TSeries seems to even support having 2D data (one 2D array per time stamp). I assume that irf_powerfft could in principle be generalized to handle that too, but from what I see in the old implementation, it only handled vector data: comp=number of components per timestamp was scalar ==> Could only describe size of vector data.

I have found at least one bug in the code now.

ErikPGJ commented 4 years ago

OK. I think I have fixed the bug and I have pushed it. irf_powerfft can however only handle scalar and 1D vector data now, as it originally did (I think), not tensor data (>=2D).

ErikPGJ commented 4 years ago

FYI, I thought I had fixed the bug and pushed it (0f3105c3).

However, I think there is ANOTHER BUG so wait a bit. ( intervalSec2 = intervalSec1 + nSamplesOut*samplFreqHz) Need to look at bit more closely, but I am going away next week so I may not have enough time now.

ErikPGJ commented 4 years ago

OK. Think I have pushed a bugfix for abovementioned bug too. It was an "old" bug; introduced (by me) in e0b725fad7762da848e77ec76cac6fc257b7198f (Aug 13).

thomas-nilsson-irfu commented 4 years ago

@ErikPGJ, Thank you!

Now it appears this issue is fixed (along with the one I sent you via e-mail back in may).. Enjoy your time off!

irfu / irfu-matlab