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

fixing bugs. case 5 runs. hits infinite loop #19

Draft
wants to merge 28 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
f907b99
fixing bugs. case 5 runs. hits infinite loop
connoralittle Sep 3, 2024
fc5b6f5
resize is needed not reserve
connoralittle Sep 4, 2024
ffd6f0b
templated policies, tentatively working. next step is matched_step
connoralittle Sep 9, 2024
094bdfa
continued bug squashing
connoralittle Sep 25, 2024
56aabce
I think this is the root source of many bugs. This variable is used b…
connoralittle Sep 25, 2024
590631b
decoupled outcome to name. Now on order recieved.
connoralittle Sep 25, 2024
e68d795
visualize runs with an off by 1 error
connoralittle Nov 12, 2024
651cf52
visualize works again properly
connoralittle Nov 14, 2024
9aff1f4
pr2 search works!
connoralittle Nov 18, 2024
6d109b6
various fixes while searching for blowup
connoralittle Jan 2, 2025
1c00802
New versions of the sub-modules.
haz Jan 2, 2025
9b07ac0
Mirror the latest of the core.
haz Jan 2, 2025
5789b28
Hush now.
haz Jan 6, 2025
31c6d24
Put the logging checks in the right place.
haz Jan 6, 2025
d734917
Fixing some nasty bugs with heuristic logging.
haz Jan 6, 2025
ab172f9
Fixed a goal bug, but still stuck.
haz Jan 6, 2025
dc8febc
Merge pull request #1 from haz/main
connoralittle Jan 6, 2025
960f707
off by 1
connoralittle Jan 6, 2025
8a57d39
Fixing some memory issues caught by memcache.
haz Jan 8, 2025
b97bb3d
Merge pull request #2 from haz/main
connoralittle Jan 8, 2025
ee53234
Couple more memory optimizations.
haz Jan 8, 2025
060dd9a
heuristic search works now
connoralittle Jan 8, 2025
e5055a1
cleaned up previous push
connoralittle Jan 8, 2025
2696126
Merge pull request #3 from haz/main
connoralittle Jan 8, 2025
3bf5fe7
rearranged again
connoralittle Jan 8, 2025
9422522
Merge branch 'main' of https://github.com/connoralittle/pr2_new
connoralittle Jan 8, 2025
934dfa3
Reverting memleak fix.
haz Jan 8, 2025
03da6ee
Merge pull request #4 from haz/main
connoralittle Jan 8, 2025
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
22 changes: 11 additions & 11 deletions src/deadend.cc
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

all_fire_context still needs to be defined

