Skip to content

Commit

Permalink
I think this is the root source of many bugs. This variable is used b…
Browse files Browse the repository at this point in the history
…oth as an ordinal and as an index, and by setting it to represent both it seems to fix several bugs down the line. C++ never threw an index out of bounds exception, so it wa hard to deduce
  • Loading branch information
connoralittle committed Sep 25, 2024
1 parent 094bdfa commit 56aabce
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions src/pr2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -288,12 +288,13 @@ void PR2Wrapper::generate_nondet_operator_mappings() {
}
PR2.general.nondet_mapping[nondet_name_to_index[op.get_nondet_name()]].push_back(op.get_id());
// outcome id comes from action name after _DETDUP_. The "8" is length of "_DETDUP_"
//Some elements don't have detdup, they have an assignment of 1
//Some elements don't have detdup, they have an assignment of 0
// Changed so it corresponds with the index
if (op.get_name().find("_detdup_") == std::string::npos){
PR2.general.nondet_outcome_mapping[op.get_id()] = 1;
PR2.general.nondet_outcome_mapping[op.get_id()] = 0;
} else {
PR2.general.nondet_outcome_mapping[op.get_id()] =
stoi(op.get_name().substr(op.get_name().find("_detdup_") + 8).substr(0, 1));
stoi(op.get_name().substr(op.get_name().find("_detdup_") + 8).substr(0, 1)) - 1;
}
op.nondet_index = nondet_name_to_index[op.get_nondet_name()];
op.nondet_outcome = PR2.general.nondet_outcome_mapping[op.get_id()];
Expand Down

2 comments on commit 56aabce

@haz
Copy link
Contributor

@haz haz commented on 56aabce Sep 25, 2024

Choose a reason for hiding this comment

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

Seems perfectly situated to mess us up when some external process starts the counting at 2 or 0 or some other nonsensical thing. Can you treat it as a string/symbol instead? Eventually, the detdup suffix will be an arbitrary name that corresponds to the cross-product of labeled outcomes that makes it up. We'll need to be rid of these stoi conversions at that point anyways.

@connoralittle
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 will see what I can do!

Please sign in to comment.