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.

Sign up for GitHub

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

Jump to bottom

TFMA auto-visualization for TFX components in KFP #3111

Merged
merged 9 commits into from
Feb 19, 2020
Merged

TFMA auto-visualization for TFX components in KFP #3111

Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5dc1c75
TFMA basic: rendering slicing metrics
jingzhang36 Feb 14, 2020
0858039
tfma rendering
jingzhang36 Feb 18, 2020
bc7765b
render
jingzhang36 Feb 18, 2020
6b0e8ac
remove out-of-date todos
jingzhang36 Feb 19, 2020
250cdf4
Merge remote-tracking branch 'origin/master' into tfma
jingzhang36 Feb 19, 2020
03295b4
FE format
jingzhang36 Feb 19, 2020
c928272
Use io.StringIO instead of file
jingzhang36 Feb 19, 2020
e3d38ff
Add comment for context and reference of TFMA widget hack
jingzhang36 Feb 19, 2020
de1a73f
Add TODO
jingzhang36 Feb 19, 2020
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
73 changes: 31 additions & 42 deletions frontend/src/lib/OutputArtifactLoader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,37 @@ export class OutputArtifactLoader {
return buildArtifactViewer(script);
}),
);
const EvaluatorArtifactUris = filterArtifactUrisByType(
'ModelEvaluation',
Copy link
Contributor

Choose a reason for hiding this comment

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

please do a real test via build a container image and edit yaml to put it in a real cluster to see whether it works.
harded coded codes can't be checked without a real test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested on my deployment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

artifactTypes,
artifacts,
);
viewers = viewers.concat(
EvaluatorArtifactUris.map(uri => {
const configFilePath = uri + '/eval_config.json';
const script = [
Bobgy marked this conversation as resolved.
Show resolved Hide resolved
`import json`,
`import tensorflow as tf`,
`import tensorflow_model_analysis as tfma`,
`from ipywidgets.embed import embed_minimal_html`,
`from IPython.core.display import display, HTML`,
`config_file=tf.io.gfile.GFile('${configFilePath}', 'r')`,
`config=json.loads(config_file.read())`,
`featureKeys=list(filter(lambda x: 'featureKeys' in x, config['evalConfig']['slicingSpecs']))`,
Copy link
Contributor

Choose a reason for hiding this comment

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

please move these Python codes to visulization server. please.
You can simply introduce a new enum type for it, cost is not big.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't feel this is necessary, but if cost is not high, we can do so too.
I'm not familiar with visualization server code, @rmgogogo can you provide a reference for @jingzhang36 ?

Copy link
Contributor Author

@jingzhang36 jingzhang36 Feb 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I'll leave a TODO for this. I personally have no preference regarding where the code is place. Just note that the python code is not run on FE even put it here. If we decide to move, we'll move the statistics, schema and the others together.

Copy link
Contributor

Choose a reason for hiding this comment

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

please check the TFDV patch as a reference. The cost is not big. Just need to add a proto enum and a new python file in types/ folder. It would largely help you well format these python codes.

do prefer take a try on that path.

Copy link
Contributor

Choose a reason for hiding this comment

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

TODO works so that we can catch beta.

`columns=[] if len(featureKeys) == 0 else featureKeys[0]['featureKeys']`,
`slicing_spec = tfma.slicer.SingleSliceSpec(columns=columns)`,
`eval_result = tfma.load_eval_result('${uri}')`,
`slicing_metrics_view = tfma.view.render_slicing_metrics(eval_result, slicing_spec=slicing_spec)`,
`embed_minimal_html('tfma_export.html', views=[slicing_metrics_view], title='Slicing Metrics')`,
Copy link
Contributor

Choose a reason for hiding this comment

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

As @rmgogogo mentioned in email, can you adapt as the following:

use io.StringIO instead of 'tfma_export.html'

More info for your reference
https://ipywidgets.readthedocs.io/en/latest/embedding.html#embedding-widgets-in-html-web-pages
(embed_minimal_html method interface) https://github.com/jupyter-widgets/ipywidgets/blob/master/ipywidgets/embed.py#L292

Copy link
Contributor

@Bobgy Bobgy Feb 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Note embed_minimal_html accepts a File like object (in addition to file name) as the first argument, I think io.StringIO can be used instead there too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

`view_html=None`,
`with open('tfma_export.html', 'r') as view: view_html = view.read()`,
`res_html = view_html.replace('dist/embed-amd.js" crossorigin="anonymous"></script>', 'dist/embed-amd.js" crossorigin="anonymous" data-jupyter-widgets-cdn="https://cdn.jsdelivr.net/gh/Bobgy/model-analysis@kfp/tensorflow_model_analysis/notebook/jupyter/js/dist/" crossorigin="anonymous"></script>')`,
`display(HTML(res_html))`,
];
return buildArtifactViewer(script);
}),
);

return Promise.all(viewers);
}

Expand Down Expand Up @@ -546,45 +577,3 @@ async function buildArtifactViewerTfdvStatistics(url: string): Promise<HTMLViewe
type: PlotType.WEB_APP,
};
}

// TODO: add tfma back
// function filterTfmaArtifactsPaths(
// artifactTypes: ArtifactType[],
// artifacts: Artifact[],
// ): string[] {
// const tfmaArtifactTypeIds = artifactTypes
// .filter(artifactType => artifactType.getName() === 'ModelEvaluation')
// .map(artifactType => artifactType.getId());
// const tfmaArtifacts = artifacts.filter(artifact =>
// tfmaArtifactTypeIds.includes(artifact.getTypeId()),
// );

// const tfmaArtifactPaths = tfmaArtifacts.map(artifact => artifact.getUri()).filter(uri => uri); // uri not empty
// return tfmaArtifactPaths;
// }

// async function getTfmaArtifactViewers(
// tfmaArtifactPaths: string[],
// ): Array<Promise<HTMLViewerConfig>> {
// return tfmaArtifactPaths.map(async artifactPath => {
// const script = [
// 'import tensorflow_model_analysis as tfma',
// `tfma_result = tfma.load_eval_result('${artifactPath}')`,
// 'tfma.view.render_slicing_metrics(tfma_result)',
// ];
// const visualizationData: ApiVisualization = {
// arguments: JSON.stringify({ code: script }),
// source: '',
// type: ApiVisualizationType.CUSTOM,
// };
// const visualization = await Apis.buildPythonVisualizationConfig(visualizationData);
// if (!visualization.htmlContent) {
// // TODO: Improve error message with details.
// throw new Error('Failed to build TFMA artifact visualization');
// }
// return {
// htmlContent: visualization.htmlContent,
// type: PlotType.WEB_APP,
// };
// });
// }