Refactor and cleanup core lift tasks

[StafaH]

Description

Consolidates the rigid and soft Franka lifting tasks into a single
isaaclab_tasks.core.lift package, eliminating the duplicated
lift_franka_soft task and its parallel mdp module. Soft and cloth variants
now live as configs alongside the rigid one and share a common MDP package.

Changes

Package consolidation (breaking)

• Moved isaaclab_tasks.core.lift_franka_soft under
  isaaclab_tasks.core.lift.config.franka_soft, next to the rigid franka
  config.
• Merged the deformable MDP terms (observations, rewards, terminations) into the
  shared isaaclab_tasks.core.lift.mdp package instead of a separate per-variant
  mdp. The old lift_franka_soft/mdp module is removed.

Environment IDs (breaking)

• Dropped the -v0 version suffix from all lift Gym environment IDs:
  • Isaac-Lift-Cube-Franka-v0 → Isaac-Lift-Cube-Franka
  • Isaac-Lift-Cube-Franka-Play-v0 → Isaac-Lift-Cube-Franka-Play
  • Isaac-Lift-Soft-Franka-v0 → Isaac-Lift-Soft-Franka
  • Isaac-Lift-Cloth-Franka-v0 → Isaac-Lift-Cloth-Franka

Fixes

• Renamed the Isaac-Lift-Cloth-Franka physics preset from the misspelled
  newton_mjwarp_vdb to newton_mjwarp_vbd, matching the soft-body task and the
  underlying VBD solver.
• The cloth RewardsCfg, which duplicated the soft task's rewards verbatim, now
  inherits them instead of redefining.

Misc

• Updated imports, docs, benchmarks, and tests to reference the new module paths
  and environment IDs.
• Minor docstring/comment cleanup in lift_env_cfg.py.

Migration

# Before
from isaaclab_tasks.core.lift_franka_soft.franka_soft_env_cfg import FrankaSoftEnvCfg
# After
from isaaclab_tasks.core.lift.config.franka_soft.franka_soft_env_cfg import FrankaSoftEnvCfg

# Deformable MDP terms now come from the shared package
from isaaclab_tasks.core.lift.mdp import deformable_lifted

Update any gym.make(...) / --task calls to use the unversioned environment
IDs listed above.

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with ./isaaclab.sh --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have updated the changelog and the corresponding version in the extension's config/extension.toml file
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[isaaclab-review-bot]

Code Review: Refactor and cleanup core lift tasks

Well-structured consolidation that eliminates meaningful code duplication. The architecture choices are sound — merging deformable MDP terms into the shared lift/mdp package and moving lift_franka_soft under lift/config/franka_soft follows the established pattern of lift/config/franka. Here's a detailed analysis:

---

✅ Design Assessment

Architecture: The refactor correctly unifies rigid and deformable lift tasks under one package while keeping variant-specific configurations isolated in their respective config directories. The shared mdp package now owns all lift-specific observation, reward, and termination functions — this is the right ownership boundary.

Positive changes:

• Termination functions (deformable_com_below_minimum, deformable_outside_table_bounds, ee_below_minimum) were correctly separated from rewards.py into terminations.py — they were misplaced in the old code
• coupled_mjwarp_vbd_solver_cfg() extracted as a factory function eliminates config duplication between soft and cloth variants
• The cloth RewardsCfg inheritance from the soft task's rewards is cleaner than the previous verbatim copy
• Fixing the newton_mjwarp_vdb → newton_mjwarp_vbd typo and removing it from environ_docs.py is a welcome correctness fix

Cross-module impact: Primarily self-contained. Breaking changes (env ID renames, import paths) are well-documented in the changelog entry. The migration path is clear.

---

🔍 Findings

🟡 franka_soft_env_cfg.py — Comment says "one-way coupled" but code is "two-way"

In the diff for PhysicsCfg, the old comment said:

# Newton physics: MJWarp rigid + VBD soft, one-way coupled

The new code correctly updates this to two-way coupled in franka_soft_env_cfg.py, but the franka_cloth_env_cfg.py comment still references "matches newton/examples/softbody/example_softbody_franka.py" which may be stale if that example has diverged. Minor, but worth verifying the comment accuracy.

🟡 franka_cloth_env_cfg.py — FrankaClothEnvCfg.__post_init__ does not call super().__post_init__()

The cloth config's __post_init__ duplicates several settings from the parent (decimation, episode_length_s, sim.dt, sim.render_interval, viewer settings) rather than calling super().__post_init__() and only overriding what differs. This is pre-existing behavior (not introduced by this PR), but since the file was substantially refactored, it's worth noting:

• The parent sets self.sim.gravity = (0.0, 0.0, 0.0) which the cloth task intentionally does NOT inherit (cloth needs gravity for draping). This is correct.
• However, if any new settings are added to FrankaSoftEnvCfg.__post_init__() in the future, the cloth config won't pick them up — a maintenance hazard.

