What-If Tool: Add ability to sort PD plots by interestingness #2461
Conversation
|
@tolga-b please review |
| Categorical features are repesented as | ||
| {name: samples:[] interestingness:}. | ||
| """ | ||
| try: |
There was a problem hiding this comment.
Some design related comments:
First, do we need to recompute all PDPs just for sorting although they exist on the js side?
Second, we could probably move this code to js side by unpacking chart data from containers found by featureContainerByName. Do you think its better to have it on python side for future flexibility?
There was a problem hiding this comment.
As discussed, they only exist for a given feature on JS side if the feature has been expanded, so there is no way to access all plots of JS without having python compute all of them.
| this.makeAsyncRequest_(url, | ||
| setEligibleFields, null, 'plot ordering', sortErrorCallback.bind(this)); | ||
| } else { | ||
| urlParams['features'] = this.partialDepPlotEligibleFeatures; |
There was a problem hiding this comment.
nit: Not sure which is faster, but we could pass this to TensorBoard version as well so you don't have to call python get_eligible_features again. Not necessary.
There was a problem hiding this comment.
that part isn't the bottleneck of this operation, the generating of all PD plots for all features is. so i don't think its necessary.
| @@ -119,6 +119,11 @@ var WITView = widgets.DOMWidgetView.extend({ | |||
| this.touch(); | |||
| }); | |||
|
|
|||
| this.view_.addEventListener('sort-eligible-features', (e) => { | |||
There was a problem hiding this comment.
if examples get updated, will e.detail change to trigger sort again? Do we need a counter?
|
Onduty: redirecting the review to team member, davidsoergel, to load balance. |
|
@davidsoergel can you please review? Thanks. |
|
yep, will do today
…On Fri, Sep 13, 2019 at 9:22 AM James Wexler ***@***.***> wrote:
@davidsoergel <https://github.com/davidsoergel> can you please review?
Thanks.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2461?email_source=notifications&email_token=AAKI6NJZXZSHVRI4QK55FKDQJOH23A5CNFSM4IG4C5LKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6U76RQ#issuecomment-531234630>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAKI6NL24VTPILKBCFQ3YP3QJOH23ANCNFSM4IG4C5LA>
.
|
Long-standing feature request to allow the tool to sort all features by how interesting their partial dependence plots are, to help users explore that space in a more directed manner.
Add new backend capability to sort features by how interesting their PD plots are (for a given selected example, or globally for all examples, depending on user selection). This is done by measuring the Y-axis movement in each PD plot for each feature and sorting them by this measure.
Add new button to run this sort, which runs in the background and can take a while. Therefore added a spinner on the button to show it is calculating
Add path in all implementations (TensorBoard, jupyter, and colab) to handle this sort request and return the response
Add UI to indicate features for which the PD plot is completely flat. These features are grayed-out and at the bottom of the sorted list
Added unit tests for new python code
Updated all demos to be able to handle sorting of eligible features in JS for demo purposes
During sorting processing:
After sorting:
Ran demos and used feature. Verified correct sorting.
Ran TensorBoard, colab notebooks, and jupyter notebooks and used feature to verify it works through all code paths.
Ran unit tests of new py code