Cancellation in Image Classification (fixes #4632) #4650
Conversation
|
I don't know if I should also add a Also, if anyone has other opinions as to where to put more checkpoints, please, let me know! |
| @@ -992,6 +995,7 @@ public Tensor ProcessImage(in VBuffer<byte> imageBuffer) | |||
|
|
|||
| for (int epoch = 0; epoch < epochs; epoch += 1) | |||
| { | |||
| Host.CheckAlive(); | |||
There was a problem hiding this comment.
Host.CheckAlive(); [](start = 20, length = 18)
I would just put the check in this loop and in the CreateFeaturizedCacheFile. Please also report numbers in perf differences before and after. Please remove CheckAlive from everywhere else as its not very significant and only pollutes the code. You also need to call TryCleanupTemporaryWorkspace for a graceful termination. #Closed
There was a problem hiding this comment.
I have added a new method "CheckAlive" to the ImageClassification trainer, with a try...catch to call TryCleanupTemporaryWorkspace when it's needed.
Also changed the places where I added the checkpoints.
I will see how to get the perf difference now. #Closed
There was a problem hiding this comment.
So I ran the ImageClassificationBench.TrainResnetV250 benchmark, with and without the changes of this PR, and they both behaved in pretty much the same way.
Without the changes this was the summary output of the benchmark:
Method | Mean | Error | StdDev | Extra Metric |
---------------- |--------:|--------:|---------:|-------------:|
TrainResnetV250 | 41.55 s | 5.580 s | 0.3058 s | - |
And with the changes, the summary was:
Method | Mean | Error | StdDev | Extra Metric |
---------------- |--------:|--------:|---------:|-------------:|
TrainResnetV250 | 40.10 s | 2.723 s | 0.1493 s | - |
So on average the version with the changes was reported to ran faster.
In any case, the CheckAlive() method is simply doing if-statements evaluations, so I don't think it can introduce meaningful performance difference (given that image classification training is a task expected to take a considerable amount of time anyway). #Closed
There was a problem hiding this comment.
So, as suggested online by @codemzs I have reran the benchmarks, but using the CIFAR-10 dataset.
Without the changes introduced in the PR the summary is as follows:
Method | Mean | Error | StdDev | Extra Metric |
---------------- |--------:|--------:|---------:|-------------:|
TrainResnetV250 | 79.29 m | 4.850 m | 0.2658 m | - |
With the changes:
Method | Mean | Error | StdDev | Extra Metric |
---------------- |--------:|--------:|--------:|-------------:|
TrainResnetV250 | 78.82 m | 21.71 m | 1.190 m | - |
So, again, my understanding is that there's some variability in the time it takes to train this model (and that's why the benchmark with the changes ran a little bit faster), and the introduction of the CheckAlive() method doesn't really have an impact on the performance of this. #Closed
|
CacheFeaturizedImagesToDisk can take significant time, we must add there. In reply to: 573918512 [](ancestors = 573918512) |
ghost
locked as resolved and limited conversation to collaborators
Adds support for cancellation to the Image Classification trainer in a similar manner as done in #3062 (and other PRs) by adding cancellation checkpoints to the train method.
I've tested it by running the sample related to this trainer. Since the other PR's that included checkpoints for cancellation don't include unit tests, I also didn't include any in here.
Fixes #4632 .