TFMA auto-visualization for TFX components in KFP #3111
Conversation
There was a problem hiding this comment.
@jingzhang36 Thanks for the quick change!
There's a minor issue
| `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')`, |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
|
Part of #2283 |
| `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']))`, |
There was a problem hiding this comment.
please move these Python codes to visulization server. please.
You can simply introduce a new enum type for it, cost is not big.
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
TODO works so that we can catch beta.
|
I was thinking about moving my forked branch to gs://ml-pipeline, but I just felt it's fine as a temporary hack. I will try fix the infinite redirection issue instead to improve experience for this. |
|
Thanks @jingzhang36! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Bobgy The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| @@ -311,6 +311,41 @@ export class OutputArtifactLoader { | |||
| return buildArtifactViewer(script); | |||
| }), | |||
| ); | |||
| const EvaluatorArtifactUris = filterArtifactUrisByType( | |||
| 'ModelEvaluation', | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Tested on my deployment
|
/unhold |
|
/hold cancel |
|
/test kubeflow-pipeline-sample-test |
|
/test kubeflow-pipeline-e2e-test |
|
/retest |
* TFMA basic: rendering slicing metrics * tfma rendering * render * remove out-of-date todos * FE format * Use io.StringIO instead of file * Add comment for context and reference of TFMA widget hack * Add TODO
We add TFMA auto-visualization for TFX components in KFP.
Note: Widgets in TFMA visualization depend on tfma_widget_js file, which unfortunately doesn't work outside Notebook (as of now). So this PR depends on the work of @Bobgy https://cdn.jsdelivr.net/gh/Bobgy/model-analysis@kfp/tensorflow_model_analysis/notebook/jupyter/js/dist/, which provides a bundle of js files that can support TFMA widgets to work in KFP context.
This change is