remove fallback row add #32
Conversation
There was a problem hiding this comment.
I would tend to agree that we don't want to have bogus rows among the normal rows, but at the same time it provides a good way to store the doc/chunk which failed.
If we remove this, I believe a logging of the chunks and docs in error should occur in a dedicated file.
wdyt @alozowski ?
| @@ -291,19 +291,6 @@ def _process_responses_and_build_dataset( | |||
| logger.warning( | |||
| f"No parseable JSON found (or empty list) for row_index={row_index}, chunk_id={chunk_id}, model={model_name}. Creating fallback row." | |||
There was a problem hiding this comment.
| f"No parseable JSON found (or empty list) for row_index={row_index}, chunk_id={chunk_id}, model={model_name}. Creating fallback row." | |
| f"No parseable JSON found (or empty list) for row_index={row_index}, chunk_id={chunk_id}, doc_id={doc_id}, and model={model_name}." |
There was a problem hiding this comment.
unfortunately, we’ll also need to cascade catch this during answer generation, judging, etc, which seems like asking for inaccuracies / errors.
+1 for logging it somewhere however
There was a problem hiding this comment.
Yep I agree - just want to make sure with Alina that there isn't something we haven't thought of handled here
There was a problem hiding this comment.
Not sure if this deletion might cause issues, @sumukshashidhar did you test it with this fallback_row removed?
There was a problem hiding this comment.
yes I did. the only difference is that we no longer have an "unknown" "unknown" row that would cause cascading errors
|
Approving, but please create an issue so we remember to log this |
There is an unnatural row add that occurs if the JSON is unparsable. We do not need this, as it produces a bogus question row