Fix place_cups success condition to accept any N distinct cups - #299
Open
llm-learner wants to merge 2 commits into
Open
llm-learner wants to merge 2 commits into
llm-learner wants to merge 2 commits into
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The instruction says "place 1 cup on the cup holder", without specifying which cup. However,
_on_peg_conditions[:index + 1]requiresmug0for variation 0 andmug0+mug1for variation 1.In
place_cups/variation0/episodes/episode2(all_variations/episodes/episode13),mug2is placed on the holder and released. The sensor detects it, the gripper is empty, but the original success condition remains false for 200 physics steps (~10 seconds).The video uses the PerAct fork and CoppeliaSim 4.1.0, with the original success condition. The same predicate is present in upstream
02720bba; a full-scene integration test against current upstream has not been run.Fix
Check any N distinct cups using the existing
ConditionSetandOrConditions. Keep the current sensors andNothingGraspedrequirement. Multiple sensors detecting one cup still count as one cup. Variation 2 is unchanged.Test
The regression test covers all 3 variations, 512 cup/sensor detection matrices, and both gripper states: 3,072 checks. The original predicate fails 161 checks; the corrected predicate passes all checks. These are mocked-sensor unit tests.
中文说明
指令只要求杯子数量,没有指定编号,但原判据要求固定的前 N 个杯子。视频中放好的是 mug2:传感器检测到它、夹爪已松开,但原环境仍判失败。视频来自 PerAct 环境;上游源码存在相同判据,尚未进行当前上游完整场景的集成测试。
修复改为检查任意 N 个不同杯子,保留原传感器和夹爪为空条件,不重复计数同一杯子。回归测试覆盖3,072种状态,原判据161项失败,修正后全部通过。该修改会影响 variation 0/1 的成功率统计,评测时应注明所用版本。
place_cups_test20_original_bug_zh_en.mp4
PerAct counterpart: MohitShridhar#3