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 pathfinding by introducing Cython #123

Merged
merged 167 commits into from
Sep 30, 2024

Conversation

CyberiaResurrection
Copy link
Contributor

After trying to use Numba to just-in-time compile strategic bits of pathfinding (and failing miserably), I had much better luck with Cython.

For Imperial sectors, running time went from 142 minutes total (132 minutes pathfinding) to 40 minutes total (31 minutes pathfinding), for a reduction of 76.5% pathfinding time.

For Reft + 8 surrounding sectors, pathfinding went from 258 seconds to 50 seconds, for a reduction of 80.6% pathfinding time.

@CyberiaResurrection
Copy link
Contributor Author

@tjoneslo, before I mark this as ready for review, I'd appreciate you pulling this branch and following the instructions in the README to see if you can get the cython modules to build.

@LeamHall
Copy link
Member

I started on this to add the "newbie to the project" perspective and had to work through GitHub issues (gh command refused to work). Finally grabbed the source repo and then started on the readme.md.

While a 76-81% reduction in pathfinding time is nice, is it really worth it in the grand scheme of things? Per the readme, there's a requirements.txt file which means either a venv or a container. Since the requirements include packages with specific versions then upgrading/staying current will become an issue. Then there are the fonts to get (need an rpm based list, too), a file to move, and some inline compiling?

There are a lot of changes in this PR, and I understand the desire for efficiency, but I think Python is getting in your way here. I love Python, but when you need to do that much work for a gain that may not even give a 1:1 time ROI, are you using the right language?

If this is what you want to do, then go for it! But if efficiency is a primary driver, are you on the right path?

@CyberiaResurrection
Copy link
Contributor Author

@LeamHall , I grok where you're coming from - I'm ultimately looking to do multiple full-charted-space runs, so my breakpoint for time saving is probably a little different from yours.

As for language, Python is what @tjoneslo went with initially, and it's easier going with that than going for a full rewrite in another language.

@CyberiaResurrection
Copy link
Contributor Author

@tjoneslo , @LeamHall , I've added non-cython fallbacks so PyRoute will at least run without the built cython modules, albeit at a speed penalty.

@CyberiaResurrection CyberiaResurrection marked this pull request as ready for review September 17, 2024 01:01
@CyberiaResurrection CyberiaResurrection merged commit 0843426 into makhidkarun:master Sep 30, 2024
1 check passed
@CyberiaResurrection CyberiaResurrection deleted the CythonFilter branch September 30, 2024 05:02
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