Skip to content

Fix util get_joint_value to fall back to object_positions like the root copy - #4

Open
midasbal wants to merge 1 commit into
AxisAIOrg:mainfrom
midasbal:fix/util-get-joint-value-fallback
Open

Fix util get_joint_value to fall back to object_positions like the root copy#4
midasbal wants to merge 1 commit into
AxisAIOrg:mainfrom
midasbal:fix/util-get-joint-value-fallback

Conversation

@midasbal

Copy link
Copy Markdown

What

In util/validate_offline_trajectory.py, get_joint_value only looks up
robot_joints and returns None for anything else. The root
validate_offline_trajectory.py copy falls back to object_positions when a
joint is not in robot_joints. The fallback was never ported to the util/
copy.

Why it matters

check_joint_threshold treats a None value as a failed check (return False).
Object and hinge joints (cabinet levels, stove buttons, etc.) live in
object_positions, not robot_joints, and JointThresholdChecker is used
throughout tasks_config.json on exactly those joints (white_cabinet/bottom_level,
flat_stove/button, wooden_cabinet/top_level, and more). As a result the
util/ validator, which the README points to as the advanced version for
tasks-JSON use, silently marks genuinely-passing trajectories as failing. That
is a false negative in a data-validation tool, which quietly drops good data.

Fix

Port the object_positions fallback into util's get_joint_value so it
matches the root copy.

Verification

  • Behaviorally identical to the root copy's get_joint_value across a range of
    inputs (robot_joints hit, object_positions single-value list, multi-value
    list, scalar, empty list, non-numeric, missing).
  • End to end: for a state with white_cabinet/bottom_level at [0.42] in
    object_positions, check_joint_threshold (mode gt, threshold -0.01)
    goes from False (incorrect fail) to True.

Notes

Scoped to the one function. This is one instance of a broader duplication
between the root and util/ copies; happy to follow up with a deduplication or
a small regression test if you'd like either.

@faisalnugroho

Copy link
Copy Markdown

Verified the inconsistency locally: in the root validate_offline_trajectory.py, get_joint_value checks robot_joints first and falls back to object_positions, while util/validate_offline_trajectory.py returns None for anything outside robot_joints. Since check_joint_threshold fails on None, object-position-based checks (e.g. drawer/slider joints stored as object positions) silently fail when run through the util/ copy but pass through the root copy. The parity fix looks correct and low-risk. One suggestion while you're in there: consider a short comment or shared helper so the two copies can't drift apart again — happy to follow up with that as a separate PR if the maintainer prefers.

@midasbal

Copy link
Copy Markdown
Author

Thanks for verifying it independently and for the review. Agreed that the deeper
issue is the two copies drifting apart. I kept this PR scoped to just the parity
fix so it stays easy to review and merge. For drift-prevention, a shared helper
or a full dedup both seem reasonable, but that touches how the maintainers want
the root and util/ copies organized, so I'd rather leave the approach to them.
Happy to do the follow-up myself once they indicate a preference, or defer to you
if you'd prefer to take it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants