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

Fix multiple terminal states bug from bmdp format #19

Closed
wants to merge 2 commits into from

Conversation

jmskov
Copy link
Collaborator

@jmskov jmskov commented Nov 16, 2023

Fixes a bug in reading the terminal states from the "bmdp" file format. Previously errored on Line 33 if more than 1 terminal state was present.

@Zinoex
Copy link
Owner

Zinoex commented Nov 17, 2023

Thanks for the fix, John! And I look forward to hearing what you think so far.

Regarding this bug, I might give a different perspective, and I think the disagreement comes from the fact that the format in bmdp-tool is not documented and thus just depends on the implementation. In bmdp-tool/synthesis.cpp, the terminal states are read in a for loop using file stream input fin >> termState;, which does automatically remove whitespace and newlines. Since delimited files generally have a fixed number of columns per line and a variable number of rows/lines, I would assume that this means it would be one terminal state per line for num_terminal lines rather than one line with all terminal states. This is why I implemented it as one terminal state per line, which I have confirmed works just fine with bmdp-tool. Let me know what you think - I am open to the other option. I just wanted to be sure that we agree on what the format should be.

@jmskov jmskov closed this Nov 17, 2023
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