Change LC_NUMERIC for CentOS CI jobs to verify locale invariance (#18097

)

* Load the user's locale before performing tests

* Change the locale for the CentOS CI jobs to weed out locale-related bugs

* Mark tests that fail due to the decimal point with xfail

* Run localedef when generating the CentOS CI image

* Cancel some Scala tests when C locale uses a non-standard decimal sep.

* Rename xfail helper to xfail_when_nonstandard_decimal_separator

* Fix scalastyle errors

* Disable more Python tests that fail due to locale-related issues

* Move assumeStandardDecimalSeparator into separate object to fix scaladoc

* Disable the "symbol pow" test when running with non-standard decimal sep

* Disable new tests that fail due to locale-related issues

ci/docker/Dockerfile.build.centos7

@@ -106,11 +106,18 @@ RUN pip3 install --no-cache-dir --upgrade pip && \
     protobuf==3.5.2 \
     tabulate==0.7.5
 
+# Fix the en_DK.UTF-8 locale to test locale invariance
+RUN localedef -i en_DK -f UTF-8 en_DK.UTF-8
+
 ARG USER_ID=0
 COPY install/docker_filepermissions.sh /work/
 RUN /work/docker_filepermissions.sh
 
 ENV PYTHONPATH=./python/
+# Verify that MXNet works correctly when the C locale is set to a locale that uses a comma as the
+# decimal separator. Please see #16134 for an example of a bug caused by incorrect handling of
+# number serialization and deserialization.
+ENV LC_NUMERIC=en_DK.UTF-8
 WORKDIR /work/mxnet
 
 COPY runtime_functions.sh /work/

ci/docker/runtime_functions.sh

