add caffe/random_fn lint rule to check for use of rand, rand_r, random
authorJeff Donahue <jeff.donahue@gmail.com>
Wed, 23 Apr 2014 01:00:22 +0000 (18:00 -0700)
committerJeff Donahue <jeff.donahue@gmail.com>
Wed, 23 Apr 2014 06:40:18 +0000 (23:40 -0700)
matlab/caffe/matcaffe.cpp
scripts/cpp_lint.py

index 5059d22..21f51e8 100644 (file)
@@ -272,7 +272,7 @@ static void init(MEX_ARGS) {
   mxFree(param_file);
   mxFree(model_file);
 
-  init_key = random();
+  init_key = random();  // NOLINT(caffe/random_fn)
 
   if (nlhs == 1) {
     plhs[0] = mxCreateDoubleScalar(init_key);
index 6aa1544..76eee4b 100755 (executable)
@@ -154,6 +154,7 @@ _ERROR_CATEGORIES = [
   'build/namespaces',
   'build/printf_format',
   'build/storage_class',
+  'caffe/random_fn',
   'legal/copyright',
   'readability/alt_tokens',
   'readability/braces',
@@ -1560,6 +1561,38 @@ def CheckForMultilineCommentsAndStrings(filename, clean_lines, linenum, error):
           'Use C++11 raw strings or concatenation instead.')
 
 
+c_random_function_list = (
+    'rand(',
+    'rand_r(',
+    'random(',
+    )
+
+def CheckCaffeRandom(filename, clean_lines, linenum, error):
+  """Checks for calls to C random functions (rand, rand_r, random, ...).
+
+  Caffe code should (almost) always use the caffe_rng_* functions rather
+  than these, as the internal state of these C functions is independent of the
+  native Caffe RNG system which should produce deterministic results for a
+  fixed Caffe seed set using Caffe::set_random_seed(...).
+
+  Args:
+    filename: The name of the current file.
+    clean_lines: A CleansedLines instance containing the file.
+    linenum: The number of the line to check.
+    error: The function to call with any errors found.
+  """
+  line = clean_lines.elided[linenum]
+  for function in c_random_function_list:
+    ix = line.find(function)
+    # Comparisons made explicit for clarity -- pylint: disable=g-explicit-bool-comparison
+    if ix >= 0 and (ix == 0 or (not line[ix - 1].isalnum() and
+                                line[ix - 1] not in ('_', '.', '>'))):
+      error(filename, linenum, 'caffe/random_fn', 2,
+            'Use caffe_rng_rand() (or other caffe_rng_* function) instead of '
+            + function +
+            ') to ensure results are deterministic for a fixed Caffe seed.')
+
+
 threading_list = (
     ('asctime(', 'asctime_r('),
     ('ctime(', 'ctime_r('),
@@ -1570,7 +1603,6 @@ threading_list = (
     ('getpwuid(', 'getpwuid_r('),
     ('gmtime(', 'gmtime_r('),
     ('localtime(', 'localtime_r('),
-    ('rand(', 'rand_r('),
     ('strtok(', 'strtok_r('),
     ('ttyname(', 'ttyname_r('),
     )
@@ -4530,6 +4562,7 @@ def ProcessLine(filename, file_extension, clean_lines, line,
   CheckForNonStandardConstructs(filename, clean_lines, line,
                                 nesting_state, error)
   CheckVlogArguments(filename, clean_lines, line, error)
+  CheckCaffeRandom(filename, clean_lines, line, error)
   CheckPosixThreading(filename, clean_lines, line, error)
   CheckInvalidIncrement(filename, clean_lines, line, error)
   CheckMakePairUsesDeduction(filename, clean_lines, line, error)