Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename KerasNLP symbols for a multi-modal future #1803

Merged

Conversation

mattdangerw
Copy link
Member

@mattdangerw mattdangerw commented Aug 28, 2024

This PR (if it's been done right), should not break any existing functionality. It's adds a lot a new aliases for existing symbols, that going forward will be the main way we document them.

  • Classifier -> TextClassifier and XXClassifer -> XXTextClassifier.
  • Add new base classes keras_nlp.models.TextClassifierPrepreprocessor, keras_nlp.models.CausalLMPreprocessor and keras_nlp.models.MaskedLMPreprocessor.

There's no need to update if you don't want to, we will continue to to support the existing usages indefinitely. This is primarily to reduce confusion in our own docs; we want ImageClasssifier and TextClassifier to have obviously distinct usage.

This new base classes also enable a cross-model way of writing task code with split preprocessing.

preprocessor = keras_nlp.models.TextClassifierPrepreprocessor.from_preset(
    "bert_base_en",
)
classifier = keras_nlp.models.TextClassifier(
    "bert_base_en",
    num_classes=2,
)
... run prepreprocessing and training separately

As a follow up, I think we can remove a lot of code by pushing common code onto base classes. But I will do that in a later PR to keep this from getting to big.

This PR (if it's been done right), should not break any existing
functionality. It's adds a lot a new aliases for existing symbols,
that going forward will be the main way we document them.

- `Classifier` -> `TextClassifier` and `XXClassifer` -> `XXTextClassifier`.
- Add new base classes `keras_nlp.models.TextClassifierPrepreprocessor`,
  `keras_nlp.models.CausalLMPreprocessor` and
  `keras_nlp.models.MaskedLMPreprocessor.

There's no need to update if you don't want to, we will continue to
to support the existing usages indefinitely. This is primarily to
reduce confusion in our own docs; we want `ImageClasssifier` and
`TextClassifier` can have obviously distinct usage.

This new base classes enable a cross-model way of writing task code with
split preprocessing.

```
preprocessor = keras_nlp.models.TextClassifierPrepreprocessor.from_preset(
    "bert_base_en",
)
classifier = keras_nlp.models.TextClassifier(
    "bert_base_en",
    num_classes=2,
)
... run prepreprocessing and training separately
```

As a follow up, I think we can remove a lot of code by pushing common
code onto base classes. But I will do that in a later PR to keep this
from getting to big.
Copy link
Member

@SamanehSaadat SamanehSaadat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Matt!

@mattdangerw mattdangerw force-pushed the rename-for-cv-consolidation branch from 6925d03 to 00503ef Compare August 30, 2024 00:39
@mattdangerw mattdangerw force-pushed the rename-for-cv-consolidation branch from 00503ef to f95439c Compare August 30, 2024 17:18
@mattdangerw mattdangerw merged commit 5234a81 into keras-team:master Sep 3, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants