Skip to content

Add eye-illustration GRPO task (p5.brush render + graded reference judge) - #451

Draft
joyliu-q wants to merge 16 commits into
mainfrom
devin/1787670158-eye-rl
Draft

Add eye-illustration GRPO task (p5.brush render + graded reference judge)#451
joyliu-q wants to merge 16 commits into
mainfrom
devin/1787670158-eye-rl

Conversation

@joyliu-q

@joyliu-q joyliu-q commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

A GRPO task that trains a model to illustrate an eye: the policy emits a complete p5.js/p5.brush sketch, the reward function renders it to PNG in a Modal Sandbox, and a VLM critic grades the render against crops of a reference photograph. Output stays editable source code, and each rollout's render shows up in the dashboard through sample.metadata["image"] (#450).

eye_rl/train_eyes.py is the whole task: prompt dataset, system prompt, sandbox renderer, judge, reward, launcher.

def eye_rm(sample):
    code = extract_sketch(sample.response)          # js fence, one setup(), no net/DOM
    png = render_sketch(code)                        # chromium + p5.brush in a Sandbox
    score = ink_and_coverage_gates(png) and judge(png, reference_crop)
    sample.metadata["image"] = png                   # → dashboard rollout viewer

Three things the renderer has to do that aren't obvious:

  • p5.brush leaks a transform. Its watercolour mask flush leaves a full-canvas modelview matrix behind, so every brush.fill after a colour change landed half a canvas off — which looked like fills erasing the drawing. Each brush method is wrapped in a guard that snapshots and restores _renderer.uMVMatrix.mat4; that's what makes painterly output possible at all.
  • Models hallucinate brush APIs. A proxy falls unknown brush names back to HB, no-ops unknown methods, aliases the bare pick/spline/hatch globals, and retries with the blamed line stripped — render success went 27/64 → 63/64 samples per rollout.
  • Judges are gameable. Absolute Y/N scoring was answered YYYYY for ink clouds and crosshairs, so the reward is a 3-vote graded comparison against a reference at temperature, behind ink/coverage/speckle gates, with sclera/iris/lash checks capping the rating and a separate lash-fan term.

eye_rl/test_reward.py is a dependency-free regression over the gates, the extractor, the renderer and the judge parser.

Trained in joy-dev on Qwen3.5-4B, continuing across checkpoints to step 160; reward 0.03 → 0.56, and 64/64 samples render a socket wash, almond sclera, gradient iris, pupil, lash fan and brow.

step 159 renders

Checklist

  • Example is documented with comments throughout, in a Literate Programming style.
  • Example does not require third-party dependencies to be installed locally
  • Example pins its dependencies
    • Example pins container images to a stable tag, not a dynamic tag like latest
    • Example specifies a python_version for the base image, if it is used
    • Example dependencies are pinned (p5@1.11.3, p5.brush@1.1.2, puppeteer-core@23.11.1)

Link to Devin session: https://modal.devinenterprise.com/sessions/db37ccdaa48246ffa16776b8b9aa35a5
Requested by: @joyliu-q

@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 3 new potential issues.

Open in Devin Review

Comment thread eye_rl/train_eyes.py Outdated
Comment thread eye_rl/train_eyes.py Outdated
Comment thread eye_rl/train_eyes.py
Comment on lines +196 to +200
const pick = real.pick, setHatch = real.setHatch;
const noop = () => {};
const patched = {
pick: (n) => pick(names.includes(n) ? n : "HB"),
setHatch: (n, c, w) => setHatch(names.includes(n) ? n : "hatch_brush", c, w),

@devin-ai-integration devin-ai-integration Bot Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Detached brush.pick/setHatch may lose this-binding

The renderer facade captures real.pick and real.setHatch as bare references and calls them with no receiver, so this is undefined inside them. If p5.brush's implementations rely on this, name normalization could throw or misbehave. p5.brush 1.1.2 internals were not verified.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

p5.brush 1.1.2 exports pick/setHatch as standalone functions on a namespace object, not methods relying on this — renders succeed (60-62 of 64 samples per rollout return render: ok, including hatched irises which go through setHatch).

@devin-ai-integration devin-ai-integration Bot changed the title Add eye-illustration GRPO task (p5.brush render + graded reference judge) Add anime eye-illustration GRPO task (p5.js render + reference-graded judge) Aug 25, 2026
@devin-ai-integration devin-ai-integration Bot changed the title Add eye-illustration GRPO task (p5.js render + reference-graded VLM judge) Add eye-illustration GRPO task (p5.brush render + graded reference judge) Aug 26, 2026
Restore p5's modelview matrix around each brush call: flushing a watercolour
mask leaves the full-canvas quad transform in the matrix, so every shape after a
fill colour change landed half a canvas off. With that fixed brush.fill/bleed
work in the headless sandbox, so the grammar paints skin, sclera and iris as
washes instead of banning fills.
The first watercolour run copied the prompt's small example radii and drew a
120px eye on an empty skin canvas, so the ink gate zeroed most samples. State
that brush.circle takes a radius and give full-scale numbers for the socket,
opening, iris and lash walk.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 2 new potential issues.

Open in Devin Review

Comment thread eye_rl/train_eyes.py
Comment on lines +750 to +768
if model == "27b":
config = TrainConfig(
model=Qwen3_8_27B(),
dataset=dataset,
recipe=Qwen3_8_27b_Recipe(
custom_rm_function=eye_rm,
num_rollout=num_rollout,
rollout_batch_size=8,
n_samples_per_prompt=8,
rollout_max_response_len=3072,
rollout_temperature=1.0,
save_interval=10,
apply_chat_template_kwargs='{"enable_thinking": false}',
image_overlay=overlay_all_images,
),
)
result = config.train()
print(f"Training run id: {result.training_run_id}")
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 27b training silently ignores the checkpoint argument

The 27b branch builds Qwen3_8_27b_Recipe without forwarding load (or the override_opt_param_scheduler override the 4b branch sets at train_eyes.py). Running launch(model="27b", load=...) trains from scratch instead of continuing from the checkpoint the docstring promises.

Prompt for agents
In launch() in eye_rl/train_eyes.py, the model=="27b" branch constructs Qwen3_8_27b_Recipe but never passes the `load` argument, so continuing from a checkpoint (which the function docstring says `load` enables) has no effect for the 27b model. The 4b branch correctly passes `load=load` and sets extra_config={"override_opt_param_scheduler": True} when load is set. Qwen3_8_27b_Recipe is a SlimeRecipe subclass and supports both `load` and extra_config. Add the same `load=load` and conditional extra_config handling to the 27b Qwen3_8_27b_Recipe construction so checkpoint continuation works consistently across both model paths.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Correct — fixed: the 27b recipe now takes load=load and the same override_opt_param_scheduler override as the 4b path.

Comment thread eye_rl/train_eyes.py
Comment on lines +397 to +399
proc.wait()
out = proc.stdout.read()
err = proc.stderr.read()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Sequential stdout/stderr read after sandbox wait

render_sketch calls proc.wait() then reads stdout and stderr sequentially. If Modal's sandbox streams are pipe-buffered rather than fully buffered on completion, a large base64 PNG on stdout plus stderr output could stall. Worth confirming Modal Sandbox stream semantics handle large payloads here.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Checked against the real libraries rather than reasoning about init order: the facade block runs from a <script> tag before the sketch, and p5 1.11.3's global-mode _setup does not re-copy lerpColor over the wrapper — sketches passing hex strings to lerpColor render their gradients instead of throwing, both locally and through render_sketch in the sandbox. Across a 64-sample rollout there are no lerpColor-blamed recovery attempts in the render stderr.

Replaces the absolute 0-4 reference rubric with a ridge probe fitted on all
220 hand ratings (love/okay/nope) over CLIP embeddings, weighted 0.55, and
keeps the 27B VLM only as an anatomy/eyelash critic (0.25/0.20).

The probe lock is created on first use: a live threading.Lock among the
module globals made the reward unpicklable, so cloudpickle silently shipped
the reward without its helpers and rollouts died with NameError.
With taste as the only learned term, 210 steps drove every sample to the same
pale watercolour eye whatever style the prompt asked for: one template scores
acceptably for all seven families. style_score ranks the render against short
CLIP phrases for all seven and rewards the mass on the family that was asked
for (41% top-1 on the rated pool against 14% chance; the collapsed template
scores 0), at 0.20 weight taken from taste.
The softmax form of style_score was all-or-nothing: on a full collapsed rollout
every one of 64 samples scored exactly 0, so the term only shifted the reward
level and never told the policy which sample was closer to the brief. Scoring
the cosine margin between the asked-for family and the closest competing one
varies within a batch, putting the collapsed template near 0.25 and the
hand-rated stylised pool near 0.75.

The judge's anatomy and lash answers have saturated (0.97 and 0.99 mean), so
their weight was a constant offset; it moves to taste and style, and anatomy
keeps its teeth through the caps.
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.

1 participant