-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Add support for returning classifier scores in transformers output #8512
Add support for returning classifier scores in transformers output #8512
Conversation
Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>
{0: "POSITIVE"}, | ||
{0: "POSITIVE"}, | ||
] | ||
assert len(values.to_dict()) == 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all of these validations are being moved to structural relationships since scores are somewhat non-deterministic depending on the model used.
Documentation preview for 4bca13a will be available here when this CircleCI job completes successfully. More info
|
mlflow/transformers.py
Outdated
for entry in data: | ||
for label, score in zip(entry["labels"], entry["scores"]): | ||
flattened_data.append( | ||
{"sequence": entry["sequence"], "labels": label, "score": score} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{"sequence": entry["sequence"], "labels": label, "score": score} | |
{"sequence": entry["sequence"], "label": label, "score": score} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used these keys in order to match exactly what the return dict naming formats are for the transformers pipelines. I think it might be a little confusing to users that if they call the pipeline directly they get a response that says "labels" but if they use it in serving it says "label". This was just to make it consistent with the transformers package implementation :)
outputs=Schema( | ||
[ | ||
ColSpec("string", name="sequence"), | ||
ColSpec("string", name="labels"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ColSpec("string", name="labels"), | |
ColSpec("string", name="label"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above (purely for consistency with the transformers API)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after addressing comments
mlflow/transformers.py
Outdated
{'sequence': {0: 'My dog loves to eat spaghetti', | ||
1: 'My dog loves to eat spaghetti', | ||
2: 'My dog hates going to the vet', | ||
3: 'My dog hates going to the vet'}, | ||
'label': {0: 'happy', 1: 'sad', 2: 'sad', 3: 'happy'}, | ||
'score': {0: 0.9896970987319946, | ||
1: 0.010302911512553692, | ||
2: 0.957074761390686, | ||
3: 0.042925238609313965}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we replace this with what the dataframe looks like because this function returns a dataframe. It's difficult to guess what this functions returns from the converted dictionary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great point. Changing to the .to_string()
output format
mlflow/transformers.py
Outdated
# interim_output = self._parse_lists_of_dict_to_list_of_str(raw_output, output_key) | ||
# output = self._parse_list_output_for_multiple_candidate_pipelines(interim_output) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# interim_output = self._parse_lists_of_dict_to_list_of_str(raw_output, output_key) | |
# output = self._parse_list_output_for_multiple_candidate_pipelines(interim_output) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TY for the catch :)
"outputs": '[{"type": "string"}]', | ||
"outputs": '[{"name": "sequence", "type": "string"}, {"name": "labels", ' | ||
'"type": "string"}, {"name": "score", "type": "double"}]', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use dict instead of string here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to match the output of ModelSignature.to_dict()
. The alternative might be to construct the ModelSignature instance results direclty to a dict instead of a JSON encoded representation, but that will probably make this test far more complicated to follow.
docs/source/models.rst
Outdated
@@ -2425,12 +2425,12 @@ Pipeline Type Input Type Output Type | |||
Instructional Text Generation str or List[str] str or List[str] | |||
Conversational str or List[str] str or List[str] | |||
Summarization str or List[str] str or List[str] | |||
Text Classification str or List[str] str or List[str] | |||
Text Classification str or List[str] pd.DataFrame |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to include the dataframe schema like this?
pd.DataFrame (dtypes: {foo: int, bar: float})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great idea :)
mlflow/transformers.py
Outdated
@@ -1800,6 +1807,52 @@ def _coerce_exploded_dict_to_single_dict(self, data): | |||
else: | |||
return data | |||
|
|||
def _parse_zero_shot_text_classifier_output_to_df(self, data): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def _parse_zero_shot_text_classifier_output_to_df(self, data): | |
def _{ flatten | expand }_zero_shot_text_classifier_output(self, data): |
Nit: I'd use flatten
or expand
here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed. flatten
is more appropriate. Changed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments, looks good to me once they are addressed :)
Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>
Related Issues/PRs
#xxxWhat changes are proposed in this pull request?
Add support for returning score attributes for label classification in text-based LLM classification pipelines for transformers
How is this patch tested?
Validation that serving works correctly for ZeroShotClassificationPipelines and TextClassificationPipelines. Existing unit tests and integration tests have been updated to conform to the updated return types.
Does this PR change the documentation?
Release Notes
Is this a user-facing change?
Added return scores for text-based classification pipelines in transformers
What component(s), interfaces, languages, and integrations does this PR affect?
Components
area/artifacts
: Artifact stores and artifact loggingarea/build
: Build and test infrastructure for MLflowarea/docs
: MLflow documentation pagesarea/examples
: Example codearea/model-registry
: Model Registry service, APIs, and the fluent client calls for Model Registryarea/models
: MLmodel format, model serialization/deserialization, flavorsarea/recipes
: Recipes, Recipe APIs, Recipe configs, Recipe Templatesarea/projects
: MLproject format, project running backendsarea/scoring
: MLflow Model server, model deployment tools, Spark UDFsarea/server-infra
: MLflow Tracking server backendarea/tracking
: Tracking Service, tracking client APIs, autologgingInterface
area/uiux
: Front-end, user experience, plotting, JavaScript, JavaScript dev serverarea/docker
: Docker use across MLflow's components, such as MLflow Projects and MLflow Modelsarea/sqlalchemy
: Use of SQLAlchemy in the Tracking Service or Model Registryarea/windows
: Windows supportLanguage
language/r
: R APIs and clientslanguage/java
: Java APIs and clientslanguage/new
: Proposals for new client languagesIntegrations
integrations/azure
: Azure and Azure ML integrationsintegrations/sagemaker
: SageMaker integrationsintegrations/databricks
: Databricks integrationsHow should the PR be classified in the release notes? Choose one:
rn/breaking-change
- The PR will be mentioned in the "Breaking Changes" sectionrn/none
- No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" sectionrn/feature
- A new user-facing feature worth mentioning in the release notesrn/bug-fix
- A user-facing bug fix worth mentioning in the release notesrn/documentation
- A user-facing documentation change worth mentioning in the release notes