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

Add Traceback to the AutoMLSearch Logs #1840

Merged
merged 19 commits into from Feb 18, 2021
Merged

Add Traceback to the AutoMLSearch Logs #1840

merged 19 commits into from Feb 18, 2021

Conversation

bchen1116
Copy link
Contributor

@bchen1116 bchen1116 commented Feb 12, 2021

fix #1778

Screenshot of the evalml_debug.log file:
image

@codecov
Copy link

codecov bot commented Feb 12, 2021

Codecov Report

Merging #1840 (545b272) into main (ca20843) will increase coverage by 0.2%.
The diff coverage is 100.0%.

Impacted file tree graph

@@           Coverage Diff            @@
##            main    #1840     +/-   ##
========================================
+ Coverage   99.9%   100.0%   +0.2%     
========================================
  Files        255      255             
  Lines      20662    20644     -18     
========================================
+ Hits       20628    20636      +8     
+ Misses        34        8     -26     
Impacted Files Coverage Δ
evalml/automl/callbacks.py 100.0% <100.0%> (ø)
evalml/tests/automl_tests/test_automl.py 100.0% <100.0%> (ø)
...lml/tests/model_understanding_tests/test_graphs.py 100.0% <0.0%> (+0.3%) ⬆️
.../automl_tests/test_automl_search_classification.py 100.0% <0.0%> (+0.4%) ⬆️
evalml/automl/automl_search.py 99.7% <0.0%> (+0.4%) ⬆️
evalml/tests/pipeline_tests/test_pipelines.py 100.0% <0.0%> (+0.5%) ⬆️
evalml/utils/gen_utils.py 99.6% <0.0%> (+0.5%) ⬆️
...understanding_tests/test_permutation_importance.py 100.0% <0.0%> (+0.8%) ⬆️
evalml/tests/component_tests/test_components.py 100.0% <0.0%> (+0.9%) ⬆️
...ests/automl_tests/test_automl_search_regression.py 100.0% <0.0%> (+0.9%) ⬆️
... and 2 more

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 ca20843...545b272. Read the comment docs.

@@ -1848,6 +1848,11 @@ def test_automl_error_callback_none(mock_fit, mock_score, X_y_binary, caplog):
automl = AutoMLSearch(X_train=X, y_train=y, problem_type="binary", error_callback=None, train_best_pipeline=False, n_jobs=1)
automl.search()
assert msg in caplog.text
traceback_string1 = "return self._execute_mock_call(*args, **kwargs)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are messages that exist in the tracebacks only

traceback_string2 = "return self._mock_call(*args, **kwargs)"
traceback_string3 = "cv_pipeline.fit(X_train, y_train)"
for messages in [traceback_string1, traceback_string2, traceback_string3, msg]:
assert messages not in caplog.text
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only silent_error_callback doesn't log the traceback

Copy link
Contributor

Choose a reason for hiding this comment

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

It's confusing though cause there is no traceback since nothing errors. I think we should update the test. Could be done in a separate PR but it'd be a small change.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@bchen1116 @freddyaboulton if an issue is coming out of this, can we also maybe parameterize these tests into one? Ping me if you do it in a separate PR, I'd like to make sure it doesn't get lost in the backlog.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@freddyaboulton @chukarsten simplified the tests, so I don't think we'll need to file that in a new issue.

Copy link
Contributor

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

@bchen1116 Looks good! Thanks for updating all of our existing tests!

I have a comment on adding some info to the traceback in the event of a PipelineScoreError that I'd like to resolve before merge.

evalml/automl/callbacks.py Outdated Show resolved Hide resolved
evalml/automl/callbacks.py Outdated Show resolved Hide resolved
traceback_string2 = "return self._mock_call(*args, **kwargs)"
traceback_string3 = "cv_pipeline.fit(X_train, y_train)"
for messages in [traceback_string1, traceback_string2, traceback_string3, msg]:
assert messages not in caplog.text
Copy link
Contributor

Choose a reason for hiding this comment

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

It's confusing though cause there is no traceback since nothing errors. I think we should update the test. Could be done in a separate PR but it'd be a small change.

traceback_string2 = "return self._mock_call(*args, **kwargs)"
traceback_string3 = "cv_pipeline.fit(X_train, y_train)"
for messages in [traceback_string1, traceback_string2, traceback_string3, msg]:
assert messages not in caplog.text
Copy link
Collaborator

Choose a reason for hiding this comment

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

@bchen1116 @freddyaboulton if an issue is coming out of this, can we also maybe parameterize these tests into one? Ping me if you do it in a separate PR, I'd like to make sure it doesn't get lost in the backlog.

Copy link
Contributor

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

Looks great @bchen1116 !

@bchen1116 bchen1116 merged commit 458fb40 into main Feb 18, 2021
@chukarsten chukarsten mentioned this pull request Feb 23, 2021
@dsherry dsherry mentioned this pull request Mar 10, 2021
@freddyaboulton freddyaboulton deleted the bc_1778_traceback branch May 13, 2022 15:01
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.

No stack trace visible in debug log
3 participants