Skip to content

Conversation

@pseudo-rnd-thoughts
Copy link

A repeat of #249 for the 2.0 branch

@greptile-apps
Copy link

greptile-apps bot commented Jan 7, 2026

Greptile Summary

This PR backports RLlib integration improvements to the 2.0 branch, including dependency updates and enhanced error handling.

  • Updated gymnasium from 0.29.1 to 1.1.1 and removed numpy version constraint for RLlib compatibility
  • Changed INI file path from hardcoded "pufferlib/config/ocean/drive.ini" to dynamically computed path using __file__, improving robustness for different installation scenarios
  • Enhanced C binding error handling by checking INI file existence before parsing and properly raising Python exceptions (FileNotFoundError for missing files, ValueError for parse failures)
  • Modified observation space upper bound from 1 to 2 in single_observation_space, which could affect trained models

Confidence Score: 3/5

  • This PR is moderately safe but requires verification of the observation space change
  • The dependency updates and error handling improvements are solid, but the observation space bound change from 1 to 2 is undocumented and could break compatibility with existing trained models or cause unexpected behavior if observations don't actually exceed 1.0
  • pufferlib/ocean/drive/drive.py requires attention due to the unexplained observation space change

Important Files Changed

Filename Overview
pufferlib/ocean/drive/binding.c Improved error handling by adding INI file existence check and better error messages with PyErr_SetString
pufferlib/ocean/drive/drive.py Changed INI file path from hardcoded string to computed path, and expanded observation space upper bound from 1 to 2
setup.py Updated dependency versions: removed numpy version constraint and upgraded gymnasium from 0.29.1 to 1.1.1

Sequence Diagram

sequenceDiagram
    participant User
    participant Drive as Drive Environment
    participant Binding as C Binding Layer
    participant FileSystem as File System
    
    User->>Drive: Initialize Drive(...)
    Note over Drive: Calculate INI file path:<br/>_PACKAGE_DIR/config/ocean/drive.ini
    Drive->>Binding: env_init(ini_file=_INI_FILE, ...)
    Binding->>FileSystem: fopen(ini_file, "r")
    alt File Not Found
        FileSystem-->>Binding: NULL
        Binding-->>Drive: PyErr_SetString(FileNotFoundError)
        Drive-->>User: Exception raised
    else File Exists
        FileSystem-->>Binding: FILE* handle
        Binding->>Binding: fclose(ini_check)
        Binding->>Binding: ini_parse(ini_file, handler, &conf)
        alt Parse Failure
            Binding-->>Drive: PyErr_SetString(ValueError)
            Drive-->>User: Exception raised
        else Parse Success
            Binding->>Drive: Initialize environment
            Drive->>Drive: Set observation space<br/>(low=-1, high=2)
            Drive-->>User: Environment ready
        end
    end
Loading

Copy link

@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.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

+ self.max_road_objects * self.road_features
)
self.single_observation_space = gymnasium.spaces.Box(low=-1, high=1, shape=(self.num_obs,), dtype=np.float32)
self.single_observation_space = gymnasium.spaces.Box(low=-1, high=2, shape=(self.num_obs,), dtype=np.float32)
Copy link

Choose a reason for hiding this comment

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

logic: observation space upper bound increased from 1 to 2 without explanation

Suggested change
self.single_observation_space = gymnasium.spaces.Box(low=-1, high=2, shape=(self.num_obs,), dtype=np.float32)
self.single_observation_space = gymnasium.spaces.Box(low=-1, high=1, shape=(self.num_obs,), dtype=np.float32)

Does this change reflect actual observation values that can exceed 1, or is this related to RLlib compatibility requirements?

Prompt To Fix With AI
This is a comment left during a code review.
Path: pufferlib/ocean/drive/drive.py
Line: 87:87

Comment:
**logic:** observation space upper bound increased from 1 to 2 without explanation

```suggestion
        self.single_observation_space = gymnasium.spaces.Box(low=-1, high=1, shape=(self.num_obs,), dtype=np.float32)
```

 Does this change reflect actual observation values that can exceed 1, or is this related to RLlib compatibility requirements?

How can I resolve this? If you propose a fix, please make it concise.

@pseudo-rnd-thoughts pseudo-rnd-thoughts changed the title Integration with RLlib Integration with RLlib (v2.0) Jan 7, 2026
@eugenevinitsky
Copy link

2.0 is frozen so we're not really taking substantive PRs to it anymore. So I'm going to close this, re-open if you disagree strongly

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