-
Notifications
You must be signed in to change notification settings - Fork 29
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
Fixes bug producing infrequent NaN from calcnaff #587
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Any comment @lnadolski ?
%It seems there is a bug in nafflib, something returns nan even for valid data | ||
niter = 0; | ||
while any(isnan(frequency)) && (niter < nitermax) | ||
pause(2); | ||
fprintf('Warning Nan returned by NAFF (x%d)\n', niter); | ||
niter = niter +1; | ||
[frequency,amplitude,phase] = nafflib(Y, Yp, WindowType,nfreq,1); % add debugging | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So apparently, up to now, if the output is wrong, we try again until it gets right !
This looks crazy, your fix is really welcome, @pcsch !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second test for NaN was wrong Y was replaced by Yp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart 3 small modifications in calcnaff, this looks fine for me.
%It seems there is a bug in nafflib, something returns nan even for valid data | ||
niter = 0; | ||
while any(isnan(frequency)) && (niter < nitermax) | ||
pause(2); | ||
fprintf('Warning Nan returned by NAFF (x%d)\n', niter); | ||
niter = niter +1; | ||
[frequency,amplitude,phase] = nafflib(Y, Yp, WindowType,nfreq,1); % add debugging | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK again ! Thanks @pcsch and @lnadolski
Dear all,
This pull request concerns MATLAB.
As noted in #40 calcnaff sometimes produces NaN values even though the input is valid and a second run does result in valid frequencies.
The problem is that the interface between MATLAB and the NAFF algorithm did no write to the last element of the
complex input to NAFF.
This pull request fixes that.
In my tests this also fixed the fluctuating results from run to run (e.g. phase) seen in #40 as well.
Fixes #40