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

Add string defaults for Bert backbone classes

Open jbischof opened this issue 3 years ago • 2 comments

In #361 we developed unique string ids and used them for intelligent defaults in BertClassifier. We should add defaults to the BertBase, BertLarge, etc classes as well.

  • This should likely be the uncased_en model, which has the widest coverage and utililty
  • Need to change all callsites and examples that create random graphs.
# Now a ValueError
BertBase(vocabulary_size=10000)
# New call
BertBase(vocabulary_size=10000, weights=None)

jbischof avatar Sep 20 '22 22:09 jbischof

I'm not actually too opinionated on wether we add the defaults or not.

There will probably be some models like gpt-2, where having a default is really natural (there is only one option). And other's where it's not it's too magical and implicit. I don't mind weights=None for the random graph, especially because that's probably fairly niche.

The only think I would like to push for is consistency. Either we ask for the explicit checkpoint ID everywhere as a style choice (preprocessing, backbones, and classifier), or we provide a reasonable default everywhere. After https://github.com/keras-team/keras-nlp/pull/361 it seems to me like we are straddling, so we should choose one or the other.

mattdangerw avatar Sep 20 '22 22:09 mattdangerw

Let's have a default everywhere. Just splitting this off into a separate issue since it will have a big delta.

jbischof avatar Sep 20 '22 22:09 jbischof

Deprecated by #387

jbischof avatar Oct 20 '22 23:10 jbischof