Skip to content

Model scaffolding and runs#4

Open
seanrivera wants to merge 15 commits into
mainfrom
model-scaffolding-and-runs
Open

Model scaffolding and runs#4
seanrivera wants to merge 15 commits into
mainfrom
model-scaffolding-and-runs

Conversation

@seanrivera
Copy link
Copy Markdown
Member

This is part 3 of the big 4 part code review. This is the scaffolding and interface code for different models. Currently mostly focused on local models, with a hacky interface to deal with chat interfaces for frontier models.

@seanrivera seanrivera requested a review from pranavguru April 25, 2026 21:24
Copy link
Copy Markdown
Member

@pranavguru pranavguru left a comment

Choose a reason for hiding this comment

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

Did not review the multigrid portion as it is not high priority now

Comment thread pyproject.toml Outdated
)

# Process and generate
inputs = self.processor(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why is the prompt here different from the prompt used in the lmstudio and ollama adapters?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Honestly Paligemma is both too old and weak to handle grid nav, and the adapter has been left behind for many iterations. We can probably remove it, if we're fine cutting some comparisons from v1.0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds good - lets remove it then @seanrivera

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we can get rid of GUIAction - we are not doing this as a separate domain as of now

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@seanrivera this hasn't been addressed yet

self._task_spec = task_spec
return task_spec

def to_canonical(self, domain_spec: TaskSpecification) -> CanonicalTaskSpec:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I can see some issues in the round trip conversion (from_canonical --> to_canonical )

  • Rules are silently dropped: from_canonical always produces Rules() defaults (line 184); to_canonical doesn't serialize Rules at all. So a maze with observability="view_cone", key_consumption=False, or
    hidden_mechanisms=["s1"] round-trips into a maze without those constraints. The hidden-switch tier-5 mazes lose all their identifying features. Either add CanonicalRules to the canonical taxonomy or stuff into domain_config. Seems like there is a similar issue with dependency_chain and distractors.
  • Coordinate normalization is lossy at boundaries (lines 80–82). int(pos[0] * (grid_w - 1)) plus the max/min clamp silently moves objects toward the interior. Combined with to_canonical's pos.x / (grid_w -1), the round trip isn't idempotent at right/bottom edges.

Double check if the above are issues

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@seanrivera this hasn't been addressed yet

Comment thread gridworld/actions.py
from typing import Dict


class MiniGridActions(IntEnum):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is this the official action space defined by MiniGrid?

Copy link
Copy Markdown
Member

@pranavguru pranavguru May 8, 2026

Choose a reason for hiding this comment

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

@seanrivera this hasn't been answered yet

@seanrivera seanrivera force-pushed the model-scaffolding-and-runs branch from 0445a24 to fe82acd Compare May 1, 2026 22:08
@seanrivera seanrivera force-pushed the model-scaffolding-and-runs branch from fe82acd to 1256b78 Compare May 9, 2026 21:38
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