Original file line number Diff line number Diff line change
Expand Up @@ -117,21 +117,21 @@ void update_deadends(vector< DeadendTuple* > &failed_states) {

////////////////////////////////////////////

// Check to see if we have any consistent "all-fire" operators
reg_items.clear();
PR2.general.regressable_cond_ops->generate_consistent_items(*failed_state,
reg_items,
PR2.deadend.regress_trigger_only);
// // Check to see if we have any consistent "all-fire" operators
// reg_items.clear();
// PR2.general.regressable_cond_ops->generate_consistent_items(*failed_state,
// reg_items,
// PR2.deadend.regress_trigger_only);

// For each operator, create a new deadend avoidance pair
for (auto item : reg_items) {
// // For each operator, create a new deadend avoidance pair
// for (auto item : reg_items) {

RegressableOperator *ro = (RegressableOperator*)item;
// RegressableOperator *ro = (RegressableOperator*)item;

fsaps.push_back(new FSAP(failed_state->regress(ro->op, ro->op.all_fire_context),
ro->op));
// fsaps.push_back(new FSAP(failed_state->regress(ro->op, ro->op.all_fire_context),
// ro->op));

}
// }

////////////////////////////////////////////

Expand Down
2 changes: 1 addition & 1 deletion src/fd_integration/partial_state.cc
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

allocate reintroduced

Copy link
Contributor

Choose a reason for hiding this comment

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

Was it removed by me?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was. It used to be allocating an array but the data structure is a vector now. I assume that is partly why

Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ PR2State::PR2State(std::vector<int> init_vals) {
}

PR2State::PR2State(const State &state) {
// _allocate();
_allocate(PR2.general.num_vars);
for (auto var : state)
vars[var.get_variable().get_id()] = var.get_value();
}
Expand Down
6 changes: 4 additions & 2 deletions src/fd_integration/partial_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ class StateInterface;
class PR2State : public StateInterface {
std::vector<int> vars; // values for vars
std::vector< std::pair<int,int> > * _varvals = NULL; // varval pairs for partial states
// void _allocate();
// void _deallocate();
void _allocate(int size) {
vars.reserve(size);
}
void _deallocate() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the memory be released?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I may be wrong, but I believe the C++ vector should automatically deallocate everything when it goes out of scope. Looking it up reserve doesn't seem to require a free method

// void _copy_buffer_from_state(const PR2State &state);

public:
Expand Down
2 changes: 1 addition & 1 deletion src/fd_integration/pr2_search_algorithm.cc
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Task changed to root task. Comparison between nodes asserts the tasks must be the same. Unsure which change would need to be made

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sitting on it, removing the assert seems to be the right choice

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you link to where the assert occurs? This in the FD core?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

task_proxy.h line 174. The assert ensures factproxies only compare when they have the same task. The taskproxy produces operators with facts from one task and the lazy search algorithm produces facts with another, making them incomperable.

Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ unique_ptr<SearchAlgorithm> PR2Search::get_search_engine() {
numeric_limits<double>::infinity(),
"PR2 Search",
utils::Verbosity::SILENT,
weak_task,
tasks::g_root_task,
new DeadendAwareSuccessorGenerator());

return unique_ptr<SearchAlgorithm>(engine);
Expand Down
47 changes: 3 additions & 44 deletions src/pr2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,9 @@ void PR2Wrapper::generate_nondet_operator_mappings() {

for (auto op : PR2.proxy->get_operators()) {
//If not in the mapping yet
PR2.general.conditional_mask.push_back(new vector<int>());
PR2.deadend.nondetop2fsaps.push_back(new vector< FSAP* >());

if (nondet_name_to_index.find(op.get_nondet_name()) == nondet_name_to_index.end()) {
nondet_name_to_index[op.get_nondet_name()] = current_nondet_index;
PR2.general.nondet_mapping.push_back(vector<int>());
Expand Down Expand Up @@ -306,50 +309,6 @@ void PR2Wrapper::generate_nondet_operator_mappings() {
}

PR2.proxy->set_nondet_index_map(*nondet_index_map);

//TODO
//Reinstantiate conditional mask and possibly nondetop2fspas
//They were previously global and not instantiated anywhere


// /* Build the data structures required for mapping between the
// * deterministic actions and their non-deterministic equivalents. */
// int cur_nondet = 0;
// for (auto op : PR2.proxy->get_operators()) {

// int nondet_index = -1;

// PR2.general.conditional_mask.push_back(new vector<int>());
// PR2.deadend.nondetop2fsaps.push_back(new vector< FSAP* >());

// if (PRP.general.nondet_index_mapping.find(op.get_nondet_name()) == PRP.general.nondet_index_mapping.end()) {

// nondet_index = cur_nondet;
// PRP.general.nondet_index_mapping[op.get_nondet_name()] = cur_nondet;

// PRP.general.nondet_mapping.push_back(new vector<GlobalOperator *>());
// PRP.general.conditional_mask.push_back(new vector<int>());

// PRP.deadend.nondetop2fsaps.push_back(new vector< FSAP* >());

// cur_nondet++;

// } else {
// nondet_index = PRP.general.nondet_index_mapping[op.get_nondet_name()];
// }

// op.nondet_index = nondet_index;
// PRP.general.nondet_mapping[nondet_index]->push_back(&op);
// op.nondet_outcome = PRP.general.nondet_mapping[nondet_index]->size() - 1;

// for (auto eff : op.get_all_effects()) {
// for (auto cond : eff.conditions) {
// vector<int> *var_list = PRP.general.conditional_mask[nondet_index];
// if (find(var_list->begin(), var_list->end(), cond.var) == var_list->end())
// PRP.general.conditional_mask[nondet_index]->push_back(cond.var);
// }
// }
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it worth keeping this unworking code around for when we re-create the mask?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I still have that code in a local repository need be. I removed it to clean up the code. Conditional mask is put in the same places but I don't know if it has been tested yet.

}

void PR2OperatorProxy::update_nondet_info() {
Expand Down
13 changes: 8 additions & 5 deletions src/simulator.cc
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Operators were being overwritten. Temporary fix is to assign them to a new pointer.

Original file line number Diff line number Diff line change
Expand Up @@ -193,19 +193,22 @@ bool Simulator::check_1safe() {
if (PR2.deadend.force_1safe_weak_plans)
safe_checks = engine->get_plan().size();

PR2OperatorsProxy ops = PR2.proxy->get_operators();
for (unsigned i = 0; i < safe_checks; i++) {
const PR2OperatorProxy op = PR2.proxy->get_operators()[engine->get_plan()[i]];
const PR2OperatorProxy op = ops[engine->get_plan()[i]];
PR2OperatorProxy *op_stable = new PR2OperatorProxy(op);
vector<NondetSuccessor *> successors;
new_s = generate_nondet_successors(old_s, &op, successors);
new_s = generate_nondet_successors(old_s, op_stable, successors);

for (auto succ : successors) {
if (is_deadend(*(succ->state))) {
PR2State * new_dead_state = new PR2State(*(succ->state));
int op_ind = PR2.general.nondet_mapping[op.nondet_index][succ->id];
const PR2OperatorProxy bad_op = PR2.proxy->get_operators()[op_ind];
int op_ind = PR2.general.nondet_mapping[op_stable->nondet_index][succ->id - 1];
const PR2OperatorProxy bad_op = ops[op_ind];
PR2OperatorProxy *bad_op_stable = new PR2OperatorProxy(bad_op);
if (PR2.deadend.generalize)
generalize_deadend(*new_dead_state);
new_deadends.push_back(new DeadendTuple(new_dead_state, new PR2State(*old_s), &bad_op));
new_deadends.push_back(new DeadendTuple(new_dead_state, new PR2State(*old_s), bad_op_stable));
}
}

Expand Down