aws / fmeval

Foundation Model Evaluations Library
http://aws.github.io/fmeval
Apache License 2.0
151 stars 40 forks source link

feat: add target_context to dataset columns #266

Closed oyangz closed 1 month ago

oyangz commented 2 months ago

Issue #, if available:

Description of changes:

Update: Initially we planned on the target_context dataset column taking list of strings. This does not work with ray operations such as map_batches due to issues including https://github.com/ray-project/ray/issues/39559 and other unsupported data type errors. Thus the target_context dataset column has been modified to take a string, and we will use string concatenation when there are multiple target contexts, similar to the existing target_output field.

Description (updated):

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

franluca commented 2 months ago

Non-blocking thought: is target context the only column type that we anticipate being a special case (ie requiring different logic for json parsing)? If we end up having other columns that are similar in behavior to target context, we will want to group these column types together.

Just to double check, is the different logic there because we expect the context to be a list? If so:

  1. what happens if it is actually not a list?
  2. this is pretty similar to the situation with multiple ground truths, which we had the workaround to concatenate together using the "". Is this no longer necessary? if so, I find it a bit undesirable to have different treatment for the same case.
xiaoyi-cheng commented 2 months ago

this is pretty similar to the situation with multiple ground truths, which we had the workaround to concatenate together using the "". Is this no longer necessary? if so, I find it a bit undesirable to have different treatment for the same case.

Interesting. Let's hold off this PR and discuss offline. Maybe we should keep target_context as a string and concatenate the strings together.