Nota-NetsPresso / shortened-llm

Compressed LLMs for Efficient Text Generation [ICLR'24 Workshop]
63 stars 8 forks source link

Refactor data-related code & Add BOS-token option #5

Closed bokyeong1015 closed 6 months ago

bokyeong1015 commented 6 months ago

Description

Changes

Note

lifelongeeek commented 6 months ago

@bokyeong1015 Most changes in this PR are LGTM. I have two suggestions:

  1. As one of the unit tests, could you provide some example setting/result that reproduce metrics in paper or technical reports?

  2. Why don't we make default value of add_bos_to_every=True? My understanding is that keeping the add_bos_to_every=False setting is to reproduce past paper results, however, this setting cause inconsistent evaluation between samples in batches. (i.e., BOS is appended for only first sample).

bokyeong1015 commented 6 months ago
  1. Example scripts and results are provided. By the way, the reproducibility of the current PR version has been carefully checked.
  2. Good comment. The current repo is primarily aimed at reproducing the paper; however, I strongly agree with your feedback and will consider it for future updates :)