Consider adding a brief comment in FrankaClothEnvCfg.__post_init__ noting that it intentionally does NOT call super (to preserve gravity).

🟡 lift_franka_soft.py state machine script — dead code path after rename

Line 296:

elif args_cli.task == "Isaac-Lift-Cloth-Franka":

The old code compared against "Isaac-Lift-Soft-Franka-Cloth-v0". The default task is "Isaac-Lift-Soft-Franka", so users would need to explicitly pass --task Isaac-Lift-Cloth-Franka to hit this branch. This is fine, but the script's --help text only mentions the default — consider documenting the cloth option in the argparse help string.

🔵 __init__.pyi — ObservationTermCfg import added to observations.py

The DeformableSampledPointsInRobotRootFrame.__init__ signature changed from cfg (untyped) to cfg: ObservationTermCfg. This is a good typing improvement. The TYPE_CHECKING guard is correctly used.

🔵 Import style: absolute vs relative

The change from from . import mdp to from isaaclab_tasks.core.lift import mdp (and similar for agents) is necessary for Isaac Lab's lazy_export() mechanism to work correctly when the module is imported from different relative positions after the directory restructure. This is correct.

---

🧪 Test Coverage

This is a refactoring PR (restructuring existing code, not adding new behavior). The relevant observations:

• Existing tests (test_action_state_recorder_term.py, test_rsl_rl_export_flow.py, test_environment_determinism.py) are correctly updated with new environment IDs
• Benchmark configs updated with new IDs
• The coupled_mjwarp_vbd_solver_cfg() factory function produces identical config to what was inline before — no behavioral change to test
• The termination function relocation is a pure code move with no logic changes

Verdict: No new tests needed for a code-move refactor. Existing test coverage (environment determinism, export flow, action/state recording) should catch regressions from the restructuring.

---

📋 Checklist Items

• ✅ Changelog entry (mhaiderbhai-merge-lift-tasks.major.rst) is thorough and well-written
• ✅ .skip changelog fragments for isaaclab and isaaclab_rl (test-only changes)
• ⚠️ PR checklist items are all unchecked — please tick off the applicable ones before merge

---

Verdict

Minor fixes needed. The refactor is clean and well-executed. The breaking changes are properly documented with migration paths. Ship after addressing the checklist items and optionally the comment suggestions above.

---

Update (25c3c02): Incremental review after rebase onto develop.

The 12 new commits include a merge from develop bringing in infrastructure changes (cloner refactoring, Newton 1.2.1, PPISP peer dependency changes, etc.) plus a minor doc line-highlight fix. These develop changes are outside the scope of this lift-tasks PR.

Previous findings status:

• 🔴 My inline comment was NOT addressed. Line 304 in lift_franka_soft.py still passes FrankaClothEnvCfg() instead of env_cfg to gym.make(), ignoring CLI arguments (--num_envs, --device) for the cloth variant. Please fix.
• 🟡 Other findings (comment accuracy, __post_init__ documentation, argparse help) remain optional suggestions.

No new issues introduced by the merge or doc fix.

[greptile-apps]

Greptile Summary

This PR consolidates the lift_franka_soft package into lift.config.franka_soft, merges the separate deformable MDP module into the shared lift.mdp package, renames Gym environment IDs (dropping the -v0 suffix), and fixes the misspelled newton_mjwarp_vdb preset name to newton_mjwarp_vbd. The FrankaClothEnvCfg.RewardsCfg, which was a verbatim copy of the soft task's rewards, now inherits from the parent instead.

• Package consolidation: lift_franka_soft/ is fully removed; its configs move to lift/config/franka_soft/ and its deformable MDP terms (rewards, observations, terminations functions) are merged into the shared lift/mdp/ package.
• Breaking ID rename: All four lift Gym env IDs drop the -v0 suffix (Isaac-Lift-Cube-Franka-v0 → Isaac-Lift-Cube-Franka, etc.); the changelog documents the migration.
• Solver deduplication: coupled_mjwarp_vbd_solver_cfg() is introduced in franka_soft_env_cfg.py so both the soft and cloth tasks share an identical solver configuration instead of duplicating the inline CoupledMJWarpVBDSolverCfg block.

Confidence Score: 4/5

Safe to merge after fixing the gym.make call in the state machine script; the core library changes are structurally clean.

The consolidation is well-executed — the deformable MDP functions are faithfully migrated, the shared solver helper eliminates real duplication, and the cloth reward inheritance removes verbatim copy-paste. The one functional regression is in the state machine script: after updating the task IDs the cloth branch still passes FrankaClothEnvCfg() (a fresh default) to gym.make instead of the already-parsed env_cfg, so --num_envs and --device CLI arguments are silently ignored for that path.

scripts/environments/state_machine/lift_franka_soft.py — cloth branch of main() passes a fresh config to gym.make instead of the one parsed from CLI args.

