feat: Add support for multiple histories #170
Conversation
| ): | ||
| results.append(result) | ||
| trajectory_results: list[TokenizedResult] = [] | ||
| for history in [ |
There was a problem hiding this comment.
Probably doesn't matter but we could make Trajectory inherit from History to avoid having to create a new one here.
There was a problem hiding this comment.
I do think it would be good to have an explicit way to mark an assistant message as trainable or not. I guess we get that right now by passing Choice vs Message objects but if we end up standardizing on using Message objects everywhere (or someone wants to pass allow_training_without_logprobs for any reason) we'll have an issue. That can come later though I suppose.
Also, just checking that these are being masked out somewhere so the different histories in the same trajectory don't attend to each other?
|
@corbt no masking, here's a caveat I shared in our discussion:
I also mused "maybe we could add support for auto-splitting reasoning trajectories once we figure out what works," but I'm not ready to tackle that atm |
|
Ah I may have been unclear; my question was about attention masking, not loss masking. Are we masking to make sure the different message histories within a trajectory aren't able to attend to each other? |
|
@corbt yes, each history will yield it's own |
There was a problem hiding this comment.
Sweet, excited to have this in. @arcticfly we'll definitely have to document how this works.
No description provided.