ronantakizawa/codereview-bench huggingface dataset:
Repository: tensorflow/models
File: official/utils/testing/integration.py (Python)

Code before:
    extra_flags: Additional flags passed by the the caller of this function.

  Raises:
    OSError: Any error detected in the run causes an OSError. Because the code
      is run as a subprocess, it is not able to manifest the specific python
      error that was raised.
  """

  extra_flags = [] if extra_flags is None else extra_flags

  model_dir = "/tmp/model_test_{}".format(hash(time.time()))
  if os.path.exists(model_dir):
    shutil.rmtree(model_dir)

  args = [PYTHON_CMD, file_path, "--model_dir", model_dir,
          "--train_epochs", "1", "--epochs_per_eval", "1",
          "--use_synthetic_data",
          "--max_train_steps", "1"] + extra_flags

  p = subprocess.Popen(args, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
                       stderr=subprocess.PIPE)
  output, err = p.communicate()
  returncode = p.returncode

  if os.path.exists(model_dir):
    shutil.rmtree(model_dir)

  if returncode != 0:
    raise OSError("Run failed:\nStdout:\n{}\n\nStderr:\n{}".format(
        output.decode("utf-8"), err.decode("utf-8")
    ))


Review comment (suggestion):
I would suggest this to be put into a final block, in case your call to process gets any error and exit early.

Code after:
# Copyright 2018 The TensorFlow Authors. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
# ==============================================================================
"""Helper code to run complete models from within python.
"""

from __future__ import absolute_import
from __future__ import division
from __future__ import print_function


import os
import shutil
import sys
import time


def run_synthetic(main, extra_flags=None):
  """Performs a minimal run of a model.

    This function is intended to test for syntax errors throughout a model. A
  very limited run is performed using synthetic data.

  Args:
    main: The primary function used to excercise a code path. Generally this
      function is "<MODULE>.main(argv)".
    extra_flags: Additional flags passed by the the caller of this function.
  """

  extra_flags = [] if extra_flags is None else extra_flags

  model_dir = "/tmp/model_test_{}".format(hash(time.time()))
  if os.path.exists(model_dir):
    shutil.rmtree(model_dir)

  args = [sys.argv[0], "--model_dir", model_dir, "--train_epochs", "1",
          "--epochs_per_eval", "1", "--use_synthetic_data",
          "--max_train_steps", "1"] + extra_flags

  try:
    main(args)
  finally:
    if os.path.exists(model_dir):
      shutil.rmtree(model_dir)
Repository: tensorflow/models
File: official/resnet/imagenet_main.py (Python)

Code before:
      num_images=_NUM_IMAGES['train'], boundary_epochs=[30, 60, 80, 90],
      decay_rates=[1, 0.1, 0.01, 0.001, 1e-4])

  return resnet_run_loop.resnet_model_fn(features, labels, mode, ImagenetModel,
                                         resnet_size=params['resnet_size'],
                                         weight_decay=1e-4,
                                         learning_rate_fn=learning_rate_fn,
                                         momentum=0.9,
                                         data_format=params['data_format'],
                                         version=params['version'],
                                         loss_filter_fn=None,
                                         multi_gpu=params['multi_gpu'])


def main(unused_argv):
  input_function = FLAGS.use_synthetic_data and get_synth_input_fn() or input_fn
  resnet_run_loop.resnet_main(FLAGS, imagenet_model_fn, input_function)


if __name__ == '__main__':
  tf.logging.set_verbosity(tf.logging.INFO)

  parser = resnet_run_loop.ResnetArgParser(
      resnet_size_choices=[18, 34, 50, 101, 152, 200])
  FLAGS = parser.parse_args()
  tf.app.run(argv=[sys.argv[0]])


Review comment (suggestion):
I think my question is that why the parser need to be in __main__ instead of main() function.

The code in __main__ is only executed when imagenet_main.py is invoked as main target instead of imported as module. For this file, I can hardly foresee it being import as a module for real prod doc, except the case for test. If this assumption is true, then why not push everything here to main(), so that your test code and just import it as module, and avoid calling a new python process.

Code after:
      num_images=_NUM_IMAGES['train'], boundary_epochs=[30, 60, 80, 90],
      decay_rates=[1, 0.1, 0.01, 0.001, 1e-4])

  return resnet_run_loop.resnet_model_fn(features, labels, mode, ImagenetModel,
                                         resnet_size=params['resnet_size'],
                                         weight_decay=1e-4,
                                         learning_rate_fn=learning_rate_fn,
                                         momentum=0.9,
                                         data_format=params['data_format'],
                                         version=params['version'],
                                         loss_filter_fn=None,
                                         multi_gpu=params['multi_gpu'])


def main(argv):
  parser = resnet_run_loop.ResnetArgParser(
      resnet_size_choices=[18, 34, 50, 101, 152, 200])
  flags = parser.parse_args(args=argv[1:])

  input_function = flags.use_synthetic_data and get_synth_input_fn() or input_fn
  resnet_run_loop.resnet_main(flags, imagenet_model_fn, input_function)


if __name__ == '__main__':
  tf.logging.set_verbosity(tf.logging.INFO)
  tf.app.run(argv=sys.argv)
Repository: tensorflow/models
File: research/deeplab/datasets/build_cityscapes_data.py (Python)

Code before:
        # Read the semantic segmentation annotation.
        seg_data = tf.gfile.FastGFile(label_files[i], 'rb').read()
        seg_height, seg_width = label_reader.read_image_dims(seg_data)
        if height != seg_height or width != seg_width:
          raise RuntimeError('Shape mismatched between image and label.')
        # Convert to tf example.
        re_match = _IMAGE_FILENAME_RE.search(image_files[i])
        if re_match is None:
          raise RuntimeError('Invalid image filename: ' + image_files[i])
        filename = os.path.basename(re_match.group(1))
        example = build_data.image_seg_to_tfexample(
            image_data, filename, height, width, seg_data)
        tfrecord_writer.write(example.SerializeToString())
    sys.stdout.write('\n')
    sys.stdout.flush()


def main(unused_argv):
  # Only support converting 'train' and 'val' sets for now.
  for dataset_split in ['train', 'val']:
    _convert_dataset(dataset_split)


if __name__ == '__main__':
  tf.app.run()


Review comment (suggestion):
Let's change it back to

for dataset_split in ['train_fine', 'val_fine']:

It seems that we need to do more changes in order to support test_fine (e.g., skip reading groundtruth).
Let's change it back, since this should be orthogonal to your this pull request.
Sorry about going back and forth.


Code after:
        sys.stdout.write('\r>> Converting image %d/%d shard %d' % (
            i + 1, num_images, shard_id))
        sys.stdout.flush()
        # Read the image.
        image_data = tf.gfile.FastGFile(image_files[i], 'rb').read()
        height, width = image_reader.read_image_dims(image_data)
        # Read the semantic segmentation annotation.
        seg_data = tf.gfile.FastGFile(label_files[i], 'rb').read()
        seg_height, seg_width = label_reader.read_image_dims(seg_data)
        if height != seg_height or width != seg_width:
          raise RuntimeError('Shape mismatched between image and label.')
        # Convert to tf example.
        re_match = _IMAGE_FILENAME_RE.search(image_files[i])
        if re_match is None:
          raise RuntimeError('Invalid image filename: ' + image_files[i])
        filename = os.path.basename(re_match.group(1))
        example = build_data.image_seg_to_tfexample(
            image_data, filename, height, width, seg_data)
        tfrecord_writer.write(example.SerializeToString())
    sys.stdout.write('\n')
    sys.stdout.flush()


def main(unused_argv):
  # Only support converting 'train_fine', 'val_fine' and 'test_fine' sets for now.
  for dataset_split in ['train_fine', 'val_fine', 'test_fine']:
    _convert_dataset(dataset_split)


if __name__ == '__main__':
  tf.app.run()
Repository: tesseract-ocr/tesseract
File: src/api/renderer.h (C/C++)

Code before:
  void AppendPDFObject(const char *data);
  // Create the /Contents object for an entire page.
  char* GetPDFTextObjects(TessBaseAPI* api, double width, double height);
  // Turn an image into a PDF object. Only transcode if we have to.
  static bool imageToPDFObj(Pix* pix, const char* filename, long int objnum,
                          char** pdf_object, long int* pdf_object_size, const int jpg_quality);
};


/**
 * Renders tesseract output into a plain UTF-8 text string
 */
class TESS_API TessUnlvRenderer : public TessResultRenderer {
 public:
  explicit TessUnlvRenderer(const char *outputbase);

 protected:
  virtual bool AddImageHandler(TessBaseAPI* api);
};

/**
 * Renders tesseract output into a plain UTF-8 text string for LSTMBOX
 */
class TESS_API TessLSTMBOXRenderer : public TessResultRenderer {
 public:
  explicit TessLSTMBOXRenderer(const char *outputbase);

 protected:
  virtual bool AddImageHandler(TessBaseAPI* api);
};

/**
 * Renders tesseract output into a plain UTF-8 text string
 */
class TESS_API TessBoxTextRenderer : public TessResultRenderer {
 public:
  explicit TessBoxTextRenderer(const char *outputbase);

 protected:
  virtual bool AddImageHandler(TessBaseAPI* api);
};

#ifndef DISABLED_LEGACY_ENGINE

/**
 * Renders tesseract output into an osd text string
 */
class TESS_API TessOsdRenderer : public TessResultRenderer {
 public:
  explicit TessOsdRenderer(const char* outputbase);


Review comment (suggestion):
```suggestion
  explicit TessLSTMBOXRenderer(const char* outputbase);
```

Code after:
  void AppendPDFObject(const char *data);
  // Create the /Contents object for an entire page.
  char* GetPDFTextObjects(TessBaseAPI* api, double width, double height);
  // Turn an image into a PDF object. Only transcode if we have to.
  static bool imageToPDFObj(Pix* pix, const char* filename, long int objnum,
                          char** pdf_object, long int* pdf_object_size, const int jpg_quality);
};


/**
 * Renders tesseract output into a plain UTF-8 text string
 */
class TESS_API TessUnlvRenderer : public TessResultRenderer {
 public:
  explicit TessUnlvRenderer(const char *outputbase);

 protected:
  virtual bool AddImageHandler(TessBaseAPI* api);
};

/**
 * Renders tesseract output into a plain UTF-8 text string for LSTMBOX
 */
class TESS_API TessLSTMBOXRenderer : public TessResultRenderer {
 public:
  explicit TessLSTMBOXRenderer(const char* outputbase);

 protected:
  virtual bool AddImageHandler(TessBaseAPI* api);
};

/**
 * Renders tesseract output into a plain UTF-8 text string
 */
class TESS_API TessBoxTextRenderer : public TessResultRenderer {
 public:
  explicit TessBoxTextRenderer(const char *outputbase);

 protected:
  virtual bool AddImageHandler(TessBaseAPI* api);
};

#ifndef DISABLED_LEGACY_ENGINE

/**
 * Renders tesseract output into an osd text string
 */
class TESS_API TessOsdRenderer : public TessResultRenderer {
 public:
  explicit TessOsdRenderer(const char* outputbase);
Repository: tesseract-ocr/tesseract
File: src/training/tesstrain_utils.sh (Shell)

Code before:
        echo -e "ERROR: "$*
    else
        echo -e "ERROR: "$* | tee -a ${LOG_FILE}
    fi
    exit 1
}

# Helper function to run a command and append its output to a log. Aborts early
# if the program file is not found.
# Usage: run_command CMD ARG1 ARG2...
run_command() {
    local cmd=$(which $1)
    if [[ -z ${cmd} ]]; then
      for d in api training; do
        cmd=$(which $d/$1)
        if [[ ! -z ${cmd} ]]; then
          break
        fi
      done
      if [[ -z ${cmd} ]]; then
          err_exit "$1 not found"
      fi
    fi
    shift
    tlog "[$(date)] ${cmd} $@"
    if ! "${cmd}" "$@" 2>&1 1>&2 | tee -a ${LOG_FILE}; then
        err_exit "Program $(basename ${cmd}) failed. Abort."
    fi
}

# Check if all the given files exist, or exit otherwise.
# Used to check required input files and produced output files in each phase.
# Usage: check_file_readable FILE1 FILE2...
check_file_readable() {
    for file in $@; do
        if [[ ! -r ${file} ]]; then
            err_exit "${file} does not exist or is not readable"
        fi
    done
}

# Sets the named variable to given value. Aborts if the value is missing or
# if it looks like a flag.
# Usage: parse_value VAR_NAME VALUE
parse_value() {
    local val="${2:-}"
    if [[ -z "$val" ]]; then
        err_exit "Missing value for variable $1"
        exit
    fi
    if [[ ${val:0:2} == "--" ]]; then

Review comment (suggestion):
```suggestion
    if ! "${cmd}" "$@" 2>&1 | tee -a ${LOG_FILE}; then
```

Code after:
        echo -e "ERROR: "$*
    else
        echo -e "ERROR: "$* | tee -a ${LOG_FILE}
    fi
    exit 1
}

# Helper function to run a command and append its output to a log. Aborts early
# if the program file is not found.
# Usage: run_command CMD ARG1 ARG2...
run_command() {
    local cmd
    cmd=$(which $1 || \
              for d in api training; do
                  which $d/$1 && break
              done) || err_exit "'$1' not found"
    shift
    tlog "[$(date)] ${cmd} $@"
    if ! "${cmd}" "$@" |& tee -a ${LOG_FILE}; then
        err_exit "Program $(basename ${cmd}) failed. Abort."
    fi
}

# Check if all the given files exist, or exit otherwise.
# Used to check required input files and produced output files in each phase.
# Usage: check_file_readable FILE1 FILE2...
check_file_readable() {
    for file in $@; do
        if [[ ! -r ${file} ]]; then
            err_exit "${file} does not exist or is not readable"
        fi
    done
}

# Sets the named variable to given value. Aborts if the value is missing or
# if it looks like a flag.
# Usage: parse_value VAR_NAME VALUE
parse_value() {
    local val="${2:-}"
    if [[ -z "$val" ]]; then
        err_exit "Missing value for variable $1"
        exit
    fi
    if [[ ${val:0:2} == "--" ]]; then
        err_exit "Invalid value $val passed for variable $1"
        exit
    fi
    eval $1=\"$val\"
}

# Does simple command-line parsing and initialization.
Repository: tesseract-ocr/tesseract
File: src/arch/simddetect.cpp (C++)

Code before:
      }
#endif
    }
#endif
  }
#else
#error "I don't know how to test for SIMD with this compiler"
#endif
#endif

#ifdef HAVE_NEON
#ifdef ANDROID
  {
    AndroidCpuFamily family = android_getCpuFamily();
    if (family == ANDROID_CPU_FAMILY_ARM)
      neon_available_ = (android_getCpuFeatures() &
                         ANDROID_CPU_ARM_FEATURE_NEON);
#if 0
    /* There is no NEON flag to test on ARM64. A wild guess would have
     * it being the ASIMD flag, but I'll leave this disabled for now. */
    else if (family == ANDROID_CPU_FAMILY_ARM64)
      neon_available_ = (android_getCpuFeatures() &
                         ANDROID_CPU_ARM64_FEATURE_ASIMD);
#endif
    else
      neon_available_ = 0;
  }
#else
  /* Assume linux */
  neon_available_ = getauxval(AT_HWCAP) & HWCAP_NEON;
#endif
#endif

  // Select code for calculation of dot product based on autodetection.
  if (false) {
    // This is a dummy to support conditional compilation.
#if defined(HAVE_AVX2)
  } else if (avx2_available_) {
    // AVX2 detected.
    SetDotProduct(DotProductAVX, &IntSimdMatrix::intSimdMatrixAVX2);
#endif
#if defined(HAVE_AVX)
  } else if (avx_available_) {
    // AVX detected.
    SetDotProduct(DotProductAVX, &IntSimdMatrix::intSimdMatrixSSE);
#endif
#if defined(HAVE_SSE4_1)
  } else if (sse_available_) {
    // SSE detected.
    SetDotProduct(DotProductSSE, &IntSimdMatrix::intSimdMatrixSSE);
#endif

Review comment (suggestion):
```suggestion
      neon_available_ = false;
```

Code after:
      }
#endif
    }
#endif
  }
#else
#error "I don't know how to test for SIMD with this compiler"
#endif
#endif

#ifdef HAVE_NEON
#ifdef ANDROID
  {
    AndroidCpuFamily family = android_getCpuFamily();
    if (family == ANDROID_CPU_FAMILY_ARM)
      neon_available_ = (android_getCpuFeatures() &
                         ANDROID_CPU_ARM_FEATURE_NEON);
#if 0
    /* There is no NEON flag to test on ARM64. A wild guess would have
     * it being the ASIMD flag, but I'll leave this disabled for now. */
    else if (family == ANDROID_CPU_FAMILY_ARM64)
      neon_available_ = (android_getCpuFeatures() &
                         ANDROID_CPU_ARM64_FEATURE_ASIMD);
#endif
    else
      neon_available_ = false;
  }
#else
  /* Assume linux */
  neon_available_ = getauxval(AT_HWCAP) & HWCAP_NEON;
#endif
#endif

  // Select code for calculation of dot product based on autodetection.
  if (false) {
    // This is a dummy to support conditional compilation.
#if defined(HAVE_AVX2)
  } else if (avx2_available_) {
    // AVX2 detected.
    SetDotProduct(DotProductAVX, &IntSimdMatrix::intSimdMatrixAVX2);
#endif
#if defined(HAVE_AVX)
  } else if (avx_available_) {
    // AVX detected.
    SetDotProduct(DotProductAVX, &IntSimdMatrix::intSimdMatrixSSE);
#endif
#if defined(HAVE_SSE4_1)
  } else if (sse_available_) {
    // SSE detected.
    SetDotProduct(DotProductSSE, &IntSimdMatrix::intSimdMatrixSSE);
#endif
Repository: tesseract-ocr/tesseract
File: include/tesseract/genericvector.h (C/C++)

Code before:
    init_to_size(size, init_val);
  }

  // Copy
  GenericVector(const GenericVector& other) {
    this->init(other.size());
    this->operator+=(other);
  }
  GenericVector<T>& operator+=(const GenericVector& other);
  GenericVector<T>& operator=(const GenericVector& other);

  ~GenericVector();

  // Reserve some memory.
  void reserve(int size);
  // Double the size of the internal array.
  void double_the_size();

  // Resizes to size and sets all values to t.
  void init_to_size(int size, const T& t);
  // Resizes to size without any initialization.
  void resize_no_init(int size) {
    reserve(size);
    size_used_ = size;
  }
  void resize_no_init(int size, int rounded_size) {
    reserve(rounded_size);
    size_used_ = size;
  }

  // Return the size used.
  int size() const {
    return size_used_;
  }
  // Workaround to avoid g++ -Wsign-compare warnings.
  size_t unsigned_size() const {
    static_assert(sizeof(size_used_) <= sizeof(size_t),
                  "Wow! sizeof(size_t) < sizeof(int32_t)!!");
    assert(0 <= size_used_);
    return static_cast<size_t>(size_used_);
  }
  int size_reserved() const {
    return size_reserved_;
  }

  // Return true if empty.
  bool empty() const {
    return size_used_ == 0;
  }

  // Return the object from an index.

Review comment (suggestion):
I would like to replace class `GenericVector` by standard classes, but each new function makes that more difficult. Try to avoid changing `genericvector.h`.

Code after:
    init_to_size(size, init_val);
  }

  // Copy
  GenericVector(const GenericVector& other) {
    this->init(other.size());
    this->operator+=(other);
  }
  GenericVector<T>& operator+=(const GenericVector& other);
  GenericVector<T>& operator=(const GenericVector& other);

  ~GenericVector();

  // Reserve some memory.
  void reserve(int size);
  // Double the size of the internal array.
  void double_the_size();

  // Resizes to size and sets all values to t.
  void init_to_size(int size, const T& t);
  // Resizes to size without any initialization.
  void resize_no_init(int size) {
    reserve(size);
    size_used_ = size;
  }

  // Return the size used.
  int size() const {
    return size_used_;
  }
  // Workaround to avoid g++ -Wsign-compare warnings.
  size_t unsigned_size() const {
    static_assert(sizeof(size_used_) <= sizeof(size_t),
                  "Wow! sizeof(size_t) < sizeof(int32_t)!!");
    assert(0 <= size_used_);
    return static_cast<size_t>(size_used_);
  }
  int size_reserved() const {
    return size_reserved_;
  }

  // Return true if empty.
  bool empty() const {
    return size_used_ == 0;
  }

  // Return the object from an index.
  T& get(int index) const;
  T& back() const;
  T& operator[](int index) const;
  // Returns the last object and removes it.
Repository: tesseract-ocr/tesseract
File: src/arch/intsimdmatrix.cpp (C++)

Code before:
      for (int j = 0; j < num_outputs_per_register_set; ++j) {
        int8_t weight = 0;
        if (output + j < num_out) weight = w(output + j, num_in);
        shaped_w[shaped_index++] = weight;
      }
      output += num_outputs_per_register_set;
    }
  }
}

// Computes matrix.vector v = Wu.
// u is of size W.dim2() - 1 and the output v is of size W.dim1().
// u is imagined to have an extra element at the end with value 1, to
// implement the bias, but it doesn't actually have it.
void IntSimdMatrix::MatrixDotVector(const GENERIC_2D_ARRAY<int8_t>& w,
                                    const GenericVector<double>& scales,
                                    const int8_t* u, double* v) {
  int num_out = w.dim1();
  int num_in = w.dim2() - 1;
  // Base implementation.
  for (int i = 0; i < num_out; ++i) {
    const int8_t* wi = w[i];
    int total = 0;
    for (int j = 0; j < num_in; ++j) total += wi[j] * u[j];
    // Add in the bias and correct for integer values.
    v[i] = (static_cast<double>((total) + wi[num_in] * INT8_MAX) * scales[i]);
  }
}

}  // namespace tesseract


Review comment (suggestion):
```suggestion
    v[i] = (total + wi[num_in] * INT8_MAX) * scales[i];
```

Code after:
      for (int j = 0; j < num_outputs_per_register_set; ++j) {
        int8_t weight = 0;
        if (output + j < num_out) weight = w(output + j, num_in);
        shaped_w[shaped_index++] = weight;
      }
      output += num_outputs_per_register_set;
    }
  }
}

// Computes matrix.vector v = Wu.
// u is of size W.dim2() - 1 and the output v is of size W.dim1().
// u is imagined to have an extra element at the end with value 1, to
// implement the bias, but it doesn't actually have it.
void IntSimdMatrix::MatrixDotVector(const GENERIC_2D_ARRAY<int8_t>& w,
                                    const GenericVector<double>& scales,
                                    const int8_t* u, double* v) {
  int num_out = w.dim1();
  int num_in = w.dim2() - 1;
  // Base implementation.
  for (int i = 0; i < num_out; ++i) {
    const int8_t* wi = w[i];
    int total = 0;
    for (int j = 0; j < num_in; ++j) total += wi[j] * u[j];
    // Add in the bias and correct for integer values.
    v[i] = (total + wi[num_in] * INT8_MAX) * scales[i];
  }
}

}  // namespace tesseract