keras-nlp icon indicating copy to clipboard operation
keras-nlp copied to clipboard

Make Decoding Functions Graph-compatible (with XLA Support!)

Open abheesht17 opened this issue 3 years ago • 7 comments

Resolves #241

  • [x] Greedy Search
  • [ ] Beam Search (will probably open a separate PR for this)
  • [x] Top-p Search
  • [x] Top-k Search
  • [x] Random Search

Will have to think a bit more about Beam Search.

abheesht17 avatar Jul 13 '22 18:07 abheesht17

Also re-beam search, separate PR sounds good!

mattdangerw avatar Jul 20 '22 23:07 mattdangerw

/gcbrun

mattdangerw avatar Jul 22 '22 21:07 mattdangerw

I think a pull request went by recently where we stopped doing seeded random generation because of discrepancies.

https://github.com/keras-team/keras-nlp/pull/269

Is this safe to land as is @chenmoneygithub @jessechancy ?

mattdangerw avatar Jul 22 '22 21:07 mattdangerw

Seeded random generation should be removed. This is mainly because even when fully seeded, the randomness output is different on accelerator-testing with GPU.

jessechancy avatar Jul 23 '22 02:07 jessechancy

@chenmoneygithub do you know why the accelerator testing is failing here? This would be a great one to actually test on accelerators.

mattdangerw avatar Aug 11 '22 17:08 mattdangerw

I found it out, it's because the git branch has not synced to master branch, so the build file is outdated.

@abheesht17 Could you sync and push again? Thanks!

chenmoneygithub avatar Aug 11 '22 17:08 chenmoneygithub

I found it out, it's because the git branch has not synced to master branch, so the build file is outdated.

@abheesht17 Could you sync and push again? Thanks!

Sure!

abheesht17 avatar Aug 11 '22 18:08 abheesht17