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

Better heuristic function that improves runtime on difficult levels #5

Merged
merged 19 commits into from
Mar 29, 2024

Conversation

taufeeque9
Copy link
Collaborator

@taufeeque9 taufeeque9 commented Mar 11, 2024

  • Check deadlock condition for when the box is stuck in a corner that's not a goal
  • Add a script astar_log_level.cc to solve a particular level in a file. This should be used when logging levels across a small number of files.
  • astar_log.cc should still be used to log levels across a large number of files.

@taufeeque9 taufeeque9 requested a review from rhaps0dy March 11, 2024 22:50
@taufeeque9 taufeeque9 requested a review from rhaps0dy March 12, 2024 09:19
Copy link
Collaborator

@rhaps0dy rhaps0dy left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! I think this code still has a bug, though fewer.

What was wrong with classes in the sokoban_py_envpool_test.py?

@taufeeque9 taufeeque9 requested a review from rhaps0dy March 25, 2024 22:37
@taufeeque9 taufeeque9 changed the title Better heuristic function that improves runtime on difficult levels by 40x Better heuristic function that improves runtime on difficult levels Mar 25, 2024
Copy link
Collaborator

@rhaps0dy rhaps0dy left a comment

Choose a reason for hiding this comment

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

Looks good, some comments:

  • should both scripts be included?
  • check the formatting for the incorrect-solution branch

@@ -33,11 +33,14 @@ void RunAStar(const std::string& level_file_name,
std::ifstream log_file_in(log_file_name);
// check if the file is empty
if (log_file_in.peek() == std::ifstream::traits_type::eof()) {
log_file_out << "Level, Actions, Steps, SearchSteps" << std::endl;
log_file_out << "Level,Actions,Steps,SearchSteps" << std::endl;
} else { // skip levels that have already been run
std::string line;
std::getline(log_file_in, line); // skip header
while (std::getline(log_file_in, line)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be the case that line number = level number, but are we sure? Especially if sevearal concurrent workers are run. Maybe you should assert that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this file is only used to sequentially get the logs for the levels in a single file. So there shouldn't be any concurrent workers working on the same file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds like this file has been replaced by astar_log_level.cc, is that true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not really. astar_log_level.cc is to get the logs of a single level in a file, whereas astar_log.cc is to get the logs of all the levels in a file.

I think both are useful because if there are many different levels in many different files, then running astar_log.cc would be better. However, if we want to parallelize levels in a small number of files, then parallelizing across the levels with astar_log_level.cc should be better.

@taufeeque9 taufeeque9 merged commit 7562370 into sokoban Mar 29, 2024
2 checks passed
@taufeeque9 taufeeque9 deleted the tf/astar-better-heuristic branch March 29, 2024 05:59
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