Neel/spatial map answer extraction #57
Conversation
| @@ -68,8 +68,7 @@ | |||
| ASTEvalTransform, | |||
| PrependStringTransform, | |||
| ExtractAnswerGrid, | |||
| ExtractAnswerSpatialMap, | |||
| ExtractAnswerMaze, | |||
| ExtractAnswerSpatialMapAndMaze, | |||
There was a problem hiding this comment.
Please add "ExtractQuestionOptions" so that when we run the formatters this import does not get removed (considered unused).
There was a problem hiding this comment.
done and checked in
| The code is from: https://github.com/alvinmingwisc/spatial_reason_vlm/tree/main/eval, | ||
| and included with minimal modifications. | ||
| Extracts the answer from the text based on known model output patterns. | ||
| Searches for both a letter and whole word answer and returns both as they are not |
There was a problem hiding this comment.
Oh is this why you have added the OR metrics? If so, I don't think this justifies adding new metric classes, instead the answer extractor should return some combination of these two, maybe simply "x or y".
There was a problem hiding this comment.
Happy to do this another way. Especially if that means less new code. However, if let's say I return "x or y", I still need to a metric that knows how to check this. the current ones just look for case-sensitive or insensitive exact match.
One though was to make one metric that can take a single or a list (instead of having two as now) and an optional parameter for how to combine, i.e. any ("or") or all ("and"). what do you think?
There was a problem hiding this comment.
Now returning the "x or y" string
| if answers_match: | ||
| model_output_parsed = answers_match.group(1) | ||
|
|
||
| return [model_output_parsed, model_output_parsed_letter] |
There was a problem hiding this comment.
Would you mind adding a test for the extractor to showcase/check all these cases that you have covered? regex is hard to read and review, a test would give me peace of mind.
There was a problem hiding this comment.
added in a new file under tests/data_utils_tests
This PR is a set of changes to update and simplify the answer extraction or Spatial Map and Maze. Removes model specific answer extraction