Should we require a specific version of `lm_eval` to simplify `torchao/_models/_eval.py`?

Open ringohoffman opened this issue 1 year ago • 1 comments

As a follow-up to:

https://github.com/pytorch/ao/pull/1023

It is good practice to avoid having bare except clauses that suppress all errors.

cc: @msaroufim

A lot of the try/except blocks in torchao/_models/_eval.py seem like they are coming from trying to support multiple versions of lm_eval:

https://github.com/pytorch/ao/blob/e2301e9dba91fa962d673fdc3b3f0002856a3ba7/torchao/_models/_eval.py#L17-L22

https://github.com/pytorch/ao/blob/e2301e9dba91fa962d673fdc3b3f0002856a3ba7/torchao/_models/_eval.py#L41-L45

https://github.com/pytorch/ao/blob/e2301e9dba91fa962d673fdc3b3f0002856a3ba7/torchao/_models/_eval.py#L105-L108

There are further complications to this, including that in lm_eval==0.4.2, lm_eval.tasks.initialize_tasks was turned into a method of the new lm_eval.tasks.TaskManager class.

It could simplify the code considerably to pin lm_eval. torchtune does this for their eleuther_eval recipe.

Oct 07 '24 04:10 ringohoffman

we don't really have lm_eval as a dependency so i don't know if pinning it is really the solution here.

if you wanted to submit a PR getting rid of the version handling and instead have good error checking for lm_eval version with a good error message about what lm_eval version to install, I would accept that PR.

Oct 14 '24 19:10 HDCharles