search

askforalfred / alfred

ALFRED - A Benchmark for Interpreting Grounded Instructions for Everyday Tasks
MIT License
360 stars 77 forks source link

Misaligned high-level and low-level actions #84

Closed aleSuglia closed 3 years ago

aleSuglia commented 3 years ago

Hi Mohit,

Just wanted to ask your opinion about an issue I've been having with the low_to_high_idx mapping. In particular, I've noticed that in the preprocessing script you have a specific check due to some misaligned trajectories (https://github.com/askforalfred/alfred/blob/f2de579cdc812f8adf3ce4999f4eb6fb45339c0e/data/preprocess.py#L198). This makes sure that the last two actions are somehow merged together. However, the corresponding low_to_high_idx mapping is not aligned anymore (basically it will have one extra element for the stop action).

If you want to reproduce this just try to debug the code with the following trajectory: pick_two_obj_and_place-AppleSliced-None-CounterTop-10/trial_T20190907_061009_396474. You will notice that the generated indexes have an extra element at the end (13) which errors because you go out of bounds when you try to index the high-level descriptions.

I think a reliable fix for this would be to add, immediately after merge_last_two_low_actions, the following lines:

 low_to_high_idx[-1] = action_low[-1][0]["high_idx"]
 low_to_high_idx[-2] = action_low[-2][0]["high_idx"]
 action_high[-1]["high_idx"] = action_high[-2]["high_idx"]
 action_high[-2]["high_idx"] = action_high[-3]["high_idx"]

Thanks in advance for your help, Alessandro

MohitShridhar commented 3 years ago

@aleSuglia good catch! thanks! If you could create a pull request for this, that would be great! If not, I'll look into it when I get some time.

MohitShridhar commented 3 years ago

Sorry for the delay. Done: https://github.com/askforalfred/alfred/commit/c205211ad3f0105af7378939ee20a689ce9eaf7c