Refactor LatticeMaze.find_shortest_path and fix minor bug #176
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rusheb
commented
Apr 13, 2023
Comment on lines
-149
to
-158
| g_score: dict[ | ||
| CoordTup, float | ||
| ] = dict() # cost of cheapest path to node from start currently known | ||
| f_score: dict[CoordTup, float] = { | ||
| c_start: 0.0 | ||
| } # estimated total cost of path thru a node: f_score[c] := g_score[c] + heuristic(c, c_end) | ||
|
|
||
| # init | ||
| g_score[c_start] = 0.0 | ||
| g_score[c_start] = self.heuristic(c_start, c_end) |
There was a problem hiding this comment.
Here is the part in the old version where I have changed behaviour (see PR description for details)
Comment on lines
+149
to
+152
| # cheapest currently known path to node from start | ||
| g_score: dict[CoordTup, float] = {c_start: 0.0} | ||
| # estimated total cost of path through a node: f_score[c] = g_score[c] + heuristic(c, c_end) | ||
| f_score: dict[CoordTup, float] = {c_start: self.heuristic(c_start, c_end)} |
There was a problem hiding this comment.
Here is the part in the new version where I have changed behaviour (see PR description for details)
rusheb
changed the title
mivanit
approved these changes
Apr 20, 2023
|
I feel like this might have some merge conflicts with #162 so I will wait to merge that until this is merged |
actually, it looks like there are no conflicts! |
mivanit
added
the
move-to-dataset-lib
|
@mivanit, should we close this? |
I don't think this actually ended up ever getting merged? I'll check in the maze-dataset repo tommorow and apply the fix there if needed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some refactoring I did to help me understand
find_shortest_pathwhilst reading through it.Mostly renaming. Also updated a few type hints.
There is one change in behaviour which I think fixes a minor bug.
Previously, we were initializing
g_score[c_start]twice, first to zero (correct) and then to the manhattan distance (incorrect).I think I have fixed this by initializing g_score and f_score for c_start.
Sorry for mixing bug fixes and refactoring together! I will highlight the behaviour changes in a comment.