Add gpu-mode-tutorial from huggingface/gpu-mode-openenv (d3d8e7c)#426
Closed
kiankyars wants to merge 1 commit into
Closed
Add gpu-mode-tutorial from huggingface/gpu-mode-openenv (d3d8e7c)#426kiankyars wants to merge 1 commit into
kiankyars wants to merge 1 commit into
Conversation
Made-with: Cursor
meta-cla
Bot
added
the
CLA Signed
Contributor
Greptile SummaryThis PR restores the Tier 1 Issues (Fix Immediately):
Tier 2 Issues (Alignment Discussion Required):
Confidence Score: 2/5
Sequence DiagramsequenceDiagram
participant T as GRPOTrainer
participant R as rollout_func
participant E as TextArenaEnv (Server)
participant vLLM as vLLM
T->>R: rollout_func(prompts)
loop For each prompt
R->>E: env.reset() [WebSocket /ws]
E-->>R: StepResult(observation)
loop Up to max_turns
R->>vLLM: generate_rollout_completions(prompt_text)
vLLM-->>R: prompt_ids, completion_ids, logprobs, text
R->>E: env.step(TextArenaAction) [WebSocket /ws]
E-->>R: StepResult(reward, observation, done)
Note over R: ⚠️ green/yellow/repetition<br/>scores computed HERE<br/>(client-side — invariant violation)
end
end
R-->>T: {prompt_ids, completion_ids, logprobs,<br/>correct_reward, green_reward, ...}
T->>T: Optimize policy with reward_funcs
Last reviewed commit: 2018cfb |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| def create_qwen_llm(): | ||
| """Create an LLM function using the smallest Qwen instruct model.""" | ||
| #from transformers import AutoModelForCausalLM, AutoTokenizer |
Contributor
There was a problem hiding this comment.
Commented-out debug import should be removed.
Suggested change
| #from transformers import AutoModelForCausalLM, AutoTokenizer |
Prompt To Fix With AI
This is a comment left during a code review.
Path: gpu-mode-tutorial/walkthrough/examples/repl_with_llm.py
Line: 43
Comment:
Commented-out debug import should be removed.
```suggestion
```
How can I resolve this? If you propose a fix, please make it concise.| """Generate response using Qwen model.""" | ||
| response = client.chat.completions.create( | ||
| messages=messages, | ||
| max_tokens=2048, # Inrecreased for longer code responses |
Contributor
There was a problem hiding this comment.
Typo in inline comment: "Inrecreased" should be "Increased".
Prompt To Fix With AI
This is a comment left during a code review.
Path: gpu-mode-tutorial/walkthrough/examples/repl_with_llm.py
Line: 60
Comment:
Typo in inline comment: `"Inrecreased"` should be `"Increased"`.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+357
to
+376
| feedback = extract_wordle_feedback(observation) | ||
|
|
||
| # Update guess counts | ||
| previous_occurrences = guess_counts[guess] | ||
| repetition_score = scale_repetition_score(previous_occurrences, len(guess_counts)) | ||
| guess_counts[guess] += 1 | ||
|
|
||
| # calculate custom reward signals from the feedback | ||
| if not feedback: | ||
| green_score = 0.0 | ||
| yellow_score = 0.0 | ||
| else: | ||
| green_count, yellow_count = extract_feedback_counts(feedback) | ||
| green_score = green_count / 5.0 | ||
| yellow_score = yellow_count / 5.0 | ||
|
|
||
| repetition_scores.append(repetition_score) | ||
| green_scores.append(green_score) | ||
| yellow_scores.append(yellow_score) | ||
| correct_scores.append(correct_score) |
Contributor
There was a problem hiding this comment.
ALIGNMENT FLAG: External reward computation violates "Rewards in environment" invariant.
- Principle at stake: INVARIANTS.md §Architectural Invariants §3 ("Rewards in environment")
- The concern: The
rollout_oncefunction computes auxiliary reward signals (green_score,yellow_score,repetition_score) entirely on the client side by parsing the raw observation text using helpers from the env package (lines 357–376). According to the architectural invariant, "Reward computation must stay inside environment boundary" and "External reward augmentation uses Transform pipeline" (server-side only). These auxiliary signals should not be calculated in the training orchestration script. - Suggested reviewer:
@darktex
Prompt To Fix With AI
This is a comment left during a code review.
Path: gpu-mode-tutorial/walkthrough/examples/wordle.py
Line: 357-376
Comment:
**ALIGNMENT FLAG**: External reward computation violates "Rewards in environment" invariant.
- **Principle at stake**: INVARIANTS.md §Architectural Invariants §3 ("Rewards in environment")
- **The concern**: The `rollout_once` function computes auxiliary reward signals (`green_score`, `yellow_score`, `repetition_score`) entirely on the client side by parsing the raw observation text using helpers from the env package (lines 357–376). According to the architectural invariant, "Reward computation must stay inside environment boundary" and "External reward augmentation uses Transform pipeline" (server-side only). These auxiliary signals should not be calculated in the training orchestration script.
- **Suggested reviewer**: `@darktex`
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+239
to
+275
| ┌────────────────────────────────────────────────────────────┐ | ||
| │ YOUR TRAINING CODE │ | ||
| │ │ | ||
| │ env = OpenSpielEnv(...) ← Import the client │ | ||
| │ result = env.reset() ← Type-safe! │ | ||
| │ result = env.step(action) ← Type-safe! │ | ||
| │ │ | ||
| └─────────────────┬──────────────────────────────────────────┘ | ||
| │ | ||
| │ HTTP/JSON (Language-Agnostic) | ||
| │ POST /reset, POST /step, GET /state | ||
| │ | ||
| ┌─────────────────▼──────────────────────────────────────────┐ | ||
| │ DOCKER CONTAINER │ | ||
| │ │ | ||
| │ ┌──────────────────────────────────────────────┐ │ | ||
| │ │ FastAPI Server │ │ | ||
| │ │ └─ Environment (reset, step, state) │ │ | ||
| │ │ └─ Your Game/Simulation Logic │ │ | ||
| │ └──────────────────────────────────────────────┘ │ | ||
| │ │ | ||
| │ Isolated • Reproducible • Secure │ | ||
| └────────────────────────────────────────────────────────────┘ | ||
| ``` | ||
|
|
||
| !!! info "Key Insight" | ||
| You never see HTTP details - just clean Python methods! | ||
|
|
||
| ```python | ||
| env.reset() # Under the hood: HTTP POST to /reset | ||
| env.step(...) # Under the hood: HTTP POST to /step | ||
| env.state() # Under the hood: HTTP GET to /state | ||
| ``` | ||
|
|
||
| The magic? OpenEnv handles all the plumbing. You focus on RL! ✨ | ||
|
|
||
| --- |
Contributor
There was a problem hiding this comment.
ALIGNMENT FLAG: Architecture diagram depicts deprecated HTTP-only communication pattern.
- Principle at stake: INVARIANTS.md §Architectural Invariants §4 ("Communication patterns")
- The concern: The ASCII diagram (lines 239–262) and "Key Insight" callout (lines 264–271) present the client-server interface exclusively via
HTTP POST /reset,HTTP POST /step, andHTTP GET /state. However, INVARIANTS.md explicitly states that WebSocket is the canonical protocol and HTTP is being deprecated in favor of WebSocket-only (PR [PATCH] Deprecate HTTP in websockets #252). While both protocols are currently available during the transition, tutorial content should teach the canonical WebSocket pattern as primary, with HTTP noted only as auxiliary or deprecated. This diagram may mislead newcomers about the current best practice. - Suggested reviewer:
@darktex
Prompt To Fix With AI
This is a comment left during a code review.
Path: gpu-mode-tutorial/walkthrough/01-environments.md
Line: 239-275
Comment:
**ALIGNMENT FLAG**: Architecture diagram depicts deprecated HTTP-only communication pattern.
- **Principle at stake**: INVARIANTS.md §Architectural Invariants §4 ("Communication patterns")
- **The concern**: The ASCII diagram (lines 239–262) and "Key Insight" callout (lines 264–271) present the client-server interface exclusively via `HTTP POST /reset`, `HTTP POST /step`, and `HTTP GET /state`. However, INVARIANTS.md explicitly states that WebSocket is the canonical protocol and HTTP is being deprecated in favor of WebSocket-only (PR #252). While both protocols are currently available during the transition, tutorial content should teach the canonical WebSocket pattern as primary, with HTTP noted only as auxiliary or deprecated. This diagram may mislead newcomers about the current best practice.
- **Suggested reviewer**: `@darktex`
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
restores the gpu-mode-tutorial folder that was referenced in the readme (f5c5594) but never added. content copied from huggingface/gpu-mode-openenv at commit d3d8e7c - the same commit that was moved out of openenv per the hf repo history
sorry for the earlier pr spam