-
Notifications
You must be signed in to change notification settings - Fork 88
Add Nle env to OpenEnv #161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
✅ Deployment succeeded for
Nice work! Wait for a code review and we're ready to go. You can iterate locally or validate fixes by running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a NetHack Learning Environment (NLE) integration to OpenEnv, providing HTTP-based access to the challenging roguelike game NetHack 3.6.6 for reinforcement learning research.
Key changes:
- Implements NLE environment wrapper with full observation space support (14+ observation types)
- Provides HTTP server/client architecture for remote environment access
- Includes Docker containerization with NLE compilation from source
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/envs/nle_env/server/nle_environment.py | Core environment implementation wrapping NLE's gym interface with OpenEnv's Environment ABC |
| src/envs/nle_env/server/app.py | FastAPI application setup with environment variable configuration |
| src/envs/nle_env/server/init.py | Server module initialization |
| src/envs/nle_env/server/Dockerfile | Docker build configuration with NLE compilation and dependencies |
| src/envs/nle_env/models.py | Data models for actions, observations, and state |
| src/envs/nle_env/client.py | HTTP client for interacting with NLE server |
| src/envs/nle_env/init.py | Package initialization and exports |
| src/envs/nle_env/README.md | Comprehensive documentation for the NLE environment |
| examples/nle_random_agent.py | Example script demonstrating random agent gameplay |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @dataclass | ||
| class NLEAction(Action): |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The base class Action uses @dataclass(kw_only=True), so child classes should also explicitly use @dataclass(kw_only=True) for consistency with other environments in the codebase (e.g., echo_env, browsergym_env, finrl_env). Change to @dataclass(kw_only=True).
| action_id: int # Index into nethack.USEFUL_ACTIONS (0-112) | ||
|
|
||
|
|
||
| @dataclass |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The base class Observation uses @dataclass(kw_only=True), so child classes should also explicitly use @dataclass(kw_only=True) for consistency with other environments in the codebase (e.g., echo_env, browsergym_env, finrl_env). Change to @dataclass(kw_only=True).
| @dataclass | |
| @dataclass(kw_only=True) |
|
|
||
| ```bash | ||
| # Build from repository root (not from server directory) | ||
| cd /Users/sanyambhutani/GH/OpenEnv |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hardcoded absolute path should be replaced with a generic placeholder path. Consider using a relative path or a generic path like cd /path/to/OpenEnv or just refer to the 'repository root'.
| cd /Users/sanyambhutani/GH/OpenEnv | |
| cd /path/to/OpenEnv # Replace with the path to your repository root |
| from __future__ import annotations | ||
|
|
||
| from dataclasses import dataclass, field | ||
| from typing import Any, Dict, List, Optional |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Any' is not used.
Import of 'Dict' is not used.
| from typing import Any, Dict, List, Optional | |
| from typing import List, Optional |
Darktex
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent implementation with strong documentation. 3 critical issues must be fixed before merge.
See inline comments for specific fixes needed.
| - action_id=50: Search (s) | ||
| """ | ||
|
|
||
| action_id: int # Index into nethack.USEFUL_ACTIONS (0-112) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 CRITICAL: Missing Action Validation
No validation that action_id is within valid range (0-112). Invalid actions could cause runtime errors.
Suggested Fix:
@dataclass
class NLEAction(Action):
action_id: int # Index into nethack.USEFUL_ACTIONS (0-112)
def __post_init__(self):
if not 0 <= self.action_id <= 112:
raise ValueError(f"action_id must be in range 0-112, got {self.action_id}")|
|
||
| ```bash | ||
| # Build from repository root (not from server directory) | ||
| cd /Users/sanyambhutani/GH/OpenEnv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 CRITICAL: Hardcoded User Path
This is a user-specific absolute path that won't work for others.
Fix:
# Build from repository root
cd /path/to/OpenEnv
# or
cd $(git rev-parse --show-toplevel)| over HTTP. | ||
| """ | ||
|
|
||
| from typing import Dict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 CRITICAL: Incomplete Type Annotations
Dict is used without full type specification throughout this file (lines 14, 57, 71, 129).
Fix:
from typing import Any, DictThen update all method signatures:
_step_payload() -> Dict[str, Any]_parse_result(payload: Dict[str, Any]) -> ..._parse_state(payload: Dict[str, Any]) -> ...
| """ | ||
| super().__init__(transform=transform) | ||
|
|
||
| if NLE is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 IMPORTANT: Improve Error Message
Error message should clarify Docker vs local development context.
Suggested:
if NLE is None:
raise ImportError(
"NLE is not installed. This should not happen in Docker deployments.\n"
"For local development, install with: pip install nle\n"
"For Docker builds, verify the build completed successfully."
)| # This source code is licensed under the BSD-style license found in the | ||
| # LICENSE file in the root directory of this source tree. | ||
|
|
||
| """Server module for NLE environment.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 IMPORTANT: Missing __all__ Export
For consistency with the main package, consider adding:
from .nle_environment import NLEEnvironment
__all__ = ["NLEEnvironment"]| """ | ||
| Convert NLE gym observation to NLEObservation. | ||
|
|
||
| With beefy compute, we just convert numpy arrays to lists. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟢 MINOR: Informal Comment
"beefy compute" is a bit informal for production code. Consider:
# Convert numpy arrays to nested lists for JSON serialization.
# This approach prioritizes simplicity over optimization, which is
# acceptable given the compute resources available.| if __name__ == "__main__": | ||
| import uvicorn | ||
|
|
||
| # NLE must run single-threaded (workers=1) due to C extension |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟢 MINOR: Clarify Threading Comment
Explain why single-threading is needed:
# NLE must run single-threaded (workers=1) because the NetHack C extension
# uses global state and is not thread-safe| @@ -0,0 +1,126 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ POSITIVE: Excellent Example
This example is clear, well-documented, and demonstrates proper usage. Great work!
| @@ -0,0 +1,316 @@ | |||
| # NetHack Learning Environment (NLE) for OpenEnv | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ POSITIVE: Outstanding Documentation
This README is comprehensive, well-structured, and includes multiple examples. Excellent work on documentation!
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
✅ Deployment succeeded for
Nice work! Wait for a code review and we're ready to go. You can iterate locally or validate fixes by running |
Add NetHack Learning Environment (NLE) Integration
Summary
Adds a complete integration of the NetHack Learning Environment (NLE) as an OpenEnv environment.
What's Added
Core Implementation (8 files, ~600 LOC)
Models (
models.py):NLEAction- Discrete action space (113 actions: movement, commands, magic, inventory)NLEObservation- Rich observation space with 14+ types (~140KB JSON per step)NLEState- Episode state tracking (step count, game status, character info)Client (
client.py):NLEEnv- HTTP client implementingHTTPEnvClient[NLEAction, NLEObservation]from_docker_image()for automatic container deploymentServer (
server/):NLEEnvironment- Environment wrapper implementing OpenEnvEnvironmentABCreset(),step(),statepropertyapp.py- FastAPI application usingcreate_app()Dockerfile- Python 3.11 with NLE compiled from sourceEnvironment Characteristics
Action Space:
Observation Space (~140KB/step):
glyphs(21×79): Symbolic dungeon mapblstats(26): HP, XP, gold, dungeon level, etc.message(256): Game message bufferchars,colors,specials(21×79 each): Visual displayinv_glyphs,inv_strs,inv_letters,inv_oclasses: Inventorytty_chars,tty_colors,tty_cursor: Terminal displayReward:
Episode Termination:
Design Decisions & Assumptions
1. Assumption: Beefy Compute Available
Decision: Use simple JSON serialization, include all observations by default
2. Gym API Compatibility Layer
Decision: Support both gym 0.25 and 0.26+ APIs
3. Python 3.11 Requirement
4. Single Task Implementation
Decision: Implement only NetHackScore task (maximize score)
What Was Skipped / Deferred
Phase 2 Features (Low Priority)
These can be added later if needed:
Task Variants
Seeding API
env.seed(core=42, disp=123, reseed=False)TTYrec Recording/Replay
save_ttyrec_every=0)Wizard Mode
Performance Optimizations
References
rfcs/002-env-spec.md