torchtune icon indicating copy to clipboard operation
torchtune copied to clipboard

Improvement: add a "division by zero" check in chunked loss handling in kd_losses.py

Open insop opened this issue 1 year ago • 3 comments

Suggested improvement:

It would be good to add a "division by zero" check in chunked loss handling in kd_losses.py.

Context: This is based on the discussion in PR #2094.

Potential issue: ForwardKLWithChunkedOutputLoss does not check for division by zero, while the non-chunked version does.

insop avatar Jan 02 '25 23:01 insop

Great catch! We'd definitely welcome a small PR for this if you want to do it, otherwise we can try to get to it soon.

joecummings avatar Jan 06 '25 14:01 joecummings

I can make a change. @felipemello1 , let me know if you prefer to do it or have a change already.

insop avatar Jan 06 '25 16:01 insop

That would be awesome. Thanks @insop :)

felipemello1 avatar Jan 07 '25 00:01 felipemello1

Closing this now that #2239 has landed

ebsmothers avatar Jan 27 '25 23:01 ebsmothers