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

Incorrect application of rules? #6

Open
basilnsaeed opened this issue Oct 14, 2018 · 3 comments
Open

Incorrect application of rules? #6

basilnsaeed opened this issue Oct 14, 2018 · 3 comments

Comments

@basilnsaeed
Copy link

Hi,
I don't think the application of the rules on line 183+ is correct.
I believe these rules are supposed to be applied until the graph is no longer changing. However, you're considering each edge only once and then never checking it again.
Changing another edge later on might make it so that a rule is applicable to an earlier edge you checked, but in your case, the rule is not applied in such a case.

@keiichishima
Copy link
Owner

You may be right. Do you have a patch to fix the code?

@basilnsaeed
Copy link
Author

I believe a naive way to do it is have an outer (infinite) loop with an inner loop over the edges, where you apply the rules to each edge, and only break from the outer loop when you can loop over all edges without anything changing. I haven't thought of heuristics to try to make this any faster.

@keiichishima
Copy link
Owner

I tried to update the code to follow exactly same as the original paper says. Thank you for the report.

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

No branches or pull requests

2 participants