-
Notifications
You must be signed in to change notification settings - Fork 1.9k
AutoML aggregate exception #5631
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
Changes from 1 commit
c495714
55f3164
b5324e8
97e23a2
04a7943
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -196,6 +196,22 @@ public IList<TRunDetail> Execute() | |
"was reached, and the running MLContext was stopped. Details: {0}", e.Message); | ||
return iterationResults; | ||
} | ||
catch (AggregateException e) | ||
{ | ||
// This exception is thrown when the IHost/MLContext of the trainer is canceled due to | ||
// reaching maximum experiment time. Simply catch this exception and return finished | ||
// iteration results. For some trainers, Like FastTree, because training is done in parallel | ||
// in can throw multiple OperationCancelledExceptions. This causes them to be returned as an | ||
// AggregateException and misses the first catch block. This is to handle that case. | ||
if (e.InnerExceptions.All( exception => exception is OperationCanceledException)) | ||
michaelgsharp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
_logger.Warning("OperationCanceledException has been caught after maximum experiment time" + | ||
michaelgsharp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"was reached, and the running MLContext was stopped. Details: {0}", e.Message); | ||
return iterationResults; | ||
} | ||
|
||
Console.WriteLine(e.Message); | ||
michaelgsharp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} while (_history.Count < _experimentSettings.MaxModels && | ||
!_experimentSettings.CancellationToken.IsCancellationRequested && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
To add a unit test focusing on var experimentSettings = new RegressionExperimentSettings { MaxExperimentTimeInSeconds = 1 };
experimentSettings.Trainers.Clear();
experimentSettings.Trainers.Add(RegressionTrainer.FastTree); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have tried that, but it only throws this exception in 1 specific method and this wasn't a stable repro. Not stable enough to try and have a consistent test for it anyways. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. After discussing, even restricting to only FastTree will not make for a stable unit test. If the timeout occurs in certain steps of FastTree, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you force it with a test-specific Trainer, or set of trainers? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried that. The problem is that there is only 1 specific place that causes this issue when a timeout occurs, and I haven't found a way to get it to repro consistently without modifying the method to always throw there. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking more along the lines of a test-only mock trainer. Ignore if we don't already have a framework / pattern for this, that'd be more of a future-looking test strategy piece of feedback. |
||
!_experimentTimerExpired); | ||
michaelgsharp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
Uh oh!
There was an error while loading. Please reload this page.