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.

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

Show interactive iteration vs. score plot when using fit() #134

Merged
merged 38 commits into from Dec 12, 2019

Conversation

christopherbunn
Copy link
Contributor

@christopherbunn christopherbunn commented Oct 16, 2019

Resolves #78
image

@kmax12
Copy link
Contributor

kmax12 commented Oct 17, 2019

@christopherbunn this should show best score by iteration. so it should be either monotonically increasing or decreasing

@christopherbunn
Copy link
Contributor Author

christopherbunn commented Oct 18, 2019

Updated figure in original comment to show current graph

@codecov
Copy link

codecov bot commented Oct 18, 2019

Codecov Report

Merging #134 into master will increase coverage by 0.06%.
The diff coverage is 97.74%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
+ Coverage   97.08%   97.14%   +0.06%     
==========================================
  Files          95       95              
  Lines        2742     2872     +130     
==========================================
+ Hits         2662     2790     +128     
- Misses         80       82       +2
Impacted Files Coverage Δ
evalml/tests/automl_tests/test_autobase.py 100% <ø> (ø) ⬆️
evalml/models/auto_base.py 93.93% <100%> (+0.21%) ⬆️
evalml/tests/automl_tests/test_autoclassifier.py 100% <100%> (ø) ⬆️
...l/tests/automl_tests/test_pipeline_search_plots.py 97.24% <100%> (+0.54%) ⬆️
evalml/tests/automl_tests/test_autoregressor.py 100% <100%> (ø) ⬆️
evalml/models/pipeline_search_plots.py 97.6% <93.18%> (-2.41%) ⬇️
evalml/models/auto_regressor.py 100% <0%> (+9.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f24a55...3eb4736. Read the comment docs.

@@ -127,7 +148,10 @@ def fit(self, X, y, feature_types=None, raise_errors=False):
self.logger.log("\n\nMax time elapsed. Stopping search early.")
break
self._do_iteration(X, y, pbar, raise_errors)

if plot_iterations:
new_score = self.rankings['score'].max()
Copy link
Contributor

@kmax12 kmax12 Oct 21, 2019

Choose a reason for hiding this comment

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

does this work for metrics where lower is better?

Returns:

self
"""
def update_plot(fig, ax, iter_scores):
Copy link
Contributor

@kmax12 kmax12 Oct 21, 2019

Choose a reason for hiding this comment

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

let's make sure that there is a function that can be called to get this information if another program wanted to be able to generate this plot.

Copy link
Contributor

@kmax12 kmax12 Oct 21, 2019

Choose a reason for hiding this comment

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

we should then write a test case for that function in this PR

evalml/models/auto_base.py Outdated Show resolved Hide resolved
@@ -328,6 +342,31 @@ def describe_pipeline(self, pipeline_id, return_dict=False):
if return_dict:
return pipeline_results

def plot_best_score_by_iteration(self, interactive_plot=False):
Copy link
Contributor

@kmax12 kmax12 Nov 5, 2019

Choose a reason for hiding this comment

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

is it possible to also plot a grey dot for what the current iteration scored. essentially have a scatter plot of scores by iteration and a line ontop showing what the best score was by iteration

evalml/models/auto_base.py Outdated Show resolved Hide resolved
jeremyliweishih
jeremyliweishih previously requested changes Dec 6, 2019
Copy link
Contributor

@jeremyliweishih jeremyliweishih left a comment

Maybe we can add tests asserting the graph logic (always decreasing or increasing) or other functional tests but otherwise looks good to me.

evalml/models/auto_base.py Outdated Show resolved Hide resolved
title = 'Pipeline Search: Iteration vs. {}'.format(self.data.objective.name)
data = go.Scatter(x=iter_numbers, y=self.iteration_scores, mode='lines+markers')
layout = dict(title=title, xaxis_title='Iteration', yaxis_title='Score')
self.best_score_by_iter_fig = go.FigureWidget(data, layout)
Copy link
Contributor

@kmax12 kmax12 Dec 6, 2019

Choose a reason for hiding this comment

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

is it possible to make sure x-axis is integers greater than 0? doesnt make sense to have decimals for an iteration or -1 (show -1 on first iteration)

Screen Shot 2019-12-06 at 6 17 06 PM

Copy link
Contributor Author

@christopherbunn christopherbunn Dec 9, 2019

Choose a reason for hiding this comment

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

It's possible to set the plot so that it only shows positive values. However, the initial point ends up being cut off.

During search:
Screen Shot 2019-12-09 at 11 07 50 AM

At the end:
Screen Shot 2019-12-09 at 11 08 10 AM

Compared to the original end plot:
Screen Shot 2019-12-09 at 11 08 51 AM

Is it worth having it cutoff?

title = 'Pipeline Search: Iteration vs. {}'.format(self.data.objective.name)
data = go.Scatter(x=iter_numbers, y=self.iteration_scores, mode='lines+markers')
layout = dict(title=title, xaxis_title='Iteration', yaxis_title='Score')
self.best_score_by_iter_fig = go.FigureWidget(data, layout)
Copy link
Contributor

@kmax12 kmax12 Dec 6, 2019

Choose a reason for hiding this comment

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

what if we had this method return a class SearchIterationPlot

then the caller who creates it could call SearchIterationPlot.update()?

i think this is better because add_iteration_score method is a bit confusing out of context and can really only be called if someone calls best_score_by_iteration first

evalml/models/auto_base.py Outdated Show resolved Hide resolved
def best_score_by_iteration(self):
iter_numbers = list(range(len(self.iteration_scores)))
title = 'Pipeline Search: Iteration vs. {}'.format(self.data.objective.name)
data = go.Scatter(x=iter_numbers, y=self.iteration_scores, mode='lines+markers')
Copy link
Contributor

@kmax12 kmax12 Dec 6, 2019

Choose a reason for hiding this comment

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

is it possible to also add a light grey dot for the score of every iteration overlaid on this plot? we don't need to do it in this PR unless it's quick. probably makes sense to just create a new issue for it.

that can be help for users to see what the performance of each successive attempt was yielding

Copy link
Contributor Author

@christopherbunn christopherbunn Dec 9, 2019

Choose a reason for hiding this comment

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

Added dot to show current position:

image

Copy link
Contributor Author

@christopherbunn christopherbunn Dec 9, 2019

Choose a reason for hiding this comment

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

Updated to show dot for every single iteration:
image

@christopherbunn christopherbunn requested a review from kmax12 Dec 9, 2019
@christopherbunn
Copy link
Contributor Author

christopherbunn commented Dec 10, 2019

@kmax12 It appears that the random plot resizing you were seeing is a chrome specific issue since I was unable to replicate it in safari. I can fix it by turning off the integer-only formatting and manually setting the tick values for every iteration, but then for large runs the axis quickly becomes pretty cluttered.

Screen Shot 2019-12-09 at 6 05 17 PM

while time.time() - start <= self.max_time:
self._do_iteration(X, y, pbar, raise_errors)
self._do_iteration(X, y, pbar, raise_errors, plot)
Copy link
Contributor

@kmax12 kmax12 Dec 10, 2019

Choose a reason for hiding this comment

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

any reason to not just call plot.update after the self._do_iteration call? might be cleaner not to pass

Copy link
Contributor Author

@christopherbunn christopherbunn Dec 10, 2019

Choose a reason for hiding this comment

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

I think I wanted to avoid duplicating this call in fit(), but it's probably better to call it twice than to pass it through _do_iteration.

self.best_score_by_iter_fig.update_layout(showlegend=False)

def update(self):
iter_idx = self.data.rankings['id'].idxmax()
Copy link
Contributor

@kmax12 kmax12 Dec 10, 2019

Choose a reason for hiding this comment

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

is this assuming the id can be used to determine pipeline order? that isn't a safe assumption. we recently updated the structure of the the results to make this info assessable in a reliable way

see #260

Copy link
Contributor Author

@christopherbunn christopherbunn Dec 10, 2019

Choose a reason for hiding this comment

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

Updated to use self.data.results['pipeline_results'] instead

plot
"""

if hasattr(self, 'iter_plot'):
Copy link
Contributor

@kmax12 kmax12 Dec 10, 2019

Choose a reason for hiding this comment

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

i think the logic here might be a bit broken. if you call . search_iteration_plot(interactive_plot=True) twice, the second time you will get the go.Figure instead of the plot returned.

Copy link
Contributor Author

@christopherbunn christopherbunn Dec 10, 2019

Choose a reason for hiding this comment

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

I chose to do it that way so that users can call search.plot.search_iteration_plot() and get back a Plotly figure after the search process is complete. The first time it is called, it returns the SearchIterationPlot() object so that fit() can update it at every iteration.

The other way I thought of doing this is to separate it out so that there was one function called interactive_search_iteration_plot() that sets up the SearchIterationPlot() object, shows the go.FeatureWidget version, and returns the SearchIterationPlot(). search_iteration_plot() then only returns a Plotly figure for after the search process. Would this be a better implementation?

Copy link
Contributor

@kmax12 kmax12 Dec 10, 2019

Choose a reason for hiding this comment

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

hmm, i think i see what you're saying. let's talk live about this briefly tomorrow, so we can wrap this up.

@kmax12
Copy link
Contributor

kmax12 commented Dec 10, 2019

based on the issue with formatting, i think it's fine to go back with what you had before. it's better to have negative numbers than to have formatting issue. we can look into fixing it more later. sorry for the wild good chase.

@christopherbunn
Copy link
Contributor Author

christopherbunn commented Dec 10, 2019

To clarify, the issue is that the odd behavior only comes up when we have the axis set to show integer numbers only. We can still set it to not show negative numbers on the x axis, but the point for iteration 0 would be still cut off slightly.

self.curr_iteration_scores.append(self.data.rankings['score'].iloc[iter_idx])
iter_idx = self.data.results['search_order']
pipeline_res = self.data.results['pipeline_results']
iter_scores = [pipeline_res[i]['score'] for i in range(len(pipeline_res))]
Copy link
Contributor

@kmax12 kmax12 Dec 10, 2019

Choose a reason for hiding this comment

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

to make sure these score in the same order as iter_idx, should you do

iter_scores = [pipeline_res[i]['score'] for i in iter_idx]

plot
"""

if hasattr(self, 'iter_plot'):
Copy link
Contributor

@kmax12 kmax12 Dec 10, 2019

Choose a reason for hiding this comment

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

hmm, i think i see what you're saying. let's talk live about this briefly tomorrow, so we can wrap this up.

@christopherbunn christopherbunn requested review from kmax12 and jeremyliweishih and removed request for jeremyliweishih Dec 12, 2019
kmax12
kmax12 approved these changes Dec 12, 2019
Copy link
Contributor

@kmax12 kmax12 left a comment

LGTM

@christopherbunn christopherbunn dismissed jeremyliweishih’s stale review Dec 12, 2019

Updated with tests to check values

@christopherbunn christopherbunn merged commit ea32c4a into master Dec 12, 2019
@dsherry dsherry deleted the perf-by-iter-plot branch May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add plot for the best performance by iteration count
3 participants