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

Easy ~1.2x speedup for calculateRadius #45

Merged
merged 2 commits into from
Jun 12, 2024

Conversation

grinstead
Copy link
Contributor

This adds an optimization to avoid calculating where you can go from a position if you have already maxed out.

Small note that if there is a positive, minimum cost to making a move then the if statement could be expanded to

if (nextCost + minMove <= radius) {

There is no formal benchmarking system in the repo, so this just used a homebrewed one. The (fairly unscientific) results were speedups ranging from 1.15x to 1.3x

This is to aid in #9

This adds an optimization to avoid calculating where you can go from a position if you have already maxed out. I am not aware of the cost structure, but if there is a positive, minimum cost to making a step (as in, if each moved required at least 1), then you could add that minimum to the `nextCost` in the if statement.

There is no formal benchmarking system in the repo, so this just used a homebrewed one. The (fairly unscientific) results were speedups ranging from 1.15x to 1.3x
@grinstead
Copy link
Contributor Author

grinstead commented Jun 10, 2024

I'm not aware of the way the transition+tile costs work, but if it is possible to move from one tile to the next with a cost of 0, then this code will break and incorrectly trim away that movement.

At the moment the VisionConfiguration and MoveConfiguration objects both do something like map.someCostCalc(vector) || -1, which implicitly will disallow 0 (as that will get replaced with -1) . In that case this diff works fine, but it's possible that the intended code was map.someCostCalc(vector) ?? -1...

@cpojer
Copy link
Contributor

cpojer commented Jun 12, 2024

This is great, thank you for the contribution!

The cost for moving from one tile to the next is always greater than 0. -1 as a cost means it is inaccessible.

@cpojer cpojer merged commit 7ee3d91 into nkzw-tech:main Jun 12, 2024
2 checks passed
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