use nd for accuracy calculation

[szha]

Description

use ndarray for accuracy calculation.

Checklist

Essentials

• Passed code style checking (make lint)
• Changes are complete (i.e. I finished coding on this PR)
• All changes have test coverage:
• Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
• Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
• Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
• Code is well-documented:
• For user-facing API changes, API doc string has been updated.
• For new C++ functions in header files, their functionalities and arguments are documented.
• For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
• To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

• use ndarray instead of numpy for metric.Accuracy that fixes ACCURACY IS USING NUMPY, URGENT FIX #9571
• Improve accuracy doc that fixes Misleading calculation of mxnet.metric.Accuracy #9582

Comments

• Backward compatible.

[zhreshold]

int64is better?

[szha]

This requires larger space, which can show when the prediction class is large (such as in NLP applications). Should I make it an option?

[zhreshold]

should be good in most cases.

[piiswrong]

This is not as simple as changing numpy array to ndarray. See #7995
There are some cases where numpy array is faster. Please check performance against all cases.

Also flatten reshapes to 2 dimensions. This could cause problems when output is 1 dimensional. Use reshape((-1,))

[szha]

Thanks for the comment. I will switch to use reshape.

Regarding performance, that change was made before the CPU kernel optimization work. Now we should have better performance. Thanks to that, I'm not sure now whether it's worth investing time and code complexity on this now. Also, I'm not entirely convinced that the frontend code should do the job of the backend such as selecting implementation, just for the sake of performance. What do you think?

That said, if there's immediate performance hit due to switching to ND, I'm open to switching back to numpy. Given that it would be infeasible for me to check for all cases, is there any specific observation or reasons from your side that requires attention on its performance?

[piiswrong]

Since this is a performance improvement, please verify that it indeed improve performance, at least for common cases.

Please verify this change does not bring back the negative performance impact reported by #7995.
Please also verify that this change does improve performance for the case @zackchase reported

[ptrendx]

@szha Did you do the performance experiments that @piiswrong asked you about (and what were the results)? With this commit we see 20% perf regression on 8 Voltas in resnet.

[szha]

We intend to start tracking the performance of metrics going forward. See 9705 for some numbers. The acc implementation has been updated after this commit. Does the regression still exist on master head?

[DickJC123]

We saw the perf regression on a build that includes this PR and also ed823b2 "proper flatten in acc (#9619)."

[ThomasDelteil]

asscalar is equivalent to : asnumpy()[0]

This PR did not solve the problem of the metric being computed in the numpy world

[szha]

Computation happens before asnumpy() happens, so nothing is happening in the numpy world other than passing out a scalar value.

May I ask what your interest is in this PR? Do you have a use case that benefits from using ndarray for metric?

[ThomasDelteil]

Maybe my understanding is flawed but in order to be able to bring back this value on the CPU to self.sum_metric, we will need to do a wait_on_read on the pred_label ?
If we have a loop that has:

for batch in data:
   ...
   metric.update()

we will wait for the label_pred to be computed before we load the next batch on the GPU.

Whilst if we had self.sum_metric = nd.zeros(1, ctx=ctx)
self.sum_metric += (pred_label.flatten() == label.flatten()).sum()
then we wouldn't be blocking on this operation before loading the next batch of data and it would just enqueue more operations on the back-end.

I have run several experiments where having the loss computed .asscalar() on every loop is slowing the training by 2-25%.

see:
ilkarman/DeepLearningFrameworks#55
zackchase/mxnet-the-straight-dope#455

[ThomasDelteil]

@szha to precise what I mean, I don't think using numpy is slower in itself, it is just that having a blocking operation in the loop is limiting the use of the parallelization happening in the backend and lead to GPU starvation.

[sxjscience]

However, completely using the non-blocking logic will cause some other problems. To be more specific, the allocated NDArrays cannot be reused and will finally cause an OOM. We should use asscalar() to avoid this.

[ThomasDelteil]

Thanks for the precision @sxjscience , but why is it so? Once processed in the computation graph aren't they garbage collected?

[sxjscience]

I think it's because the OPs are pushed in a much faster speed than the real computation. The graph will keep expanding and the allocation OPs will be executed to allocate new space (Even before the actual computation is performed). We have to call a blocking operator at some point to make sure that the current calculation in the graph has been completed. CC @piiswrong for this.

[ThomasDelteil]

Thanks yes that's my understanding. However I think it should be left to the user to decide when to block, since it depends highly on their GPU and model size (like every 100 batches or every epoch). Also is there a reason why the accuracy is stored on the CPU rather than on specific context? My measures showed great improvements when storing the accuracy on GPU. Maybe if you don't mind we can continue the discussion there: #9571

[sxjscience]

Sure. Let’s move the discussion there. Get Outlook for iOS<https://aka.ms/o0ukef>

________________________________ From: ThomasDelteil <[email protected]> Sent: Monday, March 19, 2018 4:17:58 PM To: apache/incubator-mxnet Cc: Xingjian SHI; Mention Subject: Re: [apache/incubator-mxnet] use nd for accuracy calculation (#9583) @ThomasDelteil commented on this pull request.

________________________________ In python/mxnet/metric.py<#9583 (comment)>:

check_label_shapes(label, pred_label) - self.sum_metric += (pred_label.flat == label.flat).sum() - self.num_inst += len(pred_label.flat) + if pred_label.context != label.context: + pred_label = pred_label.as_in_context(label.context) + + self.sum_metric += (pred_label.flatten() == label.flatten()).sum().asscalar() Thanks yes that's my understanding. However I think it should be left to the user to decide when to block, since it depends highly on their GPU and model size (like every 100 batches or every epoch). Also is there a reason why the accuracy is stored on the CPU rather than on specific context? My measures showed great improvements when storing the accuracy on GPU. Maybe if you don't mind we can continue the discussion there: #9571<#9571> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#9583 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AE8D7uxiHGeRbYA_vJVn5ffFHvCLZn-5ks5tgDymgaJpZM4RvD6W>.