Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make model_id and model_family in PipelineContext optional #511

Closed
bbrowning opened this issue Jan 27, 2025 · 1 comment · Fixed by #518
Closed

Make model_id and model_family in PipelineContext optional #511

bbrowning opened this issue Jan 27, 2025 · 1 comment · Fixed by #518
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@bbrowning
Copy link
Contributor

The configuration of an LLMBlock in a pipeline yaml file should be able to define its own model family and model id to use and it can differ per LLMBlock within a single Pipeline. Thus, we should not require one to be set for the entire PipelineContext and instead these should be treated as optional parameters that override the values found within the yaml config.

Try to do this in a backwards-compatible way, where existing code still works but we just make these optional and shift users to doing this in their Block configuration instead of their PipelineContext.

@bbrowning bbrowning added this to the 0.8.0 milestone Jan 28, 2025
@bbrowning
Copy link
Contributor Author

This is necessary as we work towards more flexible Pipelines, where each LLMBlock of a pipeline can call different models instead of requiring all LLMBlocks in a pipeline to only talk to a single model.

@bbrowning bbrowning self-assigned this Jan 28, 2025
@bbrowning bbrowning added the enhancement New feature or request label Jan 28, 2025
bbrowning added a commit to bbrowning/instructlab-sdg that referenced this issue Jan 28, 2025
This makes model_family, model_id, and num_instructions_to_generate
optional on PipelineContext instead of required. This should be a
backwards-compatible change, and includes tests that verify passing
the previously-required parameters into PipelineContext work as
expected. This is a move towards supporting and expecting Pipelines
that have LLMBlocks that may use different model families or different
model ids.

If a Block's yaml config specifies a model_family or model_id and none
is set on the PipelineContext, then the Block's values will get
used. However, if these are set on the PipelineContext, they're
treated as overrides and will override the values for all Blocks in
this Pipeline. This behavior emulates the previous behavior, but we
should steer users away from ever setting model_id or model_family on
a PipelineContext if the Pipeline contains multiple LLMBlock entries.

Fixes instructlab#511

Signed-off-by: Ben Browning <[email protected]>
bbrowning added a commit to bbrowning/instructlab-sdg that referenced this issue Jan 29, 2025
This makes model_family, model_id, and num_instructions_to_generate
optional on PipelineContext instead of required. This should be a
backwards-compatible change, and includes tests that verify passing
the previously-required parameters into PipelineContext work as
expected. This is a move towards supporting and expecting Pipelines
that have LLMBlocks that may use different model families or different
model ids.

If a Block's yaml config specifies a model_family or model_id and none
is set on the PipelineContext, then the Block's values will get
used. However, if these are set on the PipelineContext, they're
treated as overrides and will override the values for all Blocks in
this Pipeline. This behavior emulates the previous behavior, but we
should steer users away from ever setting model_id or model_family on
a PipelineContext if the Pipeline contains multiple LLMBlock entries.

Fixes instructlab#511

Signed-off-by: Ben Browning <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant