Bug: Data overwrite in generation_only mode due to static file path
Issue Type: Logical Bug / Data Loss
Description
While conducting a static code review of runner.py, I identified a logical flaw in the --generation_only branch. When multiple tasks are specified (e.g., via wildcards like --tasks HumanEval/*), the script overwrites previous generation results, retaining only the outputs of the last task in the list.
Root Cause Analysis
Inside the for task in task_names: loop, the script writes to the output file using write mode ("w"). Since the file path (e.g., args.save_generations_raw_path) is defined statically outside the loop, each iteration truncates the file and discards data from the previous iteration.
Offending code snippet (in runner.py):
for task in task_names:
if args.generation_only:
# ... generation logic ...
if accelerator.is_main_process:
if args.save_generations_raw:
with open(args.save_generations_raw_path, "w") as fp: # <-- Static path + Overwrite
json.dump(generations_raw, fp)
Why this is a problem (Logical Deduction)
Assuming task_names = ["Task_A", "Task_B", "Task_C"]:
- Iteration 1 (Task_A): Creates the file and writes A's data.
- Iteration 2 (Task_B): Opens the same file, truncates it (erases A), and writes B's data.
- Iteration 3 (Task_C): Opens the same file, truncates it (erases B), and writes C's data.
Final Result: The JSON file contains only Task_C's results. All other task generations are lost.
Suggested Fix (Aggregate first, write once)
To resolve this, move the file writing logic outside the loop. Aggregate all generated outputs into dictionaries first, then write them in a single operation:
# Initialize aggregators before the loop
all_raw = {}
all_prc = {}
all_refs = {}
for task in task_names:
if args.generation_only:
if accelerator.is_main_process:
print("Generation mode only")
generations_prc, generations_raw, references = evaluator.generate_text(task)
# Aggregate by task name
all_raw[task] = generations_raw
all_prc[task] = generations_prc
all_refs[task] = references
# Write everything once after the loop
if accelerator.is_main_process:
if args.save_generations_raw:
with open(args.save_generations_raw_path, "w") as fp:
json.dump(all_raw, fp)
print("raw generations were saved")
if args.save_generations_prc:
with open(args.save_generations_prc_path, "w") as fp:
json.dump(all_prc, fp)
print("processed generations were saved")
if args.save_references:
with open(args.save_references_path, "w") as fp:
json.dump(all_refs, fp)
print("references were saved")
Environment
Discovered via static analysis; no specific runtime environment required. (I don't have a GPU environment to test this empirically, but the logic flaw is clear from the control flow.)
Additional Context
This bug does not affect the normal evaluation mode (without --generation_only) because results is aggregated and written after the loop. The fix aligns the generation_only behavior with the intended design.
Thank you for your excellent work on LINC!
Bug: Data overwrite in
generation_onlymode due to static file pathIssue Type: Logical Bug / Data Loss
Description
While conducting a static code review of
runner.py, I identified a logical flaw in the--generation_onlybranch. When multiple tasks are specified (e.g., via wildcards like--tasks HumanEval/*), the script overwrites previous generation results, retaining only the outputs of the last task in the list.Root Cause Analysis
Inside the
for task in task_names:loop, the script writes to the output file using write mode ("w"). Since the file path (e.g.,args.save_generations_raw_path) is defined statically outside the loop, each iteration truncates the file and discards data from the previous iteration.Offending code snippet (in
runner.py):Why this is a problem (Logical Deduction)
Assuming
task_names = ["Task_A", "Task_B", "Task_C"]:Final Result: The JSON file contains only Task_C's results. All other task generations are lost.
Suggested Fix (Aggregate first, write once)
To resolve this, move the file writing logic outside the loop. Aggregate all generated outputs into dictionaries first, then write them in a single operation:
Environment
Discovered via static analysis; no specific runtime environment required. (I don't have a GPU environment to test this empirically, but the logic flaw is clear from the control flow.)
Additional Context
This bug does not affect the normal evaluation mode (without
--generation_only) becauseresultsis aggregated and written after the loop. The fix aligns thegeneration_onlybehavior with the intended design.Thank you for your excellent work on LINC!