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

[BUG] AmberRst7 parser fails if there is a blank line at the end of file #298

Closed
lohedges opened this issue Feb 14, 2025 · 3 comments · Fixed by #299
Closed

[BUG] AmberRst7 parser fails if there is a blank line at the end of file #298

lohedges opened this issue Feb 14, 2025 · 3 comments · Fixed by #299
Assignees
Labels
bug Something isn't working

Comments

@lohedges
Copy link
Contributor

The SireIO::AmberRst7 parser fails if there is a blank line at the end. The line can contain valid data, which it is trying to parse. We should check for an empty line and skip.

@lohedges lohedges added the bug Something isn't working label Feb 14, 2025
@lohedges lohedges self-assigned this Feb 14, 2025
@lohedges
Copy link
Contributor Author

Actually, the code is doing it's job. The issue is that this was a 2 atom molecule, so there would be two lines if there were coordinates and velocities. Since the second line is blank, it is failing to read the velocities. There's no valid way to know that the velocities should be missing, since it is expecting them to be present given the size of the file. If the molecule was larger and a blank line existed, then all would be okay since it wouldn't even attempt to parse the velocities since there aren't enough lines.

@chryswoods
Copy link
Contributor

This is one of the many reasons why I don't like AmberRst7. The NetCDF-based formats are much better.

@lohedges
Copy link
Contributor Author

Agreed. I think this was a rare edge case where someone happened to be testing conversion of a problem molecule to make sure that information was preserved. I doubt this would have tripped us up in practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants