From c411ae613b412296caed0478d627b10c9c30c73b Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 20:06:40 -0400 Subject: [PATCH 01/16] Run CI on Python 3.5 and 3.6 --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 61b7679..0f2b925 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,8 @@ language: python python: - "2.7" + - "3.5" + - "3.6" virtualenv: system_site_packages: true before_install: From 54bb82e69d3b9b6eb9da7cdc82ea6f460c0176aa Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 20:12:00 -0400 Subject: [PATCH 02/16] Skip using system `site-packages` These won't work with Python 3. So simply drop this configuration from Travis CI. --- .travis.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0f2b925..e0c3591 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,8 +3,6 @@ python: - "2.7" - "3.5" - "3.6" -virtualenv: - system_site_packages: true before_install: - sudo apt-get -qq install python-numpy python-scipy install: From 355f1bb2342878b4b5e414aec72a7f74ecd22cdf Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 20:12:49 -0400 Subject: [PATCH 03/16] Don't use `apt-get` to install `numpy` and `scipy` As these will be Python 2 packages, we will need to do without them. Can get them as wheels from PyPI or conda packages to avoid spending time building them. --- .travis.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index e0c3591..0bf6bde 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,8 +3,6 @@ python: - "2.7" - "3.5" - "3.6" -before_install: - - sudo apt-get -qq install python-numpy python-scipy install: - cd code/liblbfgs - ./autogen.sh From d8bf8143ad3c994a817ea85816e1bfd8462b5e52 Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 20:14:52 -0400 Subject: [PATCH 04/16] Install NumPy and SciPy from pip --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 0bf6bde..bc67ded 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,7 @@ python: - "3.5" - "3.6" install: + - pip install numpy scipy - cd code/liblbfgs - ./autogen.sh - ./configure --enable-sse2 From 82014d16262602c30d0da3f3f5afe558af5531c0 Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 20:18:39 -0400 Subject: [PATCH 05/16] Disable `sudo` as it is no longer needed As we are no longer installing with `sudo`, disable it to get faster CI startup time from Travis CI. --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index bc67ded..e2efaef 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,3 +1,4 @@ +sudo: false language: python python: - "2.7" From 73cbdbf6d51f4633633e8741a51d165a87e72731 Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 20:26:02 -0400 Subject: [PATCH 06/16] Fix Python 2/3 compatibility issues with pickle --- code/cmt/python/tests/fvbn_test.py | 4 +-- code/cmt/python/tests/glm_test.py | 4 +-- code/cmt/python/tests/gsm_test.py | 4 +-- code/cmt/python/tests/mcbm_test.py | 20 +++++++------- code/cmt/python/tests/mcgsm_test.py | 4 +-- code/cmt/python/tests/mixture_test.py | 4 +-- code/cmt/python/tests/mlr_test.py | 4 +-- code/cmt/python/tests/nonlinear_test.py | 20 +++++++------- code/cmt/python/tests/preconditioner_test.py | 28 ++++++++++---------- code/cmt/python/tests/stm_test.py | 4 +-- code/cmt/python/tests/univariate_test.py | 12 ++++----- 11 files changed, 54 insertions(+), 54 deletions(-) diff --git a/code/cmt/python/tests/fvbn_test.py b/code/cmt/python/tests/fvbn_test.py index df0410e..f905003 100644 --- a/code/cmt/python/tests/fvbn_test.py +++ b/code/cmt/python/tests/fvbn_test.py @@ -67,11 +67,11 @@ def test_pickle(self): tmp_file = mkstemp()[1] # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'model': model0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: model1 = load(handle)['model'] # make sure parameters haven't changed diff --git a/code/cmt/python/tests/glm_test.py b/code/cmt/python/tests/glm_test.py index 7cbe32f..c1508cc 100644 --- a/code/cmt/python/tests/glm_test.py +++ b/code/cmt/python/tests/glm_test.py @@ -168,11 +168,11 @@ def test_glm_pickle(self): model0.bias = randn() # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'model': model0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: model1 = load(handle)['model'] # make sure parameters haven't changed diff --git a/code/cmt/python/tests/gsm_test.py b/code/cmt/python/tests/gsm_test.py index 8482c5f..778d43c 100644 --- a/code/cmt/python/tests/gsm_test.py +++ b/code/cmt/python/tests/gsm_test.py @@ -86,11 +86,11 @@ def test_pickle(self): tmp_file = mkstemp()[1] # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'model': model0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: model1 = load(handle)['model'] # make sure parameters haven't changed diff --git a/code/cmt/python/tests/mcbm_test.py b/code/cmt/python/tests/mcbm_test.py index 0ca6396..40b9858 100644 --- a/code/cmt/python/tests/mcbm_test.py +++ b/code/cmt/python/tests/mcbm_test.py @@ -149,11 +149,11 @@ def test_pickle(self): tmp_file = mkstemp()[1] # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'mcbm': mcbm0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: mcbm1 = load(handle)['mcbm'] # make sure parameters haven't changed @@ -329,11 +329,11 @@ def test_patchmcbm_pickle(self): tmp_file = mkstemp()[1] # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'model': model0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: model1 = load(handle)['model'] # make sure parameters haven't changed @@ -360,11 +360,11 @@ def test_patchmcbm_pickle(self): tmp_file = mkstemp()[1] # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'model': model0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: model1 = load(handle)['model'] # make sure parameters haven't changed @@ -389,11 +389,11 @@ def test_patchmcbm_pickle(self): logLik = mean(model0.loglikelihood(samples)) # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'model': model0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: model1 = load(handle)['model'] self.assertAlmostEqual(mean(model1.loglikelihood(samples)), logLik) @@ -404,11 +404,11 @@ def test_patchmcbm_pickle(self): model0 = PatchMCBM(rows, cols, xmask, ymask, order) # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'model': model0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: model1 = load(handle)['model'] for i in range(rows): diff --git a/code/cmt/python/tests/mcgsm_test.py b/code/cmt/python/tests/mcgsm_test.py index e965c25..58dc084 100644 --- a/code/cmt/python/tests/mcgsm_test.py +++ b/code/cmt/python/tests/mcgsm_test.py @@ -385,11 +385,11 @@ def test_pickle(self): tmp_file = mkstemp()[1] # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'mcgsm': mcgsm0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: mcgsm1 = load(handle)['mcgsm'] # make sure parameters haven't changed diff --git a/code/cmt/python/tests/mixture_test.py b/code/cmt/python/tests/mixture_test.py index c4db80a..ebb8dc2 100644 --- a/code/cmt/python/tests/mixture_test.py +++ b/code/cmt/python/tests/mixture_test.py @@ -65,11 +65,11 @@ def test_pickle(self): tmp_file = mkstemp()[1] # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'model': model0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: model1 = load(handle)['model'] # make sure parameters haven't changed diff --git a/code/cmt/python/tests/mlr_test.py b/code/cmt/python/tests/mlr_test.py index 082bdc0..9ca0563 100644 --- a/code/cmt/python/tests/mlr_test.py +++ b/code/cmt/python/tests/mlr_test.py @@ -73,11 +73,11 @@ def test_mlr_pickle(self): model0.biases = randn(*model0.biases.shape) # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'model': model0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: model1 = load(handle)['model'] # make sure parameters haven't changed diff --git a/code/cmt/python/tests/nonlinear_test.py b/code/cmt/python/tests/nonlinear_test.py index f133294..3b9081a 100644 --- a/code/cmt/python/tests/nonlinear_test.py +++ b/code/cmt/python/tests/nonlinear_test.py @@ -24,10 +24,10 @@ def test_logistic_function_pickle(self): f0 = LogisticFunction() - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'f': f0}, handle) - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: f1 = load(handle)['f'] x = randn(100) @@ -50,10 +50,10 @@ def test_exponential_function_pickle(self): f0 = ExponentialFunction() - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'f': f0}, handle) - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: f1 = load(handle)['f'] x = randn(100) @@ -78,11 +78,11 @@ def test_histogram_nonlinearity_pickle(self): f0 = HistogramNonlinearity(randn(1, 100), rand(1, 100), 10) # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'f': f0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: f1 = load(handle)['f'] x = randn(1, 100) @@ -118,11 +118,11 @@ def test_blob_nonlinearity_pickle(self): f0 = BlobNonlinearity(4, 0.1) # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'f': f0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: f1 = load(handle)['f'] x = randn(1, 100) @@ -158,11 +158,11 @@ def test_tanh_blob_nonlinearity_pickle(self): f0 = TanhBlobNonlinearity(4, 0.1) # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'f': f0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: f1 = load(handle)['f'] x = randn(1, 100) diff --git a/code/cmt/python/tests/preconditioner_test.py b/code/cmt/python/tests/preconditioner_test.py index cb9f90b..52509ee 100644 --- a/code/cmt/python/tests/preconditioner_test.py +++ b/code/cmt/python/tests/preconditioner_test.py @@ -144,11 +144,11 @@ def test_affine_preconditioner_pickle(self): tmp_file = mkstemp()[1] # store transformation - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'pre': pre0}, handle) # load transformation - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: pre1 = load(handle)['pre'] X, Y = randn(5, 100), randn(2, 100) @@ -179,11 +179,11 @@ def test_affine_transform_pickle(self): tmp_file = mkstemp()[1] # store transformation - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'pre': pre0}, handle) # load transformation - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: pre1 = load(handle)['pre'] X, Y = randn(5, 100), randn(3, 100) @@ -241,11 +241,11 @@ def test_whitening_preconditioner_pickle(self): tmp_file = mkstemp()[1] # store transformation - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'wt': wt0}, handle) # load transformation - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: wt1 = load(handle)['wt'] X, Y = randn(5, 100), randn(2, 100) @@ -287,11 +287,11 @@ def test_whitening_transform_pickle(self): tmp_file = mkstemp()[1] # store transformation - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'wt': wt0}, handle) # load transformation - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: wt1 = load(handle)['wt'] X, Y = randn(5, 100), randn(2, 100) @@ -339,11 +339,11 @@ def test_pca_preconditioner_pickle(self): tmp_file = mkstemp()[1] # store transformation - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'wt': wt0}, handle) # load transformation - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: wt1 = load(handle)['wt'] X, Y = randn(5, 100), randn(2, 100) @@ -428,11 +428,11 @@ def test_pca_transform_pickle(self): tmp_file = mkstemp()[1] # store transformation - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'wt': wt0}, handle) # load transformation - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: wt1 = load(handle)['wt'] X, Y = randn(5, 100), randn(2, 100) @@ -467,11 +467,11 @@ def test_binning_transform_pickle(self): tmp_file = mkstemp()[1] # store transformation - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'pre': pre0}, handle) # load transformation - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: pre1 = load(handle)['pre'] # make sure linear transformation hasn't changed diff --git a/code/cmt/python/tests/stm_test.py b/code/cmt/python/tests/stm_test.py index 52ba431..ddec0bf 100644 --- a/code/cmt/python/tests/stm_test.py +++ b/code/cmt/python/tests/stm_test.py @@ -283,11 +283,11 @@ def test_pickle(self): tmp_file = mkstemp()[1] # store model - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'stm': stm0}, handle) # load model - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: stm1 = load(handle)['stm'] # make sure parameters haven't changed diff --git a/code/cmt/python/tests/univariate_test.py b/code/cmt/python/tests/univariate_test.py index 0059e85..5e242ef 100644 --- a/code/cmt/python/tests/univariate_test.py +++ b/code/cmt/python/tests/univariate_test.py @@ -25,10 +25,10 @@ def test_bernoulli_pickle(self): p0 = Bernoulli(.3) - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'p': p0}, handle) - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: p1 = load(handle)['p'] x = p0.sample(100) @@ -78,10 +78,10 @@ def test_poisson_pickle(self): p0 = Poisson(2.5) - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'p': p0}, handle) - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: p1 = load(handle)['p'] x = p0.sample(100) @@ -126,10 +126,10 @@ def test_binomial_pickle(self): p0 = Binomial(13, .76) - with open(tmp_file, 'w') as handle: + with open(tmp_file, 'wb') as handle: dump({'p': p0}, handle) - with open(tmp_file) as handle: + with open(tmp_file, 'rb') as handle: p1 = load(handle)['p'] x = p0.sample(100) From d85e8da4ee35ed8abee1de0a3416ae063513ce08 Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 21:09:21 -0400 Subject: [PATCH 07/16] Use print function for Python 2/3 compatibility --- code/cmt/python/tests/speed.py | 28 +++++++++++++++------------- tutorials/images.py | 6 ++++-- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/code/cmt/python/tests/speed.py b/code/cmt/python/tests/speed.py index e199bba..8288b02 100644 --- a/code/cmt/python/tests/speed.py +++ b/code/cmt/python/tests/speed.py @@ -1,3 +1,5 @@ +from __future__ import print_function + import sys import socket @@ -16,10 +18,10 @@ args = parser.parse_args(sys.argv[1:]) ### -print socket.gethostname() -print datetime.now() -print args -print +print(socket.gethostname()) +print(datetime.now()) +print(args) +print() ### data = randn(args.dim_in, args.num_data), randn(args.dim_out, args.num_data) @@ -32,24 +34,24 @@ num_scales=6) ### -print 'model.loglikelihood' +print('model.loglikelihood') t = time() for r in range(args.repetitions): model.loglikelihood(*data) -print '{0:12.8f} seconds'.format((time() - t) / float(args.repetitions)) -print +print('{0:12.8f} seconds'.format((time() - t) / float(args.repetitions))) +print() ### -print 'model._check_performance' +print('model._check_performance') for batch_size in [1000, 2000, 5000]: t = model._check_performance(*data, repetitions=args.repetitions, parameters={'batch_size': batch_size}) - print '{0:12.8f} seconds ({1})'.format(t, batch_size) -print + print('{0:12.8f} seconds ({1})'.format(t, batch_size)) +print() ### -print 'model.posterior' +print('model.posterior') t = time() for r in range(args.repetitions): model.posterior(*data) -print '{0:12.8f} seconds'.format((time() - t) / float(args.repetitions)) -print +print('{0:12.8f} seconds'.format((time() - t) / float(args.repetitions))) +print() diff --git a/tutorials/images.py b/tutorials/images.py index 93b94e3..b242115 100644 --- a/tutorials/images.py +++ b/tutorials/images.py @@ -1,3 +1,5 @@ +from __future__ import print_function + """ Trains an MCGSM on a single grayscale image. """ @@ -64,8 +66,8 @@ def main(argv): }) # evaluate model - print 'Average log-likelihood: {0:.4f} [bit/px]'.format( - -model.evaluate(data_test[0], data_test[1], pre)) + print('Average log-likelihood: {0:.4f} [bit/px]'.format( + -model.evaluate(data_test[0], data_test[1], pre))) # synthesize a new image img_sample = sample_image(img, model, input_mask, output_mask, pre) From 925a148f2407f488a0fa6f7ef9485478eecd7695 Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 21:09:47 -0400 Subject: [PATCH 08/16] Require binaries for NumPy and SciPy Forces installation from pre-built binaries (wheels). --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index e2efaef..21e93ef 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,7 @@ python: - "3.5" - "3.6" install: - - pip install numpy scipy + - pip install --only-binary=":all:" numpy scipy - cd code/liblbfgs - ./autogen.sh - ./configure --enable-sse2 From ccc987a9991feed2ff1920418fb522af4e7c2a9e Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 21:10:03 -0400 Subject: [PATCH 09/16] `METH_KEYWORDS` -> `METH_VARARGS | METH_KEYWORDS` This appears to be an annoying bug that was introduced in the very beginning of Python 3 and has been around ever since unfortunately. :( The proposed workaround is to just use note a method can also handle varargs. Though it looks like it is getting fixed as of this year. I don't think we will have releases generally available for this fix in the near term. So this patch should suffice. More details in the linked references. ref: https://bugs.python.org/issue11587 (original) ref: https://bugs.python.org/issue15657 (duplicate, was fixed) --- code/cmt/python/src/module.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/code/cmt/python/src/module.cpp b/code/cmt/python/src/module.cpp index 859ad1e..c897327 100644 --- a/code/cmt/python/src/module.cpp +++ b/code/cmt/python/src/module.cpp @@ -762,7 +762,7 @@ static PyGetSetDef PatchModel_getset[] = { }; static PyMethodDef PatchModel_methods[] = { - {"loglikelihood", (PyCFunction)PatchModel_loglikelihood, METH_KEYWORDS, 0}, + {"loglikelihood", (PyCFunction)PatchModel_loglikelihood, METH_VARARGS | METH_KEYWORDS, 0}, {"input_mask", (PyCFunction)PatchModel_input_mask, METH_VARARGS, 0}, {"output_mask", (PyCFunction)PatchModel_output_mask, METH_VARARGS, 0}, {"input_indices", (PyCFunction)PatchModel_input_indices, 0, 0}, @@ -825,9 +825,9 @@ static PyGetSetDef PatchMCBM_getset[] = { }; static PyMethodDef PatchMCBM_methods[] = { - {"initialize", (PyCFunction)PatchMCBM_initialize, METH_KEYWORDS, PatchMCBM_initialize_doc}, - {"train", (PyCFunction)PatchMCBM_train, METH_KEYWORDS, PatchMCBM_train_doc}, - {"preconditioner", (PyCFunction)PatchMCBM_preconditioner, METH_VARARGS, 0}, + {"initialize", (PyCFunction)PatchMCBM_initialize, METH_VARARGS | METH_KEYWORDS, PatchMCBM_initialize_doc}, + {"train", (PyCFunction)PatchMCBM_train, METH_VARARGS | METH_KEYWORDS, PatchMCBM_train_doc}, + {"preconditioner", (PyCFunction)PatchMCBM_preconditioner, METH_VARARGS | METH_VARARGS, 0}, {"__reduce__", (PyCFunction)PatchMCBM_reduce, METH_NOARGS, PatchMCBM_reduce_doc}, {"__setstate__", (PyCFunction)PatchMCBM_setstate, METH_VARARGS, PatchMCBM_setstate_doc}, {0} @@ -889,8 +889,8 @@ static PyGetSetDef PatchMCGSM_getset[] = { }; static PyMethodDef PatchMCGSM_methods[] = { - {"initialize", (PyCFunction)PatchMCGSM_initialize, METH_KEYWORDS, PatchMCGSM_initialize_doc}, - {"train", (PyCFunction)PatchMCGSM_train, METH_KEYWORDS, PatchMCGSM_train_doc}, + {"initialize", (PyCFunction)PatchMCGSM_initialize, METH_VARARGS | METH_KEYWORDS, PatchMCGSM_initialize_doc}, + {"train", (PyCFunction)PatchMCGSM_train, METH_VARARGS | METH_KEYWORDS, PatchMCGSM_train_doc}, {"preconditioner", (PyCFunction)PatchMCGSM_preconditioner, METH_VARARGS, 0}, {"__reduce__", (PyCFunction)PatchMCGSM_reduce, METH_NOARGS, PatchMCGSM_reduce_doc}, {"__setstate__", (PyCFunction)PatchMCGSM_setstate, METH_VARARGS, PatchMCGSM_setstate_doc}, @@ -1124,8 +1124,8 @@ static PyGetSetDef FVBN_getset[] = { }; static PyMethodDef FVBN_methods[] = { - {"initialize", (PyCFunction)FVBN_initialize, METH_KEYWORDS, FVBN_initialize_doc}, - {"train", (PyCFunction)FVBN_train, METH_KEYWORDS, FVBN_train_doc}, + {"initialize", (PyCFunction)FVBN_initialize, METH_VARARGS | METH_KEYWORDS, FVBN_initialize_doc}, + {"train", (PyCFunction)FVBN_train, METH_VARARGS | METH_KEYWORDS, FVBN_train_doc}, {"preconditioner", (PyCFunction)FVBN_preconditioner, METH_VARARGS, 0}, {"__reduce__", (PyCFunction)FVBN_reduce, METH_NOARGS, FVBN_reduce_doc}, {"__setstate__", (PyCFunction)FVBN_setstate, METH_VARARGS, FVBN_setstate_doc}, From d20f0387a723bae9c65403f1651c294ec343c33f Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 21:36:24 -0400 Subject: [PATCH 10/16] Set input_indices arguments to METH_VARARGS It seems this function should be taking arguments. At least its usage seems to and definition seem to indicate that. So set its argument format to `METH_VARARGS`. --- code/cmt/python/src/module.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/cmt/python/src/module.cpp b/code/cmt/python/src/module.cpp index c897327..4e78114 100644 --- a/code/cmt/python/src/module.cpp +++ b/code/cmt/python/src/module.cpp @@ -765,7 +765,7 @@ static PyMethodDef PatchModel_methods[] = { {"loglikelihood", (PyCFunction)PatchModel_loglikelihood, METH_VARARGS | METH_KEYWORDS, 0}, {"input_mask", (PyCFunction)PatchModel_input_mask, METH_VARARGS, 0}, {"output_mask", (PyCFunction)PatchModel_output_mask, METH_VARARGS, 0}, - {"input_indices", (PyCFunction)PatchModel_input_indices, 0, 0}, + {"input_indices", (PyCFunction)PatchModel_input_indices, METH_VARARGS, 0}, {0} }; From 9348906e272911307e7a5ba563cdb866aca3654a Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 21:40:35 -0400 Subject: [PATCH 11/16] Update pip on Travis CI It seem the default version of pip that Travis CI is using on Python 2.7 is quite ancient (version 6.0.7). To remedy this, go ahead and upgrade pip before doing anything else. As a precautionary measure, run pip through Python when doing the upgrade. While calling pip directly to perform this upgrade shouldn't cause any issues, some have reported problems on Windows. Though these cases are likely due to how file removal is handled on Windows. In any event, taking the safest route possible seems wise when upgrading pip. --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 21e93ef..6a5fc79 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,7 @@ python: - "3.5" - "3.6" install: + - python -m pip install -U pip - pip install --only-binary=":all:" numpy scipy - cd code/liblbfgs - ./autogen.sh From 44f4c799ceea0aeacbc7610c63ad41f12a44abc1 Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 22:43:28 -0400 Subject: [PATCH 12/16] Use `^` with masks instead of `-` NumPy is deprecating the use of `-` on NumPy arrays. So make use of the suggested `^`, which will give the same result. --- code/cmt/python/tests/fvbn_test.py | 4 ++-- code/cmt/python/tests/mcbm_test.py | 18 +++++++++--------- code/cmt/python/tests/mcgsm_test.py | 14 +++++++------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/code/cmt/python/tests/fvbn_test.py b/code/cmt/python/tests/fvbn_test.py index f905003..01555ae 100644 --- a/code/cmt/python/tests/fvbn_test.py +++ b/code/cmt/python/tests/fvbn_test.py @@ -18,8 +18,8 @@ def test_fvbn(self): model = FVBN(8, 8, xmask, ymask) - self.assertLess(max(abs(model.input_mask() - xmask)), 1e-8) - self.assertLess(max(abs(model.output_mask() - ymask)), 1e-8) + self.assertLess(max(abs(model.input_mask() ^ xmask)), 1e-8) + self.assertLess(max(abs(model.output_mask() ^ ymask)), 1e-8) for i in range(8): for j in range(8): diff --git a/code/cmt/python/tests/mcbm_test.py b/code/cmt/python/tests/mcbm_test.py index 40b9858..7d4feaa 100644 --- a/code/cmt/python/tests/mcbm_test.py +++ b/code/cmt/python/tests/mcbm_test.py @@ -178,8 +178,8 @@ def test_patchmcbm(self): model = PatchMCBM(8, 8, xmask, ymask, model=MCBM(sum(xmask), 1)) - self.assertLess(max(abs(model.input_mask() - xmask)), 1e-8) - self.assertLess(max(abs(model.output_mask() - ymask)), 1e-8) + self.assertLess(max(abs(model.input_mask() ^ xmask)), 1e-8) + self.assertLess(max(abs(model.output_mask() ^ ymask)), 1e-8) for i in range(8): for j in range(8): @@ -192,8 +192,8 @@ def test_patchmcbm(self): model = PatchMCBM(rows, cols, xmask, ymask, order, MCBM(sum(xmask), 1)) - self.assertLess(max(abs(model.input_mask() - xmask)), 1e-8) - self.assertLess(max(abs(model.output_mask() - ymask)), 1e-8) + self.assertLess(max(abs(model.input_mask() ^ xmask)), 1e-8) + self.assertLess(max(abs(model.output_mask() ^ ymask)), 1e-8) for i in range(rows): for j in range(cols): @@ -203,8 +203,8 @@ def test_patchmcbm(self): model0 = PatchMCBM(rows, cols, max_pcs=3) model1 = PatchMCBM(rows, cols, model0.input_mask(), model0.output_mask(), model0.order) - self.assertLess(max(abs(model0.input_mask() - model1.input_mask())), 1e-8) - self.assertLess(max(abs(model0.output_mask() - model1.output_mask())), 1e-8) + self.assertLess(max(abs(model0.input_mask() ^ model1.input_mask())), 1e-8) + self.assertLess(max(abs(model0.output_mask() ^ model1.output_mask())), 1e-8) self.assertLess(max(abs(asarray(model0.order) - asarray(model1.order))), 1e-8) # test computation of input masks @@ -213,7 +213,7 @@ def test_patchmcbm(self): i, j = model0.order[0] input_mask = model.input_mask(i, j) for i, j in model.order[1:]: - self.assertEqual(sum(model.input_mask(i, j) - input_mask), 1) + self.assertEqual(sum(model.input_mask(i, j) ^ input_mask), 1) input_mask = model.input_mask(i, j) @@ -371,8 +371,8 @@ def test_patchmcbm_pickle(self): self.assertEqual(model0.rows, model1.rows) self.assertEqual(model0.cols, model1.cols) self.assertLess(max(abs(asarray(model1.order) - asarray(model0.order))), 1e-20) - self.assertLess(max(abs(model0.input_mask() - model1.input_mask())), 1e-20) - self.assertLess(max(abs(model0.output_mask() - model1.output_mask())), 1e-20) + self.assertLess(max(abs(model0.input_mask() ^ model1.input_mask())), 1e-20) + self.assertLess(max(abs(model0.output_mask() ^ model1.output_mask())), 1e-20) for i in range(model0.rows): for j in range(model0.cols): diff --git a/code/cmt/python/tests/mcgsm_test.py b/code/cmt/python/tests/mcgsm_test.py index 58dc084..19a98af 100644 --- a/code/cmt/python/tests/mcgsm_test.py +++ b/code/cmt/python/tests/mcgsm_test.py @@ -421,8 +421,8 @@ def test_patchmcgsm(self): model = PatchMCGSM(8, 8, xmask, ymask, model=MCGSM(sum(xmask), 1)) - self.assertLess(max(abs(model.input_mask() - xmask)), 1e-8) - self.assertLess(max(abs(model.output_mask() - ymask)), 1e-8) + self.assertLess(max(abs(model.input_mask() ^ xmask)), 1e-8) + self.assertLess(max(abs(model.output_mask() ^ ymask)), 1e-8) for i in range(8): for j in range(8): @@ -435,8 +435,8 @@ def test_patchmcgsm(self): model = PatchMCGSM(rows, cols, xmask, ymask, order, MCGSM(sum(xmask), 1)) - self.assertLess(max(abs(model.input_mask() - xmask)), 1e-8) - self.assertLess(max(abs(model.output_mask() - ymask)), 1e-8) + self.assertLess(max(abs(model.input_mask() ^ xmask)), 1e-8) + self.assertLess(max(abs(model.output_mask() ^ ymask)), 1e-8) for i in range(rows): for j in range(cols): @@ -446,8 +446,8 @@ def test_patchmcgsm(self): model0 = PatchMCGSM(rows, cols, max_pcs=3) model1 = PatchMCGSM(rows, cols, model0.input_mask(), model0.output_mask(), model0.order) - self.assertLess(max(abs(model0.input_mask() - model1.input_mask())), 1e-8) - self.assertLess(max(abs(model0.output_mask() - model1.output_mask())), 1e-8) + self.assertLess(max(abs(model0.input_mask() ^ model1.input_mask())), 1e-8) + self.assertLess(max(abs(model0.output_mask() ^ model1.output_mask())), 1e-8) self.assertLess(max(abs(asarray(model0.order) - asarray(model1.order))), 1e-8) # test computation of input masks @@ -456,7 +456,7 @@ def test_patchmcgsm(self): i, j = model0.order[0] input_mask = model.input_mask(i, j) for i, j in model.order[1:]: - self.assertEqual(sum(model.input_mask(i, j) - input_mask), 1) + self.assertEqual(sum(model.input_mask(i, j) ^ input_mask), 1) input_mask = model.input_mask(i, j) From bdfb406df4ce83d68c4028395bdf9d99fca78512 Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 22:56:31 -0400 Subject: [PATCH 13/16] Handle unicode regularizer On Python 3, the default string is of Unicode type, which caused this comparison some issues. In particular, the length comparison was off as the Unicode string may have more bytes than the equivalent ASCII string. To fix that, just encode Unicode strings as ASCII byte strings in Python. This way handling of them can proceed exactly as before. As it is also possible to get Unicode values on Python 2, it doesn't hurt to handle them appropriately as well. --- code/cmt/python/src/pyutils.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/code/cmt/python/src/pyutils.cpp b/code/cmt/python/src/pyutils.cpp index a4eb3a7..c5806a0 100644 --- a/code/cmt/python/src/pyutils.cpp +++ b/code/cmt/python/src/pyutils.cpp @@ -378,7 +378,11 @@ Regularizer PyObject_ToRegularizer(PyObject* regularizer) { Regularizer::Norm norm = Regularizer::L2; if(r_norm) { + if(PyUnicode_Check(r_norm)) + r_norm = PyUnicode_AsASCIIString(r_norm); + if(PyString_Size(r_norm) != 2) + PyErr_Clear(); throw Exception("Regularizer norm should be 'L1' or 'L2'."); switch(PyString_AsString(r_norm)[1]) { From 6842b96a93c7e9af238c3fa3f6a3ccb913ace260 Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 23:05:15 -0400 Subject: [PATCH 14/16] Improve error checking in regularizer In some cases a conversion may not go smoothly or may not be the expected type. The return values in these cases will be `NULL`. If an error is set twice though, this result in an interpreter crash. So try to handle a few of these around string conversion to avoid crashing. Also call `PyErr_Clear` before throwing exceptions where there may already be a Python exception being unwound. If there is not one being unwound, this is a no-op. --- code/cmt/python/src/pyutils.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/code/cmt/python/src/pyutils.cpp b/code/cmt/python/src/pyutils.cpp index c5806a0..22ac8cf 100644 --- a/code/cmt/python/src/pyutils.cpp +++ b/code/cmt/python/src/pyutils.cpp @@ -16,6 +16,7 @@ using std::make_pair; #define PyInt_FromLong PyLong_FromLong #define PyInt_AsLong PyLong_AsLong #define PyInt_Check PyLong_Check + #define PyString_Check PyBytes_Check #define PyString_Size PyBytes_Size #define PyString_AsString PyBytes_AsString #endif @@ -381,11 +382,18 @@ Regularizer PyObject_ToRegularizer(PyObject* regularizer) { if(PyUnicode_Check(r_norm)) r_norm = PyUnicode_AsASCIIString(r_norm); - if(PyString_Size(r_norm) != 2) + if((r_norm == NULL) || (!PyString_Check(r_norm)) || (PyString_Size(r_norm) != 2)) { PyErr_Clear(); throw Exception("Regularizer norm should be 'L1' or 'L2'."); + } + + char* r_norm_str = PyString_AsString(r_norm); + if(r_norm_str == NULL) { + PyErr_Clear(); + throw Exception("Regularizer norm should be 'L1' or 'L2'."); + } - switch(PyString_AsString(r_norm)[1]) { + switch(r_norm_str[1]) { default: throw Exception("Regularizer norm should be 'L1' or 'L2'."); From 03e588525b5c7e1c42a13b2d4bb632e0558ce526 Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 23:13:20 -0400 Subject: [PATCH 15/16] Convert `range` to `list` on Python 3 In Python 3, `range` behaves like `xrange`. So it does not eagerly evaluate and return a `list`. Instead it remains an object that can be iterated over and has a few other properties. This differs from Python 2 where `range` returns a `list`. To correct this incompatibility, simply pass the result of `range` to a `list`. If the result is already a `list` as it is on Python 2, this is a no-op. However if `range` returns an iterable object like it is on Python 3, this will convert it to a `list` giving the expected result seen on Python 2. --- code/cmt/python/tests/mcgsm_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/cmt/python/tests/mcgsm_test.py b/code/cmt/python/tests/mcgsm_test.py index 19a98af..3c66348 100644 --- a/code/cmt/python/tests/mcgsm_test.py +++ b/code/cmt/python/tests/mcgsm_test.py @@ -127,7 +127,7 @@ def callback(i, mcgsm): }) # test callback - self.assertTrue(range(cb_iter, max_iter + 1, cb_iter) == count) + self.assertTrue(list(range(cb_iter, max_iter + 1, cb_iter)) == count) From 296a76e6386149eb7a87d92bb7d95953aa324e3b Mon Sep 17 00:00:00 2001 From: John Kirkham Date: Sun, 11 Jun 2017 23:27:33 -0400 Subject: [PATCH 16/16] Get the `pvalue` from SciPy's `ks_2samp` It appears that SciPy's `ks_2samp` has slightly different behavior on Python 2 vs. Python 3. In particular on Python 2 it reduces to its `pvalue` when performing comparisons to numeric values. However on Python 3 this behavior does not appear to be supported. To address this issue, simply get the `pvalue` attribute regardless. This yields the correct behavior on both Python 2 and Python 3. More details in the linked issue. xref: https://github.com/scipy/scipy/issues/6099 --- code/cmt/python/tests/mcgsm_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/code/cmt/python/tests/mcgsm_test.py b/code/cmt/python/tests/mcgsm_test.py index 3c66348..00562b2 100644 --- a/code/cmt/python/tests/mcgsm_test.py +++ b/code/cmt/python/tests/mcgsm_test.py @@ -174,9 +174,9 @@ def test_mogsm(self): # generated samples should have the same distribution for i in range(mogsm.dim): - self.assertTrue(ks_2samp(mogsm_samples[i], mcgsm_samples[0]) > 0.0001) - self.assertTrue(ks_2samp(mogsm_samples[i], mcgsm_samples[1]) > 0.0001) - self.assertTrue(ks_2samp(mogsm_samples[i], mcgsm_samples[2]) > 0.0001) + self.assertTrue(ks_2samp(mogsm_samples[i], mcgsm_samples[0]).pvalue > 0.0001) + self.assertTrue(ks_2samp(mogsm_samples[i], mcgsm_samples[1]).pvalue > 0.0001) + self.assertTrue(ks_2samp(mogsm_samples[i], mcgsm_samples[2]).pvalue > 0.0001) posterior = mcgsm.posterior(input, mcgsm_samples)