@@ -1678,7 +1678,7 @@ build_scala_docs() {
 
     pushd $scala_path
 
-    scala_doc_sources=`find . -type f -name "*.scala" | egrep "./core|./infer" | egrep -v "/javaapi"  | egrep -v "Suite" | egrep -v "/mxnetexamples"`
+    scala_doc_sources=`find . -type f -name "*.scala" | egrep "./core|./infer" | egrep -v "/javaapi"  | egrep -v "Suite" | egrep -v "CancelTestUtil" | egrep -v "/mxnetexamples"`
     jar_native=`find native -name "*.jar" | grep "target/lib/" | tr "\\n" ":" `
     jar_macros=`find macros -name "*.jar" | tr "\\n" ":" `
     jar_core=`find core -name "*.jar" | tr "\\n" ":" `
@@ -1753,7 +1753,7 @@ build_java_docs() {
 
     pushd $java_path
 
-    java_doc_sources=`find . -type f -name "*.scala" | egrep "./core|./infer"  | egrep "/javaapi"  | egrep -v "Suite" | egrep -v "/mxnetexamples"`
+    java_doc_sources=`find . -type f -name "*.scala" | egrep "./core|./infer"  | egrep "/javaapi"  | egrep -v "Suite" | egrep -v "CancelTestUtil" | egrep -v "/mxnetexamples"`
     jar_native=`find native -name "*.jar" | grep "target/lib/" | tr "\\n" ":" `
     jar_macros=`find macros -name "*.jar" | tr "\\n" ":" `
     jar_core=`find core -name "*.jar" | tr "\\n" ":" `

conftest.py

@@ -108,6 +108,11 @@ def pytest_configure():
         logging.warning('*** test-level seed set: all "@with_seed()" '
                         'tests run deterministically ***')
 
+    # Load the user's locale settings to verify that MXNet works correctly when the C locale is set
+    # to anything other than the default value. Please see #16134 for an example of a bug caused by
+    # incorrect handling of C locales.
+    import locale
+    locale.setlocale(locale.LC_ALL, "")
 
 @pytest.hookimpl(tryfirst=True, hookwrapper=True)
 def pytest_runtest_makereport(item, call):

scala-package/core/src/test/scala/org/apache/mxnet/CancelTestUtil.scala

@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+package org.apache.mxnet
+
+import java.text.DecimalFormatSymbols
+import java.util.Locale
+
+import org.scalatest.Assertions
+
+object CancelTestUtil {
+  /**
+   * Cancel the test if the system's locale uses a decimal separator other than '.'. Please see
+   * #18097 for more information.
+   */
+  def assumeStandardDecimalSeparator(): Unit = {
+    val lcNumeric = System.getenv("LC_NUMERIC");
+
+    val decimalFormatSymbols = if (lcNumeric != null) {
+      val localeName = lcNumeric.stripSuffix(".UTF-8".stripSuffix(".utf-8"))
+      val locale = Locale.forLanguageTag(localeName)
+      DecimalFormatSymbols.getInstance(locale)
+    } else {
+      DecimalFormatSymbols.getInstance()
+    }
+
+    val isStandardDecimalPoint = (decimalFormatSymbols.getDecimalSeparator == '.') &&
+      (lcNumeric != null && lcNumeric.toLowerCase != "en_dk.utf-8") // Java doesn't seem to respect
+                                                                    // the decimal separator
+                                                                    // set in en_DK.UTF8, which is
+                                                                    // used in CentOS CI jobs.
+    if (!isStandardDecimalPoint) {
+      Assertions.cancel("Some operators " +
+        "break when the decimal separator is set to anything other than \".\". These operators " +
+        "should be rewritten to utilize the new FFI. Please see #18097 for more information.")
+    }
+  }
+}

scala-package/core/src/test/scala/org/apache/mxnet/ModuleSuite.scala

@@ -167,6 +167,8 @@ class ModuleSuite extends FunSuite with BeforeAndAfterAll {
   }
 
   test ("module reshape") {
+    CancelTestUtil.assumeStandardDecimalSeparator()
+
     var sym = Symbol.Variable("data")
     sym = Symbol.FullyConnected("fc")()(Map("data" -> sym, "num_hidden" -> 20))
 

scala-package/core/src/test/scala/org/apache/mxnet/OperatorSuite.scala

@@ -191,6 +191,8 @@ class OperatorSuite extends FunSuite with BeforeAndAfterAll
   }
 
   test("scalar op") {
+    CancelTestUtil.assumeStandardDecimalSeparator()
+
     val data = Symbol.Variable("data")
     val shape = Shape(3, 4)
     val dataTmp = NDArray.ones(shape) * 5
@@ -256,6 +258,8 @@ class OperatorSuite extends FunSuite with BeforeAndAfterAll
   }
 
   test("symbol pow") {
+    CancelTestUtil.assumeStandardDecimalSeparator()
+
     val shape = Shape(1, 1)
 
     val data = Symbol.Variable("data")
@@ -277,6 +281,8 @@ class OperatorSuite extends FunSuite with BeforeAndAfterAll
   }
 
   test("pow fn") {
+    CancelTestUtil.assumeStandardDecimalSeparator()
+
     val shape = Shape(3, 4)
     val exp = Symbol.Variable("exp")
     import SymbolConversions._

scala-package/core/src/test/scala/org/apache/mxnet/train/ConvSuite.scala

@@ -32,6 +32,7 @@ class ConvSuite extends FunSuite with BeforeAndAfterAll {
   private var tu = new TestUtil
 
   test("train mnist") {
+    CancelTestUtil.assumeStandardDecimalSeparator()
     // symbol net
     val batchSize = 100
 

tests/python/unittest/common.py

@@ -31,6 +31,14 @@
 from contextlib import contextmanager
 import pytest
 from tempfile import TemporaryDirectory
+import locale
+
+xfail_when_nonstandard_decimal_separator = pytest.mark.xfail(
+    locale.localeconv()["decimal_point"] != ".",
+    reason="Some operators break when the decimal separator is set to anything other than \".\". "
+    "These operators should be rewritten to utilize the new FFI. Please see #18097 for more "
+    "information."
+)
 
 def assertRaises(expected_exception, func, *args, **kwargs):
     try:

tests/python/unittest/test_autograd.py

@@ -20,7 +20,7 @@
 from mxnet.ndarray import zeros_like
 from mxnet.autograd import *
 from mxnet.test_utils import *
-from common import setup_module, with_seed, teardown_module
+from common import setup_module, with_seed, teardown_module, xfail_when_nonstandard_decimal_separator
 from mxnet.test_utils import EnvManager
 
 
@@ -107,6 +107,7 @@ def autograd_assert(*args, **kwargs):
     for a, b in zip(grad_vals, grad_res):
         assert same(a.asnumpy(), b.asnumpy())
 
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 def test_unary_func():
     def check_unary_func(x):

tests/python/unittest/test_contrib_autograd.py

@@ -18,7 +18,7 @@
 import mxnet.ndarray as nd
 from mxnet.contrib.autograd import *
 from mxnet.test_utils import *
-from common import setup_module, with_seed, teardown_module
+from common import setup_module, with_seed, teardown_module, xfail_when_nonstandard_decimal_separator
 
 def autograd_assert(*args, **kwargs):
     func   = kwargs["func"]
@@ -34,6 +34,7 @@ def autograd_assert(*args, **kwargs):
     for a, b in zip(grad_vals, grad_res):
         assert same(a.asnumpy(), b.asnumpy())
 
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 def test_unary_func():
     x = nd.uniform(shape=(4, 5))

tests/python/unittest/test_contrib_operator.py

@@ -23,7 +23,8 @@
 import itertools
 from numpy.testing import assert_allclose, assert_array_equal
 from mxnet.test_utils import *
-from common import with_seed, assert_raises_cudnn_not_satisfied
+from common import with_seed, assert_raises_cudnn_not_satisfied, \
+    xfail_when_nonstandard_decimal_separator
 import unittest
 
 def test_box_nms_op():
@@ -285,6 +286,7 @@ def test_multibox_target_op():
     assert_array_equal(loc_mask.asnumpy(), expected_loc_mask)
     assert_array_equal(cls_target.asnumpy(), expected_cls_target)
 
+@xfail_when_nonstandard_decimal_separator
 def test_gradient_multiplier_op():
     # We use the quadratic function in combination with gradient multiplier
     def f(x, a, b, c):

tests/python/unittest/test_contrib_optimizer.py

@@ -23,9 +23,10 @@
 
 curr_path = os.path.dirname(os.path.abspath(os.path.expanduser(__file__)))
 sys.path.insert(0, os.path.join(curr_path, '../unittest'))
-from common import with_seed
+from common import with_seed, xfail_when_nonstandard_decimal_separator
 
 
+@xfail_when_nonstandard_decimal_separator
 def test_group_adagrad():
     mx.random.seed(0)
     opt1 = mx.optimizer.contrib.GroupAdaGrad
@@ -61,6 +62,7 @@ def test_group_adagrad():
                 g_stype='row_sparse')
 
 
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 @pytest.mark.serial
 def test_adamw():

tests/python/unittest/test_contrib_stes_op.py

@@ -15,7 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 
-from common import with_seed
+from common import with_seed, xfail_when_nonstandard_decimal_separator
 import mxnet as mx
 from mxnet import nd, autograd, gluon
 from mxnet.test_utils import default_context
@@ -98,6 +98,7 @@ def check_ste(net_type_str, w_init, hybridize, in_data, ctx=None):
                                                                      str(net.w.grad()) + " but expected " + \
                                                                      str(net.expected_grads(in_data, w_init))
 
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 def test_contrib_round_ste():
     # Test with random data
@@ -119,6 +120,7 @@ def test_contrib_round_ste():
     check_ste(net_type_str="RoundSTENET", w_init=w_init, hybridize=False, in_data=in_data)
 
 
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 def test_contrib_sign_ste():
     in_data = nd.uniform(-10, 10, shape=30)  # 10 and 30 are arbitrary numbers

tests/python/unittest/test_gluon.py

@@ -27,7 +27,7 @@
 from mxnet.test_utils import use_np
 import mxnet.numpy as _mx_np
 from common import (setup_module, with_seed, assertRaises, teardown_module,
-                    assert_raises_cudnn_not_satisfied)
+                    assert_raises_cudnn_not_satisfied, xfail_when_nonstandard_decimal_separator)
 import numpy as np
 from numpy.testing import assert_array_equal
 import pytest
@@ -754,6 +754,7 @@ def test_batchnorm():
     check_layer_forward(layer, (2, 10, 10, 10))
 
 
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 def test_sync_batchnorm():
     def _check_batchnorm_result(input, num_devices=1, cuda=False):
@@ -1377,6 +1378,7 @@ def test_inline():
     assert len_1 == len_2 + 2
 
 
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 def test_activations():
     point_to_validate = mx.nd.array([-0.1, 0.1] * 3)

tests/python/unittest/test_gluon_data_vision.py

@@ -25,7 +25,8 @@
 from mxnet.gluon.data.vision import transforms
 from mxnet import image
 from mxnet.test_utils import *
-from common import assertRaises, setup_module, with_seed, teardown_module
+from common import assertRaises, setup_module, with_seed, teardown_module, \
+    xfail_when_nonstandard_decimal_separator
 
 import numpy as np
 
@@ -318,6 +319,7 @@ def test_random_rotation():
     assert_almost_equal(data, transformer(data))
 
 
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 def test_rotate():
     transformer = transforms.Rotate(10.)
@@ -389,6 +391,7 @@ def test_random_transforms():
             num_apply += 1
     assert_almost_equal(num_apply/float(iteration), 0.5, 0.1)
 
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 def test_random_gray():
     from mxnet.gluon.data.vision import transforms

tests/python/unittest/test_gluon_trainer.py

@@ -22,7 +22,7 @@
 from mxnet import gluon
 from mxnet.gluon import nn
 from mxnet.test_utils import assert_almost_equal
-from common import setup_module, with_seed, assertRaises
+from common import setup_module, with_seed, assertRaises, xfail_when_nonstandard_decimal_separator
 from copy import deepcopy
 import pytest
 
@@ -217,6 +217,7 @@ def check_init(ctxes):
     check_init([mx.cpu(1), mx.cpu(2)])
     check_init([mx.cpu(1)])
 
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 def test_trainer_reset_kv():
     def check_trainer_reset_kv(kv):
@@ -250,6 +251,7 @@ def check_trainer_reset_kv(kv):
     for kv in kvs:
         check_trainer_reset_kv(kv)
 
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 def test_trainer_sparse_kv():
     def check_trainer_sparse_kv(kv, stype, grad_stype, update_on_kv, expected):

tests/python/unittest/test_higher_order_grad.py

@@ -22,7 +22,7 @@
 from operator import mul
 import random
 
-from common import with_seed
+from common import with_seed, xfail_when_nonstandard_decimal_separator
 import mxnet
 from mxnet import nd, autograd, gluon
 from mxnet.test_utils import (
@@ -276,6 +276,7 @@ def grad_grad_op(x):
         check_nth_order_unary(array, log, [grad_op, grad_grad_op], [1, 2])
 
 
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 def test_log2():
     def log2(x):
@@ -289,7 +290,7 @@ def grad_grad_op(x):
         array = random_arrays(shape)
         check_second_order_unary(array, log2, grad_grad_op)
 
-
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 def test_log10():
     def log10(x):
@@ -415,6 +416,7 @@ def grad_grad_op(x):
         check_nth_order_unary(array, sigmoid, grad_grad_op, 2)
 
 
+@xfail_when_nonstandard_decimal_separator
 @with_seed()
 def test_sqrt():
     def sqrt(x):

tests/python/unittest/test_image.py

@@ -20,7 +20,7 @@
 import numpy as np
 import scipy.ndimage
 from mxnet.test_utils import *
-from common import assertRaises, with_seed
+from common import assertRaises, with_seed, xfail_when_nonstandard_decimal_separator
 import shutil
 import tempfile
 import unittest
@@ -366,6 +366,7 @@ def test_random_size_crop(self):
             assert ratio[0] - epsilon <= float(new_w)/new_h <= ratio[1] + epsilon, \
             'ration of new width and height out of the bound{}/{}={}'.format(new_w, new_h, float(new_w)/new_h)
 
+    @xfail_when_nonstandard_decimal_separator
     @with_seed()
     def test_imrotate(self):
         # test correctness