transformers icon indicating copy to clipboard operation
transformers copied to clipboard

enable low-precision pipeline

Open jiqing-feng opened this issue 1 year ago • 5 comments

Hi @amyeroberts @Narsil .

As the previous PR #31444 mentioned, it could enable low-precision pipelines by converting the outputs to float(). I followed the codes in here. Do you mind taking a review? Thx!

jiqing-feng avatar Jun 26 '24 03:06 jiqing-feng

Related to https://github.com/huggingface/transformers/pull/31342 but I dont quite get your changes - what exactly does it fix? When I tested all the pipelines in fp16 none of them had issues outputting logits

aliencaocao avatar Jun 26 '24 13:06 aliencaocao

Related to #31342 but I dont quite get your changes - what exactly does it fix? When I tested all the pipelines in fp16 none of them had issues outputting logits

Hi @aliencaocao , FP16 works fine but BF16 is not acceptable for numpy, you can see: image

jiqing-feng avatar Jul 04 '24 03:07 jiqing-feng

Hi @SunMarc. I have fixed all your comments; please review them. BTW, the failed tests are due to the connection error, not related to my changes.

jiqing-feng avatar Jul 05 '24 03:07 jiqing-feng

Hi @amyeroberts , would you please review this PR? Thx!

jiqing-feng avatar Jul 10 '24 05:07 jiqing-feng

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Hi @amyeroberts . I have passed all tests, do you mind take a review and merge it? Thx!

jiqing-feng avatar Jul 16 '24 02:07 jiqing-feng

Hi @amyeroberts . This PR should be ready to merge, please take a review, thx!

jiqing-feng avatar Jul 17 '24 01:07 jiqing-feng

Hi @jiqing-feng, thanks for opening this PR! I'll get to reviewing it soon, but will likely be in a few days.

amyeroberts avatar Jul 17 '24 16:07 amyeroberts

Hi @amyeroberts . Do you mind reviewing this PR? Thx.

jiqing-feng avatar Jul 30 '24 01:07 jiqing-feng

Hi @amyeroberts . Do you think it could be merged?

jiqing-feng avatar Aug 06 '24 02:08 jiqing-feng

@amyeroberts , could you help review this PR? Thx.

yao-matrix avatar Aug 19 '24 02:08 yao-matrix

@SunMarc , do you have a suggestion any other people from HF can help review and merge this PR? Seems @amyeroberts has no bandwidth on this these serval months. Thx.

yao-matrix avatar Sep 13 '24 00:09 yao-matrix

Sorry for the delay @yao-matrix!

@Rocketknight1 is managing the pipeline; Matt, would you mind reviewing this PR when you have a second?

LysandreJik avatar Sep 17 '24 10:09 LysandreJik

Overall this PR seems good to me! However, I prefer if outputs.dtype in (torch.bfloat16, torch.float16) rather than if outputs.dtype == torch.bfloat16, so we can catch float16 as well.

Other than that, I'm happy with it!

Hi @Rocketknight1 , thanks for your review, I have fixed it by your comments.

jiqing-feng avatar Sep 18 '24 01:09 jiqing-feng

Hi @Rocketknight1 , do you mind helping to re-run the tests? Thx.

jiqing-feng avatar Sep 18 '24 06:09 jiqing-feng

Hi @jiqing-feng, another PR at #33554 touched the same files. I'm sorry - I didn't realize that it was doing the same thing as this PR! That PR has been merged, so I've merged this PR with the code there to avoid conflicts.

Rocketknight1 avatar Sep 18 '24 15:09 Rocketknight1

@jiqing-feng tests pass now and this looks good - are you okay for us to merge it?

Also cc @LysandreJik for final review, but no rush!

Rocketknight1 avatar Sep 18 '24 15:09 Rocketknight1

@jiqing-feng tests pass now and this looks good - are you okay for us to merge it?

Yes, please.

jiqing-feng avatar Sep 19 '24 00:09 jiqing-feng