Skip to content

Revert "MacOS Support: tests + devShell + mujoco"#825

Closed
leshy wants to merge 1 commit intodevfrom
revert-745-jeff_macos
Closed

Revert "MacOS Support: tests + devShell + mujoco"#825
leshy wants to merge 1 commit intodevfrom
revert-745-jeff_macos

Conversation

@leshy
Copy link
Contributor

@leshy leshy commented Dec 11, 2025

Reverts #745

@chatgpt-codex-connector
Copy link

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 11, 2025

Greptile Overview

Greptile Summary

This PR reverts the MacOS support changes from PR #745, removing all MacOS-specific functionality and returning the codebase to a Linux-only configuration.

Key Changes:

  • Nix configuration: Removed complex MacOS-specific Nix flake setup including xome and lib dependencies, reverting to simpler Linux-only devShell
  • Device detection: Removed Metal Performance Shaders (MPS) support across 10+ ML model files, reverting to CUDA-or-CPU fallback
  • Network configuration: Removed platform-specific (Darwin vs Linux) multicast and buffer configuration in lcmservice.py, reverting to Linux-only commands
  • MuJoCo integration: Removed mjpython executable requirement for MacOS and simplified subprocess handling
  • Shared memory: Reverted hash digest size from 8 to 12 bytes (MacOS name length limits no longer needed)
  • Documentation: Removed MacOS installation instructions and reverted CLI command examples

The revert appears complete with no remaining MacOS-specific code detected in the codebase.

Confidence Score: 4/5

  • This revert is safe to merge with minor considerations for the lcmservice.py changes
  • The revert is clean and comprehensive, removing all MacOS-specific code systematically. However, lcmservice.py removes platform detection entirely, which means the hardcoded buffer sizes (67108864 and 16777216) may differ from the previous MacOS-specific values (TARGET_RMEM_SIZE=2097152). This could affect Linux systems but likely improves them by using larger buffers.
  • Pay attention to dimos/protocol/service/lcmservice.py - verify the reverted buffer sizes work correctly on Linux systems

Important Files Changed

File Analysis

Filename Score Overview
flake.nix 5/5 Reverted complex MacOS-specific Nix configuration back to simpler Linux-only setup, removing xome and lib dependencies
dimos/protocol/service/lcmservice.py 4/5 Removed platform-specific multicast and buffer configuration, reverted to Linux-only implementation
dimos/protocol/service/test_lcmservice.py 5/5 Removed platform mocking from tests, simplified to Linux-only test cases
dimos/models/embedding/clip.py 5/5 Removed Metal Performance Shaders (MPS) device detection, reverted to CUDA-or-CPU fallback
dimos/robot/unitree_webrtc/mujoco_connection.py 5/5 Removed mjpython executable for MacOS, reverted to sys.executable and removed stderr handling
README.md 5/5 Removed MacOS installation section and reverted command examples to original format

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant Nix as Nix DevShell
    participant App as Application
    participant Models as ML Models
    participant LCM as LCM Service
    participant Mujoco as MuJoCo Subprocess

    Note over Dev,Mujoco: Before Revert (MacOS Support)
    Dev->>Nix: nix develop (with xome, lib)
    Nix-->>Dev: MacOS + Linux support
    Dev->>App: Start application
    App->>Models: Initialize (CUDA/MPS/CPU)
    Models-->>App: Device: MPS on MacOS
    App->>LCM: Configure network (platform.system())
    LCM-->>App: MacOS-specific routes (lo0, smaller buffers)
    App->>Mujoco: Start with mjpython
    Mujoco-->>App: Process started

    Note over Dev,Mujoco: After Revert (Linux Only)
    Dev->>Nix: nix develop (simplified)
    Nix-->>Dev: Linux-only support
    Dev->>App: Start application
    App->>Models: Initialize (CUDA/CPU only)
    Models-->>App: Device: CUDA or CPU
    App->>LCM: Configure network (no platform detection)
    LCM-->>App: Linux routes (lo, larger buffers)
    App->>Mujoco: Start with sys.executable
    Mujoco-->>App: Process started
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

17 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@leshy
Copy link
Contributor Author

leshy commented Dec 11, 2025

reverted just flake

@leshy leshy closed this Dec 11, 2025
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