Skip to content

Conceptagent#274

Closed
Vman11 wants to merge 11 commits into
mainfrom
conceptagent
Closed

Conceptagent#274
Vman11 wants to merge 11 commits into
mainfrom
conceptagent

Conversation

@Vman11
Copy link
Copy Markdown

@Vman11 Vman11 commented May 15, 2026

No description provided.

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.

I feel like maybe this file or the contents of it belongs more in the ai2thor interface component

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.

EDIT: Probably better to move this into the interfaces directory (perhaps under a ai2thor subdir)

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.

Not certain yet how this piece fits into the whole ADM / pipeline but one though here is that we may not need to conform to the existing ITM scenario state format (as that's program evaluation specific).

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.

I think we should just be using the ollama_inference_engine here (or ideally any "structured inference engine" so that we can swap them around). Proposer and critic implementations should initialize with a reference to a structured inference engine to call.

"k": k,
}

prompt = (
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.

We generally store prompts under align_system/prompt_engineering or have them be variables rather than hardcoding them.

Comment on lines +197 to +198
class OllamaAI2ThorProposer(ProposerLLM):
"""Proposer with prompts tailored for embodied navigation in AI2Thor."""
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.

If the only difference here between this and OllamaITMProposer then these should just be a single class with the prompt parameterized.

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.

Confused as to why this file is added here now, or where it came from (thought it already existing somewhere).

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.

EDIT: Will be deleted

_target_: align_system.interfaces.ai2thor_interface.AI2ThorInterface

scene: FloorPlan1
prompts: [0] # 0=apple, 1=tomato, 2=red fruit
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.

Not sure why it isn't better to have the actual prompts here rather than an index into a variable that lives somewhere else?

Comment thread align_system/interfaces/ai2thor_interface.py
Comment thread align_system/interfaces/ai2thor_interface.py
Comment thread pyproject.toml
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.

Looks like some git conflict stuff got committed here.

dialog = []
dialog.insert(0,DialogElement(content=prompt_system, role="system"))
dialog.append(DialogElement(content=prompt, role="user"))
dialog.append(DialogElement(content=predict_proposer_prompt, role="user"))
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.

Minor thing, but this seems more clean and readable?

dialog = [DialogElement(content=prompt_system, role="system"),
          DialogElement(content=prompt, role="user"),
          DialogElement(content=predict_proposer_prompt, role="user")]

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.

@Vman11 FYSA Yoni is working on a variation of this that I think is a bit more generic. Once that lands I would like you to update this to use his approach (not expecting a heavy lift there)

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.

Seems like these types are ai2thor specific, so I'd prefer this file be called something like ai2thor_types.py to help namespace them.

from align_system.data_models.types import Action as MCTSAction


TASKS = {
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.

Again would be good to have this configurable (rather than hardcoded)

@dmjoy dmjoy closed this May 21, 2026
@dmjoy dmjoy deleted the conceptagent branch May 21, 2026 18:32
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