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

windows (mingw64) fixes #12

Merged
merged 10 commits into from
Feb 6, 2023
Merged

windows (mingw64) fixes #12

merged 10 commits into from
Feb 6, 2023

Conversation

pdumon
Copy link
Contributor

@pdumon pdumon commented Feb 4, 2023

  • if the output of sines generates an empty last line, a -1 char is returned somehow. read_input_data couldn't handle that
  • multiplying inf with the min_err (e.g. 0) gives a NaN on windows/mingw64 (-1#INDe000) that won't compare with errk

harminv-main.c Outdated Show resolved Hide resolved
harminv-main.c Outdated Show resolved Hide resolved
harminv-main.c Outdated Show resolved Hide resolved
harminv-main.c Outdated Show resolved Hide resolved
@stevengj
Copy link
Collaborator

stevengj commented Feb 5, 2023

LGTM with some minor stylistic tweaks.

@stevengj
Copy link
Collaborator

stevengj commented Feb 5, 2023

Actually, I'm confused. What was the problem, exactly?

-1 is EOF, the end-of-file marker. In that case, fscanf in read_input_data should return nread == EOF, and the loop should exit.

@pdumon
Copy link
Contributor Author

pdumon commented Feb 5, 2023

Actually, I'm confused. What was the problem, exactly?

-1 is EOF, the end-of-file marker. In that case, fscanf in read_input_data should return nread == EOF, and the loop should exit.

Ah, my bad. I couldn't find that -1 is actually EOF, plus I thought fscanf was actually reading a wrong value from the empty line output by sines. But you're right, the algorithm is fine as it is. The problem was downstream: just the NaN issue needs to be fixed. Just tested without the changes in eat_whitespace and read_input_data and works fine indeed.

harminv-main.c Outdated Show resolved Hide resolved
@pdumon
Copy link
Contributor Author

pdumon commented Feb 5, 2023

reverted the unnecessary 'fix'

@stevengj stevengj merged commit ed3cdda into NanoComp:master Feb 6, 2023
@stevengj
Copy link
Collaborator

stevengj commented Feb 6, 2023

Tagged as version 1.4.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants