New rtc dump analyzing tool in Python

tools/py_event_log_analyzer/misc.py

+#  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
+#
+#  Use of this source code is governed by a BSD-style license
+#  that can be found in the LICENSE file in the root of the source
+#  tree. An additional intellectual property rights grant can be found
+#  in the file PATENTS.  All contributing project authors may
+#  be found in the AUTHORS file in the root of the source tree.
+
+"""Utility functions for statistics calculations.

[ivoc, 2016/05/24 11:50:17]

I would reorder this into: Utility functions for calculating statistics.

[aleloi, 2016/05/24 15:54:40]

Done.

+"""
+
+
+import collections
+
+
+def count_reordered(seq_no_s):

[ivoc, 2016/05/24 11:50:18]

please rename seq_no_s to sequence_numbers

[kwiberg-webrtc, 2016/05/24 12:09:51]

A short comment that describes what this function does? And all the other
functions...

[aleloi, 2016/05/24 15:54:40]

Done.

[aleloi, 2016/05/24 15:54:40]

A little better now, I think

+  largest = seq_no_s[0]
+  result = 0
+  for seq_no in seq_no_s:
+    if seq_no < largest:
+      result += 1
+    largest = max(largest, seq_no)

[kwiberg-webrtc, 2016/05/24 12:09:51]

Nit: "else: largest = seq_no" instead of this statement.

[aleloi, 2016/05/24 15:54:40]

Done.

+  return result
+
+
+def ssrc_size_table(data_points):
+  d = collections.defaultdict(int)
+  for dt in data_points:
+    d[dt.ssrc] += dt.size
+  total = sum(d.values())

[kwiberg-webrtc, 2016/05/24 12:09:51]

float(sum(d.values())) here, so that you don't have to call float() in each
iteration of the loop.

[aleloi, 2016/05/24 15:54:40]

removed "float" and added "from __future__ import division" on top instead

+  for key in d:
+    d[key] /= float(total)
+  return d
+
+
+def percent_table(data):
+  d = collections.defaultdict(int)
+  for dt in data:
+    d[dt] += 1

[ivoc, 2016/05/24 11:50:17]

These 3 lines can be replaced by: d = collections.Counter(data)

[aleloi, 2016/05/24 15:54:40]

Thank you!

+  total = sum(d.values())
+  for key in d:
+    d[key] /= float(total)
+  return d
+
+
+def hists(data, n_bins):
+  """Generates a histogram of `data`.
+
+  The sequence of numbers `data` is divided into `n_bins` many
+  bins with even proportion of data points in each.

[ivoc, 2016/05/24 11:50:18]

Is there a difference between this and numpy.histogram?

[kwiberg-webrtc, 2016/05/24 12:09:50]

Not sure I understand. The intervals will have different sizes, to make the
proportion of data points in each bin as close to 1/n_bins as possible?

[aleloi, 2016/05/24 15:54:40]

I got about the same result with numpy.histogram. hists is deleted. Thank you
for the pointer!

+
+  Args:
+      data: a sequence of numbers
+      n_bins: the number of histogram bins into which data is divided.
+  Returns:
+      A dict mapping from histogram bin bounds to proportions. Dictionary
+      keys are pairs of numbers. If `d` is the return value, and `d`
+      contains an interval (10, 15), d[(10, 15)] is the proportion of
+      values in data inside this

[ivoc, 2016/05/24 11:50:17]

Please add a comment on the boundary values (included in the range or not?)

[kwiberg-webrtc, 2016/05/24 12:09:50]

Are the intervals open, closed, or something else?

[aleloi, 2016/05/24 15:54:40]

not longer relevant

+  """
+  cumulative = 0
+  start = sorted(data)[0]-1
+  result = {}
+  for i in sorted(data):

[kwiberg-webrtc, 2016/05/24 12:09:50]

i is a bad name for a variable that's not an index. Or is data really supposed
to be a sequence of indexes?

[kwiberg-webrtc, 2016/05/24 12:09:51]

You sort data twice.

[aleloi, 2016/05/24 15:54:40]

here too

+    cumulative += data[i]

[kwiberg-webrtc, 2016/05/24 12:09:51]

You add data[i] here rather than 1, using the elements of data as indexes into
data.

[aleloi, 2016/05/24 15:54:40]

here too

+    if cumulative >= 1/float(n_bins):

[ivoc, 2016/05/24 11:50:17]

Is this correct? What if the list contains only very large numbers? 

[kwiberg-webrtc, 2016/05/24 12:09:50]

cumulative and n_bins are both integers, right, with n_bins > 0? If so, this
condition is equivalent to cumulative > 0.

[aleloi, 2016/05/24 15:54:40]

here too

+      result[(start, i)] = cumulative
+      start = i+1

[kwiberg-webrtc, 2016/05/24 12:09:50]

You surround binary operators with spaces almost but not quite everywhere.

[aleloi, 2016/05/24 15:54:40]

here too

+      cumulative = 0
+  if cumulative != 0:
+    result[(start, len(data)-1)] = cumulative
+  return result
+
+
+def unwrap(data, mod):
+  """Unwraps `data` modulo `mod`.
+
+  If data contains consecutive elements with difference above `mod/2`,
+  an integer multiple of `mod` is added to one of the elements.

[ivoc, 2016/05/24 11:50:18]

By reading the code below it looks like the 'integer multiple' is at most one
and it is always added to the latter of the consecutive elements. Perhaps that
should be in the description?

[kwiberg-webrtc, 2016/05/24 12:09:50]

I think this is under-specified. What integer multiple, and to which element?

[aleloi, 2016/05/24 15:54:40]

unwrap should work like matlab's unwrap
(http://se.mathworks.com/help/matlab/ref/unwrap.html), but for integers. I added
more description, but it is still a little unclear.

+
+  """
+  prev = data[0]
+  for idxm1, curr in enumerate(data[1:]):
+    idx = idxm1 + 1
+    delta_pos = (curr-prev)%mod
+    delta_neg = (curr-prev)%mod - mod
+    delta = delta_pos if abs(delta_pos) < abs(delta_neg) else delta_neg
+    curr = prev + delta
+    data[idx] = curr
+    prev = curr
+  return data