Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Improve inference performance with loaded TransformerChain ML.NET model #371

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Improve inference performance with loaded TransformerChain ML.NET model #371

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7adb78e
Improve performance pf inferencing with ML.NET models loaded into Pip…
najeeb-kazmi Nov 20, 2019
d142639
Handle predictions without PredictedLabel column
najeeb-kazmi Nov 22, 2019
d797de1
Merge branch 'master' into model-loading
najeeb-kazmi Nov 22, 2019
aa10245
Add small model and test
najeeb-kazmi Nov 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions src/python/nimbusml/pipeline.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2073,13 +2073,9 @@ def _predict(self, X, y=None,
isinstance(X, DataFrame) and isinstance(y, (str, tuple))):
y = y_temp

is_transformer_chain = False
with ZipFile(self.model) as model_zip:
is_transformer_chain = any('TransformerChain' in item
for item in model_zip.namelist())

all_nodes = []
if is_transformer_chain:
if (hasattr(self, '_is_transformer_chain') and
self._is_transformer_chain):
inputs = dict([('data', ''), ('transform_model', self.model)])
Copy link
Member

Choose a reason for hiding this comment

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

How much is perf gain? If its not much I would like to leave this as it is. Reason is that we actually should move to new ML.NET format so then this will be broken with your new fix

Copy link
Member Author

Choose a reason for hiding this comment

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

6% gain, with .predict called on 100 row UCI Adult test data, in a 100 rep for loop, repeated 5 times.

I think we should take the change. (1) If we leave it as it is, it will still be broken when NimbusML moves to new ML.NET format, and how the graph is constructed will need to change anyway. (2) Moving to new ML.NET format will likely take a long time, as it requires non-trivial changes to how model loading is handled in the entrypoints infrastructure on ML.NET side i.e. new model implementation in addition to PredictorModel and TransformModel. (3) The changes directly below this address an issue where PredictedLabel column is only converted to int32 from bool if it exists (i.e. it is not regression or ranking) and if the dtype is bool (i.e. if it is Binary classification).

if isinstance(X, FileDataStream):
importtext_node = data_customtextloader(
Expand Down Expand Up @@ -2171,10 +2167,12 @@ def _predict(self, X, y=None,
self._run_time = time.time() - start_time
raise e

if is_transformer_chain:
if (hasattr(self, '_is_transformer_chain')
and self._is_transformer_chain
and hasattr(out_data, 'PredictedLabel')
and out_data.PredictedLabel.dtype == 'bool'):
out_data['PredictedLabel'] = out_data['PredictedLabel']*1


if y is not None:
# We need to fix the schema for ranking metrics
if evaltype == 'ranking':
Expand Down Expand Up @@ -2617,6 +2615,10 @@ def load_model(self, src):
self.model = src
self.steps = []

with ZipFile(self.model) as model_zip:
self._is_transformer_chain = any('TransformerChain' in item
for item in model_zip.namelist())

def __getstate__(self):
odict = {'export_version': 2}

Expand Down