Conceptagent#274
Conversation
There was a problem hiding this comment.
I feel like maybe this file or the contents of it belongs more in the ai2thor interface component
There was a problem hiding this comment.
EDIT: Probably better to move this into the interfaces directory (perhaps under a ai2thor subdir)
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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 = ( |
There was a problem hiding this comment.
We generally store prompts under align_system/prompt_engineering or have them be variables rather than hardcoding them.
| class OllamaAI2ThorProposer(ProposerLLM): | ||
| """Proposer with prompts tailored for embodied navigation in AI2Thor.""" |
There was a problem hiding this comment.
If the only difference here between this and OllamaITMProposer then these should just be a single class with the prompt parameterized.
There was a problem hiding this comment.
Confused as to why this file is added here now, or where it came from (thought it already existing somewhere).
| _target_: align_system.interfaces.ai2thor_interface.AI2ThorInterface | ||
|
|
||
| scene: FloorPlan1 | ||
| prompts: [0] # 0=apple, 1=tomato, 2=red fruit |
There was a problem hiding this comment.
Not sure why it isn't better to have the actual prompts here rather than an index into a variable that lives somewhere else?
There was a problem hiding this comment.
Looks like some git conflict stuff got committed here.
…ted prompts to be called by a name. Deleted Unecessary files
| 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")) |
There was a problem hiding this comment.
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")]
There was a problem hiding this comment.
@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)
There was a problem hiding this comment.
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 = { |
There was a problem hiding this comment.
Again would be good to have this configurable (rather than hardcoded)
No description provided.