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

Speed up StateReachability pass for large state machines #1370

Merged
merged 3 commits into from
Sep 21, 2023

Conversation

tbennun
Copy link
Collaborator

@tbennun tbennun commented Sep 20, 2023

The current algorithm in networkx is suboptimal. With this change, I noticed a 55x speedup in computing state reachability on realistic programs.

@tbennun tbennun requested a review from acalotoiu September 20, 2023 06:18
@tbennun
Copy link
Collaborator Author

tbennun commented Sep 20, 2023

@acalotoiu @mcopik tests fail because of an unrelated issue (has to do with fparser)

@mcopik
Copy link
Contributor

mcopik commented Sep 20, 2023

@tbennun Indeed, it looks like an incorrect dependency version. It's really strange because it should have popped up earlier. I will look into this.

@mcopik
Copy link
Contributor

mcopik commented Sep 20, 2023

@tbennun @acalotoiu It seems the module structure changed in fparser between 0.1.1 and 0.1.2. In the latter, they introduced a sub-module for Fortran2008, which should work with this import structure. However, our requirements specify exactly the version 0.1.2 - I'm not sure why an older version would be installed.

@mcopik
Copy link
Contributor

mcopik commented Sep 20, 2023

@acalotoiu @tbennun Mystery solved - they released a new version two days ago, drastically changing the package structure. While requirements.txt specifies an exact match on 0.1.2, our setup.py has a different dependency: fparser >= 0.1.2.

I think it's better to move forward with the newer version - I will propose a PR to resolve this issue.

@mcopik mcopik mentioned this pull request Sep 20, 2023
Copy link
Contributor

@acalotoiu acalotoiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you! It is a great performance improvement :).

@acalotoiu acalotoiu merged commit a4fa6ed into master Sep 21, 2023
9 checks passed
@tbennun tbennun deleted the faster-reachability branch September 21, 2023 14:00
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.

3 participants