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

Backport some small changes from Scorpion. #207

Merged
merged 2 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions driver/portfolio_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,10 @@ def compute_run_time(timeout, configs, pos):
print("remaining time: {}".format(remaining_time))
relative_time = configs[pos][0]
remaining_relative_time = sum(config[0] for config in configs[pos:])
print("config {}: relative time {}, remaining {}".format(
pos, relative_time, remaining_relative_time))
return limits.round_time_limit(remaining_time * relative_time / remaining_relative_time)
absolute_time_limit = limits.round_time_limit(remaining_time * relative_time / remaining_relative_time)
print("config {}: relative time {}, remaining time {}, absolute time {}".format(
pos, relative_time, remaining_relative_time, absolute_time_limit))
return absolute_time_limit
jendrikseipp marked this conversation as resolved.
Show resolved Hide resolved


def run_sat_config(configs, pos, search_cost_type, heuristic_cost_type,
Expand Down Expand Up @@ -120,7 +121,7 @@ def run_sat(configs, executable, sas_file, plan_manager, final_config,
configs, pos, search_cost_type, heuristic_cost_type,
executable, sas_file, plan_manager, timeout, memory)
if exitcode is None:
return
continue
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If one search has time limit 0 we want to switch to the next config, not abort the whole portfolio.

Copy link
Contributor

Choose a reason for hiding this comment

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

The rationale behind the original code is that each portfolio should have a non-zero relative time limit, else it shouldn't be part of the portfolio. This means that the time limit hits 0 for all configs at the same time (when the overall time limit is exhausted). So I don't see the purpose of the change. Can you say a scenario where it makes sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If config5 has a low relative time limit in the portfolio file, it might get an absolute time limit of 0. But there could be a config6 with a high relative time limit that gets a non-zero absolute time limit.

Copy link
Contributor

Choose a reason for hiding this comment

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

How? I don't understand. Is there rounding involved that can round down to 0?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see now that indeed this is potentially rounded down before we get here. Then the change makes sense, sorry for the noise.


yield exitcode
if exitcode == returncodes.SEARCH_UNSOLVABLE:
Expand Down
2 changes: 1 addition & 1 deletion misc/style/run-all-style-checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def check_python_style():
"flake8",
# https://flake8.pycqa.org/en/latest/user/error-codes.html
"--extend-ignore", "E128,E129,E131,E261,E266,E301,E302,E305,E306,E402,E501,E741,F401",
"--exclude", "run-clang-tidy.py,txt2tags.py,.tox",
"--exclude", "run-clang-tidy.py,txt2tags.py,.tox,.venv",
"src/translate/", "driver/", "misc/",
"build.py", "build_configs.py", "fast-downward.py"], cwd=REPO)
except FileNotFoundError:
Expand Down
1 change: 0 additions & 1 deletion misc/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ commands =

[testenv:clang-tidy]
changedir = {toxinidir}/style/
deps = PyYAML==6.0.1
jendrikseipp marked this conversation as resolved.
Show resolved Hide resolved
commands =
python run-clang-tidy.py

Expand Down
3 changes: 3 additions & 0 deletions src/search/algorithms/int_packer.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "int_packer.h"

#include <algorithm>
#include <cassert>

using namespace std;
Expand Down Expand Up @@ -83,6 +84,8 @@ void IntPacker::set(Bin *buffer, int var, int value) const {

void IntPacker::pack_bins(const vector<int> &ranges) {
assert(var_infos.empty());
assert(all_of(ranges.begin(), ranges.end(),
[](int range) {return range > 1;}));
jendrikseipp marked this conversation as resolved.
Show resolved Hide resolved

int num_vars = ranges.size();
var_infos.resize(num_vars);
Expand Down
4 changes: 4 additions & 0 deletions src/search/algorithms/named_vector.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ class NamedVector {
elements.push_back(element);
}

void push_back(T &&element) {
elements.push_back(std::move(element));
}

T &operator[](int index) {
return elements[index];
}
Expand Down
4 changes: 4 additions & 0 deletions src/search/operator_id.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ class OperatorID {
return !(*this == other);
}

bool operator<(const OperatorID &other) const {
return index < other.index;
}

int hash() const {
return index;
}
Expand Down
8 changes: 5 additions & 3 deletions src/search/task_utils/task_properties.cc
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,11 @@ PerTaskInformation<int_packer::IntPacker> g_state_packers(
vector<int> variable_ranges;
variable_ranges.reserve(variables.size());
for (VariableProxy var : variables) {
variable_ranges.push_back(var.get_domain_size());
/* IntPacker expects all variables to have at least a domain size of
two. This is not the case for some domain-abstracted tasks. */
int domain_size = max(2, var.get_domain_size());
variable_ranges.push_back(domain_size);
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above.

}
return utils::make_unique_ptr<int_packer::IntPacker>(variable_ranges);
}
);
});
}
Loading