Important Files Changed

Filename	Overview
source/isaaclab_tasks/isaaclab_tasks/core/lift/config/franka_soft/franka_cloth_env_cfg.py	Renamed from lift_franka_soft; drops the duplicated RewardsCfg (now inherited), fixes vdb→vbd typo, shares solver via coupled_mjwarp_vbd_solver_cfg(). Clean refactor with no logic regressions.
source/isaaclab_tasks/isaaclab_tasks/core/lift/config/franka_soft/franka_soft_env_cfg.py	Renamed from lift_franka_soft; adds coupled_mjwarp_vbd_solver_cfg() helper; switched mdp import to absolute path. Behavior unchanged.
scripts/environments/state_machine/lift_franka_soft.py	Updates task IDs but preserves a pre-existing bug: the cloth branch parses env_cfg (with --num_envs/--device) but then passes a fresh FrankaClothEnvCfg() to gym.make, silently discarding CLI customizations.
source/isaaclab_tasks/isaaclab_tasks/core/lift/mdp/observations.py	Deformable COM and sampled-points observation terms migrated from lift_franka_soft/mdp; DeformableSampledPointsInRobotRootFrame now has proper ObservationTermCfg type annotation on cfg.
source/isaaclab_tasks/isaaclab_tasks/core/lift/mdp/rewards.py	Deformable reward functions migrated from lift_franka_soft/mdp; logic is faithful copy of the originals.
source/isaaclab_tasks/isaaclab_tasks/core/lift/mdp/terminations.py	Deformable termination functions migrated and properly separated from rewards; previously deformable_com_below_minimum/deformable_outside_table_bounds/ee_below_minimum lived in rewards.py in the old module.
source/isaaclab_tasks/isaaclab_tasks/core/lift/mdp/init.pyi	Expanded all and imports cover all migrated deformable symbols; organized into observations/rewards/terminations sections.
source/isaaclab_tasks/isaaclab_tasks/core/lift/config/franka/init.py	Gym IDs renamed (drop -v0); import style changed from relative to absolute for agents; disable_env_checker moved before kwargs (cosmetic).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    OLD_PKG["lift_franka_soft/\n(removed)"]
    OLD_MDP["lift_franka_soft/mdp/\nobservations.py\nrewards.py\n(deleted)"]
    OLD_PKG --> OLD_MDP

    NEW_LIFT["lift/"]
    NEW_CFG["lift/config/franka_soft/\nfranka_soft_env_cfg.py\nfranka_cloth_env_cfg.py"]
    NEW_MDP["lift/mdp/\nobservations.py  ← deformable COM\nrewards.py       ← deformable rewards\nterminations.py  ← deformable terminations"]
    NEW_LIFT --> NEW_CFG
    NEW_LIFT --> NEW_MDP

    OLD_PKG -.->|"renamed to"| NEW_CFG
    OLD_MDP -.->|"merged into"| NEW_MDP

    GYM_OLD["Gym IDs (old)\nIsaac-Lift-Cube-Franka-v0\nIsaac-Lift-Soft-Franka-v0\nIsaac-Lift-Cloth-Franka-v0"]
    GYM_NEW["Gym IDs (new)\nIsaac-Lift-Cube-Franka\nIsaac-Lift-Soft-Franka\nIsaac-Lift-Cloth-Franka"]
    GYM_OLD -.->|"drop -v0"| GYM_NEW

    PRESET_OLD["Physics preset (cloth)\nnewton_mjwarp_vdb ❌"]
    PRESET_NEW["Physics preset (cloth)\nnewton_mjwarp_vbd ✅"]
    PRESET_OLD -.->|"typo fixed"| PRESET_NEW

Loading

Comments Outside Diff (1)

1. scripts/environments/state_machine/lift_franka_soft.py, line 189-199 (link)

   env_cfg discarded for cloth task — CLI args silently ignored

   parse_env_cfg is called with the user-supplied --num_envs and --device, the result is stored in env_cfg, and env_cfg.viewer.eye is even customised — but then gym.make receives a brand-new FrankaClothEnvCfg() that ignores all of that. Running the script with --num_envs 4 --device cuda:1 will silently spawn 128 envs on the default device for the cloth task. The soft-body branch just above correctly passes cfg=env_cfg; the cloth branch should do the same.

_{Reviews (1): Last reviewed commit: "Refactor and cleanup core lift tasks" | Re-trigger Greptile}

[greptile-apps]

The cloth branch should use env_cfg (already populated with --num_envs, --device, and the viewer override) just like the soft-body branch above, rather than creating a fresh default FrankaClothEnvCfg().

Suggested change

	env = gym.make("Isaac-Lift-Cloth-Franka", cfg=FrankaClothEnvCfg(), render_mode=render_mode)
	env = gym.make("Isaac-Lift-Cloth-Franka", cfg=env_cfg, render_mode=render_mode)