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

nafflib problems #40

Closed
carmignani opened this issue Mar 13, 2018 · 4 comments · Fixed by #587
Closed

nafflib problems #40

carmignani opened this issue Mar 13, 2018 · 4 comments · Fixed by #587
Assignees
Labels

Comments

@carmignani
Copy link
Member

Hello,
I tried to use the calcnaff function that we have in the nafflib directory and I found some problems.

If I run the function to analyze the same signal two times I get the same frequencies, but the phases are every time different.

A second issue about the function is that it requires a complex signal, so it cannot be used with real bpm measurements. We could give a vector of zeros as imaginary part of the signal, but then I don't know if we can trust the results.

@yrtan
Copy link

yrtan commented May 14, 2018

Tried the default example from calcnaff and got the same phases each time. Also tried it on some BPM data. I couldn't repeat the problem for AT2.0. nafflib.c revision date 25/7/2017.

You can use it for just real signals and the phases be also be correct (taking only the positive frequencies). However for the amplitude you have to sum the positive and negative frequency amplitudes to get it right. Or you can simply multiply by a factor of 2 if the frequency is not zero or fs/2.

@lnadolski
Copy link
Contributor

Dear @carmignani,
I cannot reproduced your bug.

If you run the nafflib algorithm with a real signal, its works well for me.
This is a simple example :

t = linspace(1,2pi20,1000);

[frequency amplitude phase] = nafflib(sin(t),sin(t)0, 1, 5, 1)
*** NAFF results ***
NFS = 2
AMPL= 4.207309e-01+i
2.701499e-01 abs(AMPL)= 4.999955e-01 arg(AMPL)= 5.707992e-01 FREQ=-1.247885e-01
AMPL= 4.207355e-01+i*-2.701512e-01 abs(AMPL)= 5.000000e-01 arg(AMPL)=-5.707963e-01 FREQ= 1.247885e-01

The phase is stable and the amplitude of each harmonic is 0.5 as expected.

@lnadolski
Copy link
Contributor

Nevertheless, I suspect something wrong the in the c-library, since sometimes, the function returns NaNs for valid input signal.
If I rerun again the same command with the same input argument the results is then fine.
I suspect something not properly defined with the dynamic memory allocation in the code.

I need to run the code in debugger to understand. This is quite annoying and time consuming to debug.
Any advice is very welcome.

@lnadolski lnadolski added the bug label Jul 27, 2020
@carmignani
Copy link
Member Author

I get exactly the same output running the example you wrote, but if I run it many times sometimes I get nans.
I also suspect there are some problems in the dynamic memory allocation in the C code.

pcsch added a commit to pcsch/at that referenced this issue Mar 28, 2023
lfarv pushed a commit that referenced this issue Mar 29, 2023
* Fixes bug producing infrequent NaN from calcnaff

Fixes #40

* Update calcnaff.m

The second test for NaN was wrong Y was replaced by Yp

---------

Co-authored-by: Patrick Schreiber <patrick.schreiber@synchrotron-soleil.fr>
Co-authored-by: Laurent S. Nadolski <nadolski@synchrotron-soleil.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants