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

Weighted A* #17274

Merged
merged 1 commit into from Nov 15, 2019
Merged

Weighted A* #17274

merged 1 commit into from Nov 15, 2019

Conversation

@RoosterDragon
Copy link
Member

RoosterDragon commented Oct 24, 2019

Replace Constants.CellCost and Constants.DiagonalCellCost with a dynamically calculated value based on the lowest cost terrain to traverse. Using a fixed value meant the pathfinder heuristics would be incorrect.

In the four default mods, the minimum cost is in fact 100, not 125. This increase would essentially allow the pathfinder to return suboptimal paths up to 25% longer in the worst case, but it would be quicker to do so.

This is exactly what Weighted A* does - overestimate the heuristic by some factor in order to speed up the search by checking fewer routes. This makes the heuristic inadmissible and it may now return suboptimal paths, but their worst case length is bounded by the weight. A weight of 125% will never produce paths more than 25% longer than the shortest, optimal, path.

We set the default weight to 25% to effectively maintain the existing, suboptimal, behaviour due to the choice of the old constant - in future it may prove a useful tuning knob for performance.


More reading if you are looking for more in depth on what this is and think terms like "ε-admissible" are cool:

Wikipedia on A* - specifically the section on Bounded relaxation

A* and Weighted A* Search


Below are some figures from moving a lone rifle between opposite corners (TL/BR) of the 128x128 RA map, Pool Party.

The path search runs two instances until they meet in the middle. The nodes considered in the search effectively determine how much work the search has performed. This gives an impression idea of the tradeoff of allowing suboptimal paths.

This isn't an extensive performance test obviously, but is just here to give a ballpark impression. I figured the best scenario was to maintain the existing weight (even if it was likely unintended) and now the tuning knob is easily accessible somebody could muck about with it in future if they wish and are willing to investigate the effects proper.

Weight Considered Total Considered %
100 5093+5091 10184 100%
105 4777+4793 9570 94%
110 4613+4554 9167 90%
115 4375+4353 8728 86%
120 4091+3999 8090 79%
125 3607+3586 7193 71%
130 2644+2670 5314 52%
135 2347+2311 4658 46%
140 1929+1914 3843 38%
150 1609+1679 3288 32%
175 907+1078 1985 19%
200 663+854 1517 15%
250 542+709 1251 12%
300 512+665 1177 12%
500 608+630 1238 12%
Replace Constants.CellCost and Constants.DiagonalCellCost with a dynamically calculated value based on the lowest cost terrain to traverse. Using a fixed value meant the pathfinder heuristics would be incorrect.

In the four default mods, the minimum cost is in fact 100, not 125. This increase would essentially allow the pathfinder to return suboptimal paths up to 25% longer in the worst case, but it would be quicker to do so.

This is exactly what Weighted A* does - overestimate the heuristic by some factor in order to speed up the search by checking fewer routes. This makes the heuristic inadmissible and it may now return suboptimal paths, but their worst case length is bounded by the weight. A weight of 125% will never produce paths more than 25% longer than the shortest, optimal, path.

We set the default weight to 25% to effectively maintain the existing, suboptimal, behaviour due to the choice of the old constant - in future it may prove a useful tuning knob for performance.
// The benefit of allowing the search to return suboptimal paths is faster computation time.
// The search can skip some areas of the search space, meaning it has less work to do.
// We allow paths up to 25% longer than the shortest, optimal path, to improve pathfinding time.
search.heuristicWeightPercentage = 125;

This comment has been minimized.

Copy link
@reaperrr

reaperrr Oct 25, 2019

Contributor

Would using .WithHeuristicWeight override this?

This comment has been minimized.

Copy link
@RoosterDragon

RoosterDragon Oct 25, 2019

Author Member

Yes.

Copy link
Contributor

reaperrr left a comment

Code looks good to me and didn't encounter any issues while testing

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 3, 2019

During my local tests, a value of 150% didn't produce notably longer paths, while notably reducing the performance hit, so that value looks like a good compromise for a follow-up.

I know @teinarss is working on a larger pathfinding refactor, but I think in the mean time we should take this + a follow-up that increases the default to ~150% to improve pathfinding performance for Next+1.

@reaperrr reaperrr mentioned this pull request Nov 3, 2019
16 of 16 tasks complete
@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 14, 2019

@teinarss @tovl any objections to merging this?

@@ -57,40 +57,33 @@ public static IPathSearch Search(World world, Locomotor locomotor, Actor self, B

public static IPathSearch FromPoint(World world, Locomotor locomotor, Actor self, CPos @from, CPos target, BlockedByActor check)

This comment has been minimized.

Copy link
@tovl

tovl Nov 14, 2019

Contributor

FromPoint seems to be a bit pointless now that it is basically just directing to FromPoints without any logic of it's own. Would there be any objection to replacing all references to FromPoint with FromPoints and removing it?

This comment has been minimized.

Copy link
@teinarss

teinarss Nov 14, 2019

Contributor

It harbors the important logic of creating the froms array! 😉

This comment has been minimized.

Copy link
@reaperrr

reaperrr Nov 15, 2019

Contributor

If we dropped this we'd have to create a single-entry array in every place that currently uses FromPoint, which is in half a dozen places spread over 5 files.
Might be worth considering, but not a blocker for this PR, in my opinion.

@reaperrr reaperrr merged commit 04912ea into OpenRA:bleed Nov 15, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@RoosterDragon RoosterDragon deleted the RoosterDragon:weightedAstar branch Nov 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.