transformers icon indicating copy to clipboard operation
transformers copied to clipboard

TF port of the Segment Anything Model (SAM)

Open Rocketknight1 opened this issue 2 years ago • 1 comments

This is a first draft of the SAM port - will update this PR as I port tests and make sure everything is working okay. It's also a first proof-of-concept for full GPT-4 auto-translation from PyTorch: The entire modeling_tf_sam.py file was converted from PyTorch by GPT-4 with the exception of the imports at the top, because I haven't written a prompt for those yet.

Update: I checked over all of the code and fixed the issues in the GPT port. Equivalence tests all look good! This is almost ready to merge, but there are a few small issues left:

  • [ ] Get saved model creation working and re-enable tests (problem with the serving signature)
  • [ ] Check for duplication in the processor files - I can probably refactor and simplify things a bit
  • [ ] Refactor convolutions - channels_first doesn't actually work on CPU in TF

Rocketknight1 avatar Apr 24 '23 17:04 Rocketknight1

The documentation is not available anymore as the PR was closed or merged.

This is now almost ready to go and the code should be ready for review! Remaining issues:

  • I added a tol parameter to the TF-PT equivalence test - 1e-5 is too low for SAM (errors are more like 1e-4, but I used 5e-4 in the test to avoid flakiness). This will require a couple of minor tweaks in other models that are calling that test.
  • Cleanup/refactor in the processor, there's probably some code duplication that I can remove.

Rocketknight1 avatar May 02 '23 15:05 Rocketknight1

Thanks for the review - about half of the comments relate to the processor code, which is definitely in need of a refactor, yes. Working on that now!

Rocketknight1 avatar May 03 '23 13:05 Rocketknight1

@amyeroberts @sgugger I refactored all the changes to the common tests, and just overrode check_pt_tf_outputs to change the tol in the tests instead - this is much cleaner and resolves most of the issues there. I also refactored the processor, removing the duplicated files and merging methods where appropriate. I think all comments have now been addressed!

Rocketknight1 avatar May 04 '23 18:05 Rocketknight1

@gante I think all comments are now addressed, and I added training wherever it touched a layer that had training-specific behaviour (which is literally one dropout call)

All comments from @amyeroberts and @sgugger should be addressed too - are you okay with going ahead and merging now once tests pass?

Rocketknight1 avatar May 11 '23 17:05 Rocketknight1

I think comments are addressed now - are we okay to merge?

Rocketknight1 avatar May 17 '23 12:05 Rocketknight1

I'm treating silence as agreement, merging!

Rocketknight1 avatar May 19 '23 13:05 Rocketknight1