Judge API V2 | Merge Judge and Inference configs #1776
Conversation
| @@ -76,37 +77,37 @@ class SimpleJudge(BaseJudge): | |||
| def __init__( | |||
| self, | |||
| judge_config: Union[JudgeConfig, str], | |||
There was a problem hiding this comment.
Is there a reason why you use this Union type everywhere? This isn't a pattern we have for any other CLI verb. Could you not just instantiate a judge_config from the path in the very beginning of the code, and pass around a JudgeConfig within the code?
There was a problem hiding this comment.
IMO, CLI should not be the only entry point to the judge code. For some cases, instantiating a SimpleJudge might be preferable and more flexible. Our 2 V1 notebooks as well showcase this: 1, 2.
So, when instantiating a SimpleJudge, I still want to be able to point to a default ("built-in") judge, rather than always having to construct a config object
|
Overall LGTM for readability. Please wait for another approval from someone with context. |
Description
Judge API V2: Merge Judge and Inference configs
JudgeConfigandInferenceConfigJudgeConfigconsisting of:JudgeParams(theJudgeConfigfrom before)InferenceConfigRelated issues
Towards OPE-1327
Before submitting
Reviewers
At least one review from a member of
oumi-ai/oumi-staffis required.