From 7f0c453be761b9008bf4e481448835432607d15b Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Wed, 24 Jan 2018 11:29:04 -0500 Subject: [PATCH 01/25] Take ownership of given parameters to fix memory leak. This fixes #1201 but does collateral damage: now every matrix that is passed in will be modified. This will be fixed next... --- src/mlpack/bindings/python/mlpack/cli.pxd | 4 +-- .../bindings/python/mlpack/cli_util.hpp | 15 ++++++----- .../python/print_input_processing.hpp | 4 +++ .../python/tests/test_python_binding.py | 27 ++++++++++--------- 4 files changed, 29 insertions(+), 21 deletions(-) diff --git a/src/mlpack/bindings/python/mlpack/cli.pxd b/src/mlpack/bindings/python/mlpack/cli.pxd index d081a774b0..860bc4a756 100644 --- a/src/mlpack/bindings/python/mlpack/cli.pxd +++ b/src/mlpack/bindings/python/mlpack/cli.pxd @@ -37,8 +37,8 @@ cdef extern from "" namespace "mlpack" nogil: cdef extern from "" \ namespace "mlpack::util" nogil: - void SetParam[T](string, const T&) nogil except + - void SetParamWithInfo[T](string, const T&, const bool*) nogil except + + void SetParam[T](string, T&) nogil except + + void SetParamWithInfo[T](string, T&, const bool*) nogil except + (T&) GetParamWithInfo[T](string) nogil except + void EnableVerbose() nogil except + void DisableVerbose() nogil except + diff --git a/src/mlpack/bindings/python/mlpack/cli_util.hpp b/src/mlpack/bindings/python/mlpack/cli_util.hpp index 657eb7b14a..85a9be170e 100644 --- a/src/mlpack/bindings/python/mlpack/cli_util.hpp +++ b/src/mlpack/bindings/python/mlpack/cli_util.hpp @@ -29,9 +29,9 @@ namespace util { * @param value Value to set parameter to. */ template -inline void SetParam(const std::string& identifier, const T& value) +inline void SetParam(const std::string& identifier, T& value) { - CLI::GetParam(identifier) = value; + CLI::GetParam(identifier) = std::move(value); } /** @@ -39,19 +39,20 @@ inline void SetParam(const std::string& identifier, const T& value) */ template inline void SetParamWithInfo(const std::string& identifier, - const T& matrix, + T& matrix, const bool* dims) { typedef typename std::tuple TupleType; typedef typename T::elem_type eT; // The true type of the parameter is std::tuple. - std::get<1>(CLI::GetParam(identifier)) = matrix; + const size_t dimensions = matrix.n_rows; + std::get<1>(CLI::GetParam(identifier)) = std::move(matrix); data::DatasetInfo& di = std::get<0>(CLI::GetParam(identifier)); - di = data::DatasetInfo(matrix.n_rows); + di = data::DatasetInfo(dimensions); bool hasCategoricals = false; - for (size_t i = 0; i < matrix.n_rows; ++i) + for (size_t i = 0; i < dimensions; ++i) { if (dims[i]) { @@ -65,7 +66,7 @@ inline void SetParamWithInfo(const std::string& identifier, { arma::vec maxs = arma::max(matrix, 1); - for (size_t i = 0; i < matrix.n_rows; ++i) + for (size_t i = 0; i < dimensions; ++i) { if (dims[i]) { diff --git a/src/mlpack/bindings/python/print_input_processing.hpp b/src/mlpack/bindings/python/print_input_processing.hpp index 533b81eb40..9982449fa9 100644 --- a/src/mlpack/bindings/python/print_input_processing.hpp +++ b/src/mlpack/bindings/python/print_input_processing.hpp @@ -123,6 +123,7 @@ void PrintInputProcessing( << std::endl; std::cout << prefix << " CLI.SetPassed( '" << d.name << "')" << std::endl; + std::cout << prefix << " del " << d.name << "_mat"; } else { @@ -135,6 +136,7 @@ void PrintInputProcessing( << std::endl; std::cout << prefix << "CLI.SetPassed( '" << d.name << "')" << std::endl; + std::cout << prefix << "del " << d.name << "_mat"; } std::cout << std::endl; } @@ -250,6 +252,7 @@ void PrintInputProcessing( << "bool*> " << d.name << "_dims.data)" << std::endl; std::cout << prefix << " CLI.SetPassed( '" << d.name << "')" << std::endl; + std::cout << prefix << " del " << d.name << "_mat" << std::endl; } else { @@ -264,6 +267,7 @@ void PrintInputProcessing( << "bool*> " << d.name << "_dims.data)" << std::endl; std::cout << prefix << "CLI.SetPassed( '" << d.name << "')" << std::endl; + std::cout << prefix << "del " << d.name << "_mat" << std::endl; } std::cout << std::endl; } diff --git a/src/mlpack/bindings/python/tests/test_python_binding.py b/src/mlpack/bindings/python/tests/test_python_binding.py index b72ecd34f2..38ae922364 100644 --- a/src/mlpack/bindings/python/tests/test_python_binding.py +++ b/src/mlpack/bindings/python/tests/test_python_binding.py @@ -7,8 +7,10 @@ Test that passing types to Python bindings works successfully. import unittest import pandas as pd import numpy as np +import copy from mlpack.test_python_binding import test_python_binding +from mlpack.matrix_utils import to_matrix_with_info class TestPythonBinding(unittest.TestCase): """ @@ -98,7 +100,7 @@ class TestPythonBinding(unittest.TestCase): output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - matrix_in=x) + matrix_in=copy.copy(x)) self.assertEqual(output['matrix_out'].shape[0], 100) self.assertEqual(output['matrix_out'].shape[1], 4) @@ -121,7 +123,7 @@ class TestPythonBinding(unittest.TestCase): output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - matrix_in=x) + matrix_in=copy.copy(x)) self.assertEqual(output['matrix_out'].shape[0], 3) self.assertEqual(output['matrix_out'].shape[1], 4) @@ -148,7 +150,7 @@ class TestPythonBinding(unittest.TestCase): output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - umatrix_in=x) + umatrix_in=copy.copy(x)) self.assertEqual(output['umatrix_out'].shape[0], 100) self.assertEqual(output['umatrix_out'].shape[1], 4) @@ -171,7 +173,7 @@ class TestPythonBinding(unittest.TestCase): output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - umatrix_in=x) + umatrix_in=copy.copy(x)) self.assertEqual(output['umatrix_out'].shape[0], 3) self.assertEqual(output['umatrix_out'].shape[1], 4) @@ -198,7 +200,7 @@ class TestPythonBinding(unittest.TestCase): output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - col_in=x) + col_in=copy.copy(x)) self.assertEqual(output['col_out'].shape[0], 100) self.assertEqual(output['col_out'].dtype, np.double) @@ -215,7 +217,7 @@ class TestPythonBinding(unittest.TestCase): output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - ucol_in=x) + ucol_in=copy.copy(x)) self.assertEqual(output['ucol_out'].shape[0], 100) self.assertEqual(output['ucol_out'].dtype, np.long) @@ -231,7 +233,7 @@ class TestPythonBinding(unittest.TestCase): output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - row_in=x) + row_in=copy.copy(x)) self.assertEqual(output['row_out'].shape[0], 100) self.assertEqual(output['row_out'].dtype, np.double) @@ -248,7 +250,7 @@ class TestPythonBinding(unittest.TestCase): output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - urow_in=x) + urow_in=copy.copy(x)) self.assertEqual(output['urow_out'].shape[0], 100) self.assertEqual(output['urow_out'].dtype, np.long) @@ -265,7 +267,7 @@ class TestPythonBinding(unittest.TestCase): output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - matrix_and_info_in=x) + matrix_and_info_in=copy.copy(x)) self.assertEqual(output['matrix_and_info_out'].shape[0], 100) self.assertEqual(output['matrix_and_info_out'].shape[1], 10) @@ -281,11 +283,12 @@ class TestPythonBinding(unittest.TestCase): x = pd.DataFrame(np.random.rand(10, 4), columns=list('abcd')) x['e'] = pd.Series(['a', 'b', 'c', 'd', 'a', 'b', 'e', 'c', 'a', 'b'], dtype='category') + z, d = to_matrix_with_info(x, np.float64) output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - matrix_and_info_in=x) + matrix_and_info_in=copy.copy(x)) self.assertEqual(output['matrix_and_info_out'].shape[0], 10) self.assertEqual(output['matrix_and_info_out'].shape[1], 5) @@ -294,10 +297,10 @@ class TestPythonBinding(unittest.TestCase): for i in range(4): for j in range(10): - self.assertEqual(output['matrix_and_info_out'][j, i], x[cols[i]][j] * 2) + self.assertEqual(output['matrix_and_info_out'][j, i], z[j, i] * 2) for j in range(10): - self.assertEqual(output['matrix_and_info_out'][j, 4], x[cols[4]][j]) + self.assertEqual(output['matrix_and_info_out'][j, 4], z[j, 4] * 2) def testIntVector(self): """ From 7f3f4514dadbf4c2a3b6718812437c7babe22488 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Wed, 24 Jan 2018 18:34:20 -0500 Subject: [PATCH 02/25] Print default values in the help. --- src/mlpack/bindings/python/print_doc.hpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/mlpack/bindings/python/print_doc.hpp b/src/mlpack/bindings/python/print_doc.hpp index 08bd612da4..08cbebdf26 100644 --- a/src/mlpack/bindings/python/print_doc.hpp +++ b/src/mlpack/bindings/python/print_doc.hpp @@ -40,6 +40,25 @@ void PrintDoc(const util::ParamData& d, else oss << d.name << " ("; oss << GetPythonType(d) << "): " << d.desc; + + // Print a default, if possible. + if (!d.required) + { + if (d.cppType == "std::string") + { + oss << " Default value '" << boost::any_cast(d.value) + << "'."; + } + else if (d.cppType == "double") + { + oss << " Default value " << boost::any_cast(d.value) << "."; + } + else if (d.cppType == "int") + { + oss << " Default value " << boost::any_cast(d.value) << "."; + } + } + std::cout << util::HyphenateString(oss.str(), indent + 4); } From 92a5b6b0afd10249b1b78b05091290f8c92f1e97 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Wed, 24 Jan 2018 18:34:37 -0500 Subject: [PATCH 03/25] Fix more subtle memory bugs. First: don't transfer the memory state to Armadillo. Instead, make a non-strict alias; this means that Armadillo will only allocate new memory on a size change, and we won't end up making the Python user's numpy array be nothing when we're done because we std::move()d the memory. Second: Only transfer the ownership of the matrix to numpy if the Armadillo matrix actually allocated its own memory. This can help with situations where the user passes a numpy matrix that they have a reference to that will also be a part of the output. --- .../bindings/python/mlpack/arma_numpy.pyx | 94 ++++++++----------- .../bindings/python/mlpack/arma_util.hpp | 14 +++ .../bindings/python/mlpack/cli_util.hpp | 3 +- .../python/tests/test_python_binding.py | 35 ++++--- 4 files changed, 79 insertions(+), 67 deletions(-) diff --git a/src/mlpack/bindings/python/mlpack/arma_numpy.pyx b/src/mlpack/bindings/python/mlpack/arma_numpy.pyx index 8a550c40f2..40508d17a1 100644 --- a/src/mlpack/bindings/python/mlpack/arma_numpy.pyx +++ b/src/mlpack/bindings/python/mlpack/arma_numpy.pyx @@ -31,6 +31,7 @@ cdef extern from "numpy/arrayobject.h": cdef extern from "": void SetMemState[T](T& m, int state) + size_t GetMemState[T](T& m) double* GetMemory(arma.Mat[double]& m) double* GetMemory(arma.Col[double]& m) double* GetMemory(arma.Row[double]& m) @@ -41,35 +42,28 @@ cdef extern from "": cdef arma.Mat[double]* numpy_to_mat_d(numpy.ndarray[numpy.double_t, ndim=2] X) \ except +: """ - Convert a numpy ndarray to a matrix. + Convert a numpy ndarray to a matrix. The memory will still be owned by numpy. """ if not (X.flags.c_contiguous or X.flags.owndata): # If needed, make a copy where we own the memory. X = X.copy(order="C") - cdef arma.Mat[double]* m = new arma.Mat[double]( X.data, X.shape[1], X.shape[0], False, True) - - # Transfer ownership to the Armadillo matrix. - PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) - SetMemState[arma.Mat[double]](m[0], 0) + cdef arma.Mat[double]* m = new arma.Mat[double]( X.data, X.shape[1],\ + X.shape[0], False, False) return m cdef arma.Mat[size_t]* numpy_to_mat_s(numpy.ndarray[numpy.npy_intp, ndim=2] X) \ except +: """ - Convert a numpy ndarray to a matrix. + Convert a numpy ndarray to a matrix. The memory will still be owned by numpy. """ if not (X.flags.c_contiguous or X.flags.owndata): # If needed, make a copy where we own the memory. X = X.copy(order="C") cdef arma.Mat[size_t]* m = new arma.Mat[size_t]( X.data, X.shape[1], - X.shape[0], False, True) - - # Transfer ownership to the Armadillo matrix. - PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) - SetMemState[arma.Mat[size_t]](m[0], 0) + X.shape[0], False, False) return m @@ -85,9 +79,10 @@ cdef numpy.ndarray[numpy.double_t, ndim=2] mat_to_numpy_d(arma.Mat[double]& X) \ cdef numpy.ndarray[numpy.double_t, ndim=2] output = \ numpy.PyArray_SimpleNewFromData(2, &dims[0], numpy.NPY_DOUBLE, GetMemory(X)) - # Transfer memory ownership. - SetMemState[arma.Mat[double]](X, 1) - PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) + # Transfer memory ownership, if needed. + if GetMemState[arma.Mat[double]](X) == 0: + SetMemState[arma.Mat[double]](X, 1) + PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) return output @@ -103,45 +98,40 @@ cdef numpy.ndarray[numpy.npy_intp, ndim=2] mat_to_numpy_s(arma.Mat[size_t]& X) \ cdef numpy.ndarray[numpy.npy_intp, ndim=2] output = \ numpy.PyArray_SimpleNewFromData(2, &dims[0], numpy.NPY_INTP, GetMemory(X)) - # Transfer memory ownership. - SetMemState[arma.Mat[size_t]](X, 1) - PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) + # Transfer memory ownership, if needed. + if GetMemState[arma.Mat[size_t]](X) == 0: + SetMemState[arma.Mat[size_t]](X, 1) + PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) return output cdef arma.Row[double]* numpy_to_row_d(numpy.ndarray[numpy.double_t, ndim=1] X) \ except +: """ - Convert a numpy one-dimensional ndarray to a row. + Convert a numpy one-dimensional ndarray to a row. The memory will still be + owned by numpy. """ if not (X.flags.c_contiguous or X.flags.owndata): # If needed, make a copy where we own the memory. X = X.copy(order="C") cdef arma.Row[double]* m = new arma.Row[double]( X.data, X.shape[0], - False, True) - - # Transfer ownership to the Armadillo matrix. - PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) - SetMemState[arma.Row[double]](m[0], 0) + False, False) return m cdef arma.Row[size_t]* numpy_to_row_s(numpy.ndarray[numpy.npy_intp, ndim=1] X) \ except +: """ - Convert a numpy one-dimensional ndarray to a row. + Convert a numpy one-dimensional ndarray to a row. The memory will still be + owned by numpy. """ if not (X.flags.c_contiguous or X.flags.owndata): # If needed, make a copy where we own the memory. X = X.copy(order="C") cdef arma.Row[size_t]* m = new arma.Row[size_t]( X.data, X.shape[0], - False, True) - - # Transfer ownership to the Armadillo matrix. - PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) - SetMemState[arma.Row[size_t]](m[0], 0) + False, False) return m @@ -155,9 +145,10 @@ cdef numpy.ndarray[numpy.double_t, ndim=1] row_to_numpy_d(arma.Row[double]& X) \ cdef numpy.ndarray[numpy.double_t, ndim=1] output = \ numpy.PyArray_SimpleNewFromData(1, &dim, numpy.NPY_DOUBLE, GetMemory(X)) - # Transfer memory ownership. - SetMemState[arma.Row[double]](X, 1) - PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) + # Transfer memory ownership, if needed. + if GetMemState[arma.Row[double]](X) == 0: + SetMemState[arma.Row[double]](X, 1) + PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) return output @@ -171,16 +162,18 @@ cdef numpy.ndarray[numpy.npy_intp, ndim=1] row_to_numpy_s(arma.Row[size_t]& X) \ cdef numpy.ndarray[numpy.npy_intp, ndim=1] output = \ numpy.PyArray_SimpleNewFromData(1, &dim, numpy.NPY_INTP, GetMemory(X)) - # Transfer memory ownership. - SetMemState[arma.Row[size_t]](X, 1) - PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) + # Transfer memory ownership, if needed. + if GetMemState[arma.Row[size_t]](X) == 0: + SetMemState[arma.Row[size_t]](X, 1) + PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) return output cdef arma.Col[double]* numpy_to_col_d(numpy.ndarray[numpy.double_t, ndim=1] X) \ except +: """ - Convert a numpy one-dimensional ndarray to a column vector. + Convert a numpy one-dimensional ndarray to a column vector. The memory will + still be owned by numpy. """ if not (X.flags.c_contiguous or X.flags.owndata): # If needed, make a copy where we own the memory. @@ -189,27 +182,20 @@ cdef arma.Col[double]* numpy_to_col_d(numpy.ndarray[numpy.double_t, ndim=1] X) \ cdef arma.Col[double]* m = new arma.Col[double]( X.data, X.shape[0], False, True) - # Transfer ownership to the Armadillo matrix. - PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) - SetMemState[arma.Col[double]](m[0], 0) - return m cdef arma.Col[size_t]* numpy_to_col_s(numpy.ndarray[numpy.npy_intp, ndim=1] X) \ except +: """ - Convert a numpy one-dimensional ndarray to a column vector. + Convert a numpy one-dimensional ndarray to a column vector. The memory will + still be owned by numpy. """ if not (X.flags.c_contiguous or X.flags.owndata): # If needed, make a copy where we own the memory. X = X.copy(order="C") cdef arma.Col[size_t]* m = new arma.Col[size_t]( X.data, X.shape[0], - False, True) - - # Transfer ownership to the Armadillo matrix. - PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) - SetMemState[arma.Col[size_t]](m[0], 0) + False, False) return m @@ -223,9 +209,10 @@ cdef numpy.ndarray[numpy.double_t, ndim=1] col_to_numpy_d(arma.Col[double]& X) \ cdef numpy.ndarray[numpy.double_t, ndim=1] output = \ numpy.PyArray_SimpleNewFromData(1, &dim, numpy.NPY_DOUBLE, GetMemory(X)) - # Transfer memory ownership. - SetMemState[arma.Col[double]](X, 1) - PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) + # Transfer memory ownership, if needed. + if GetMemState[arma.Col[double]](X) == 0: + SetMemState[arma.Col[double]](X, 1) + PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) return output @@ -239,8 +226,9 @@ cdef numpy.ndarray[numpy.npy_intp, ndim=1] col_to_numpy_s(arma.Col[size_t]& X) \ cdef numpy.ndarray[numpy.npy_intp, ndim=1] output = \ numpy.PyArray_SimpleNewFromData(1, &dim, numpy.NPY_INTP, GetMemory(X)) - # Transfer memory ownership. - SetMemState[arma.Col[size_t]](X, 1) - PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) + # Transfer memory ownership, if needed. + if GetMemState[arma.Col[size_t]](X) == 0: + SetMemState[arma.Col[size_t]](X, 1) + PyArray_ENABLEFLAGS(output, numpy.NPY_OWNDATA) return output diff --git a/src/mlpack/bindings/python/mlpack/arma_util.hpp b/src/mlpack/bindings/python/mlpack/arma_util.hpp index 7770877c3c..c94a51e67f 100644 --- a/src/mlpack/bindings/python/mlpack/arma_util.hpp +++ b/src/mlpack/bindings/python/mlpack/arma_util.hpp @@ -24,6 +24,20 @@ void SetMemState(T& t, int state) const_cast(t.mem_state) = state; } +/** + * Get the memory state of the given Armadillo object. + */ +template +size_t GetMemState(T& t) +{ + // Fake the memory state if we are using preallocated memory---since we will + // end up copying that memory, NumPy can own it. + if (t.mem && t.n_elem <= arma::arma_config::mat_prealloc) + return 0; + + return (size_t) t.mem_state; +} + /** * Return the matrix's allocated memory pointer, unless the matrix is using its * internal preallocated memory, in which case we copy that and return a diff --git a/src/mlpack/bindings/python/mlpack/cli_util.hpp b/src/mlpack/bindings/python/mlpack/cli_util.hpp index 85a9be170e..e1fdcc0c55 100644 --- a/src/mlpack/bindings/python/mlpack/cli_util.hpp +++ b/src/mlpack/bindings/python/mlpack/cli_util.hpp @@ -64,7 +64,8 @@ inline void SetParamWithInfo(const std::string& identifier, // Do we need to find how many categories we have? if (hasCategoricals) { - arma::vec maxs = arma::max(matrix, 1); + arma::vec maxs = arma::max( + std::get<1>(CLI::GetParam(identifier)), 1); for (size_t i = 0; i < dimensions; ++i) { diff --git a/src/mlpack/bindings/python/tests/test_python_binding.py b/src/mlpack/bindings/python/tests/test_python_binding.py index 38ae922364..b95e1a2305 100644 --- a/src/mlpack/bindings/python/tests/test_python_binding.py +++ b/src/mlpack/bindings/python/tests/test_python_binding.py @@ -96,11 +96,12 @@ class TestPythonBinding(unittest.TestCase): and the fifth forgotten. """ x = np.random.rand(100, 5); + z = copy.copy(x) output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - matrix_in=copy.copy(x)) + matrix_in=z) self.assertEqual(output['matrix_out'].shape[0], 100) self.assertEqual(output['matrix_out'].shape[1], 4) @@ -119,11 +120,12 @@ class TestPythonBinding(unittest.TestCase): x = [[1, 2, 3, 4, 5], [6, 7, 8, 9, 10], [11, 12, 13, 14, 15]] + z = copy.copy(x) output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - matrix_in=copy.copy(x)) + matrix_in=z) self.assertEqual(output['matrix_out'].shape[0], 3) self.assertEqual(output['matrix_out'].shape[1], 4) @@ -146,11 +148,12 @@ class TestPythonBinding(unittest.TestCase): Same as testNumpyMatrix() but with an unsigned matrix. """ x = np.random.randint(0, high=500, size=[100, 5]) + z = copy.copy(x) output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - umatrix_in=copy.copy(x)) + umatrix_in=z) self.assertEqual(output['umatrix_out'].shape[0], 100) self.assertEqual(output['umatrix_out'].shape[1], 4) @@ -169,11 +172,12 @@ class TestPythonBinding(unittest.TestCase): x = [[1, 2, 3, 4, 5], [6, 7, 8, 9, 10], [11, 12, 13, 14, 15]] + z = copy.copy(x) output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - umatrix_in=copy.copy(x)) + umatrix_in=z) self.assertEqual(output['umatrix_out'].shape[0], 3) self.assertEqual(output['umatrix_out'].shape[1], 4) @@ -196,11 +200,12 @@ class TestPythonBinding(unittest.TestCase): Test a column vector input parameter. """ x = np.random.rand(100) + z = copy.copy(x) output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - col_in=copy.copy(x)) + col_in=z) self.assertEqual(output['col_out'].shape[0], 100) self.assertEqual(output['col_out'].dtype, np.double) @@ -213,11 +218,12 @@ class TestPythonBinding(unittest.TestCase): Test an unsigned column vector input parameter. """ x = np.random.randint(0, high=500, size=100) + z = copy.copy(x) output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - ucol_in=copy.copy(x)) + ucol_in=z) self.assertEqual(output['ucol_out'].shape[0], 100) self.assertEqual(output['ucol_out'].dtype, np.long) @@ -229,11 +235,12 @@ class TestPythonBinding(unittest.TestCase): Test a row vector input parameter. """ x = np.random.rand(100) + z = copy.copy(x) output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - row_in=copy.copy(x)) + row_in=z) self.assertEqual(output['row_out'].shape[0], 100) self.assertEqual(output['row_out'].dtype, np.double) @@ -246,11 +253,12 @@ class TestPythonBinding(unittest.TestCase): Test an unsigned row vector input parameter. """ x = np.random.randint(0, high=500, size=100) + z = copy.copy(x) output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - urow_in=copy.copy(x)) + urow_in=z) self.assertEqual(output['urow_out'].shape[0], 100) self.assertEqual(output['urow_out'].dtype, np.long) @@ -263,11 +271,12 @@ class TestPythonBinding(unittest.TestCase): Test that we can pass a matrix with all numeric features. """ x = np.random.rand(100, 10) + z = copy.copy(x) output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - matrix_and_info_in=copy.copy(x)) + matrix_and_info_in=z) self.assertEqual(output['matrix_and_info_out'].shape[0], 100) self.assertEqual(output['matrix_and_info_out'].shape[1], 10) @@ -283,12 +292,12 @@ class TestPythonBinding(unittest.TestCase): x = pd.DataFrame(np.random.rand(10, 4), columns=list('abcd')) x['e'] = pd.Series(['a', 'b', 'c', 'd', 'a', 'b', 'e', 'c', 'a', 'b'], dtype='category') - z, d = to_matrix_with_info(x, np.float64) + z = copy.copy(x) output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - matrix_and_info_in=copy.copy(x)) + matrix_and_info_in=z) self.assertEqual(output['matrix_and_info_out'].shape[0], 10) self.assertEqual(output['matrix_and_info_out'].shape[1], 5) @@ -297,10 +306,10 @@ class TestPythonBinding(unittest.TestCase): for i in range(4): for j in range(10): - self.assertEqual(output['matrix_and_info_out'][j, i], z[j, i] * 2) + self.assertEqual(output['matrix_and_info_out'][j, i], z[cols[i]][j] * 2) for j in range(10): - self.assertEqual(output['matrix_and_info_out'][j, 4], z[j, 4] * 2) + self.assertEqual(output['matrix_and_info_out'][j, 4], z[cols[4]][j]) def testIntVector(self): """ From 86ffadc4858f4490feade4dbeebf8603b1e9f7a1 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Wed, 24 Jan 2018 18:43:40 -0500 Subject: [PATCH 04/25] This import is no longer needed. --- src/mlpack/bindings/python/tests/test_python_binding.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mlpack/bindings/python/tests/test_python_binding.py b/src/mlpack/bindings/python/tests/test_python_binding.py index b95e1a2305..93ac2615c6 100644 --- a/src/mlpack/bindings/python/tests/test_python_binding.py +++ b/src/mlpack/bindings/python/tests/test_python_binding.py @@ -10,7 +10,6 @@ import numpy as np import copy from mlpack.test_python_binding import test_python_binding -from mlpack.matrix_utils import to_matrix_with_info class TestPythonBinding(unittest.TestCase): """ From 462737772df44887742ba43ea082960e9a36063c Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 26 Jan 2018 13:44:24 -0500 Subject: [PATCH 05/25] Refactor CLI bindings to hold pointers to models. Specifically, whereas before any serializable type would actually be held as a std::tuple, but now we will hold this as a std::tuple. This also requires some changes for memory handling. Our assumption is that the CLI object will *own* (and delete) any pointers it is holding, so we must also do some extra handling in the end_program.hpp code. --- src/mlpack/bindings/cli/CMakeLists.txt | 2 + src/mlpack/bindings/cli/add_to_po.hpp | 6 +- src/mlpack/bindings/cli/cli_option.hpp | 18 ++++-- src/mlpack/bindings/cli/default_param.hpp | 13 +++- .../bindings/cli/default_param_impl.hpp | 19 +++++- .../bindings/cli/delete_allocated_memory.hpp | 57 ++++++++++++++++++ src/mlpack/bindings/cli/end_program.hpp | 31 ++++++++++ .../bindings/cli/get_allocated_memory.hpp | 59 +++++++++++++++++++ src/mlpack/bindings/cli/get_param.hpp | 14 +++-- .../bindings/cli/get_printable_param.hpp | 15 ++++- .../bindings/cli/get_printable_param_impl.hpp | 19 +++++- .../bindings/cli/get_printable_param_name.hpp | 3 +- .../cli/get_printable_param_value.hpp | 3 +- src/mlpack/bindings/cli/get_raw_param.hpp | 23 ++++++-- .../bindings/cli/map_parameter_name.hpp | 3 +- src/mlpack/bindings/cli/output_param.hpp | 2 +- src/mlpack/bindings/cli/output_param_impl.hpp | 14 ++--- .../bindings/cli/parse_command_line.hpp | 2 +- src/mlpack/bindings/cli/set_param.hpp | 21 ++++++- 19 files changed, 285 insertions(+), 39 deletions(-) create mode 100644 src/mlpack/bindings/cli/delete_allocated_memory.hpp create mode 100644 src/mlpack/bindings/cli/get_allocated_memory.hpp diff --git a/src/mlpack/bindings/cli/CMakeLists.txt b/src/mlpack/bindings/cli/CMakeLists.txt index 095fee3c1a..83c50292ee 100644 --- a/src/mlpack/bindings/cli/CMakeLists.txt +++ b/src/mlpack/bindings/cli/CMakeLists.txt @@ -5,7 +5,9 @@ set(SOURCES cli_option.hpp default_param.hpp default_param_impl.hpp + delete_allocated_memory.hpp end_program.hpp + get_allocated_memory.hpp get_param.hpp get_raw_param.hpp get_printable_param.hpp diff --git a/src/mlpack/bindings/cli/add_to_po.hpp b/src/mlpack/bindings/cli/add_to_po.hpp index 2251649a83..0c3a1d789b 100644 --- a/src/mlpack/bindings/cli/add_to_po.hpp +++ b/src/mlpack/bindings/cli/add_to_po.hpp @@ -89,13 +89,15 @@ void AddToPO(const util::ParamData& d, (boost::program_options::options_description*) output; // Generate the name to be given to boost::program_options. - const std::string mappedName = MapParameterName(d.name); + const std::string mappedName = + MapParameterName::type>(d.name); std::string boostName = (d.alias != '\0') ? mappedName + "," + std::string(1, d.alias) : mappedName; // Note that we have to add the option as type equal to the mapped type, not // the true type of the option. - AddToPO::type>(boostName, d.desc, *desc); + AddToPO::type>::type>( + boostName, d.desc, *desc); } } // namespace cli diff --git a/src/mlpack/bindings/cli/cli_option.hpp b/src/mlpack/bindings/cli/cli_option.hpp index 47233b29ec..7944039628 100644 --- a/src/mlpack/bindings/cli/cli_option.hpp +++ b/src/mlpack/bindings/cli/cli_option.hpp @@ -28,6 +28,8 @@ #include "set_param.hpp" #include "get_printable_param_name.hpp" #include "get_printable_param_value.hpp" +#include "get_allocated_memory.hpp" +#include "delete_allocated_memory.hpp" namespace mlpack { namespace bindings { @@ -88,19 +90,21 @@ class CLIOption data.cppType = cppName; // Apply default value. - if (std::is_same::type>::value) + if (std::is_same::type, + typename ParameterType::type>::type>::value) { data.value = boost::any(defaultValue); } else { - typename ParameterType::type tmp; - data.value = boost::any(std::tuple::type>( - defaultValue, tmp)); + typename ParameterType::type>::type tmp; + data.value = boost::any(std::tuple(defaultValue, tmp)); } const std::string tname = data.tname; - const std::string boostName = MapParameterName(identifier); + const std::string boostName = MapParameterName< + typename std::remove_pointer::type>(identifier); std::string progOptId = (alias[0] != '\0') ? boostName + "," + std::string(1, alias[0]) : boostName; @@ -152,6 +156,10 @@ class CLIOption &GetPrintableParamName; CLI::GetSingleton().functionMap[tname]["GetPrintableParamValue"] = &GetPrintableParamValue; + CLI::GetSingleton().functionMap[tname]["GetAllocatedMemory"] = + &GetAllocatedMemory; + CLI::GetSingleton().functionMap[tname]["DeleteAllocatedMemory"] = + &DeleteAllocatedMemory; } }; diff --git a/src/mlpack/bindings/cli/default_param.hpp b/src/mlpack/bindings/cli/default_param.hpp index 815ee8c3de..74347c9601 100644 --- a/src/mlpack/bindings/cli/default_param.hpp +++ b/src/mlpack/bindings/cli/default_param.hpp @@ -54,10 +54,19 @@ std::string DefaultParamImpl( const util::ParamData& data, const typename boost::enable_if_c< arma::is_arma_type::value || - data::HasSerialize::value || std::is_same>::value>::type* /* junk */ = 0); +/** + * Return the default value of a model option (this returns the default + * filename, or '' if the default is no file). + */ +template +std::string DefaultParamImpl( + const util::ParamData& data, + const typename boost::disable_if>::type* = 0, + const typename boost::enable_if>::type* = 0); + /** * Return the default value of an option. This is the function that will be * placed into the CLI functionMap. @@ -68,7 +77,7 @@ void DefaultParam(const util::ParamData& data, void* output) { std::string* outstr = (std::string*) output; - *outstr = DefaultParamImpl(data); + *outstr = DefaultParamImpl::type>(data); } } // namespace cli diff --git a/src/mlpack/bindings/cli/default_param_impl.hpp b/src/mlpack/bindings/cli/default_param_impl.hpp index ec8f6d4095..377b25ff16 100644 --- a/src/mlpack/bindings/cli/default_param_impl.hpp +++ b/src/mlpack/bindings/cli/default_param_impl.hpp @@ -70,7 +70,6 @@ std::string DefaultParamImpl( const util::ParamData& data, const typename boost::enable_if_c< arma::is_arma_type::value || - data::HasSerialize::value || std::is_same>::value>::type* /* junk */) { @@ -81,6 +80,24 @@ std::string DefaultParamImpl( return "'" + filename + "'"; } +/** + * Return the default value of a model option (this returns the default + * filename, or '' if the default is no file). + */ +template +std::string DefaultParamImpl( + const util::ParamData& data, + const typename boost::disable_if>::type* /* junk */, + const typename boost::enable_if>::type* /* junk */) +{ + // Get the filename and return it, or return an empty string. + typedef std::tuple TupleType; + const TupleType& tuple = *boost::any_cast(&data.value); + const std::string& filename = std::get<1>(tuple); + return "'" + filename + "'"; +} + + } // namespace cli } // namespace bindings } // namespace mlpack diff --git a/src/mlpack/bindings/cli/delete_allocated_memory.hpp b/src/mlpack/bindings/cli/delete_allocated_memory.hpp new file mode 100644 index 0000000000..edc1fcf861 --- /dev/null +++ b/src/mlpack/bindings/cli/delete_allocated_memory.hpp @@ -0,0 +1,57 @@ +/** + * @file delete_allocated_memory.hpp + * @author Ryan Curtin + * + * If any memory has been allocated by the parameter, delete it. + */ +#ifndef MLPACK_BINDINGS_CLI_DELETE_ALLOCATED_MEMORY_HPP +#define MLPACK_BINDINGS_CLI_DELETE_ALLOCATED_MEMORY_HPP + +#include + +namespace mlpack { +namespace bindings { +namespace cli { + +template +void DeleteAllocatedMemoryImpl( + const util::ParamData& /* d */, + const typename boost::disable_if>::type* = 0, + const typename boost::disable_if>::type* = 0) +{ + // Do nothing. +} + +template +void DeleteAllocatedMemoryImpl( + const util::ParamData& /* d */, + const typename boost::enable_if>::type* = 0) +{ + // Do nothing. +} + +template +void DeleteAllocatedMemoryImpl( + const util::ParamData& d, + const typename boost::disable_if>::type* = 0, + const typename boost::enable_if>::type* = 0) +{ + // Delete the allocated memory (hopefully we actually own it). + typedef std::tuple TupleType; + delete std::get<0>(*boost::any_cast(&d.value)); +} + +template +void DeleteAllocatedMemory( + const util::ParamData& d, + const void* /* input */, + void* /* output */) +{ + DeleteAllocatedMemoryImpl::type>(d); +} + +} // namespace cli +} // namespace bindings +} // namespace mlpack + +#endif diff --git a/src/mlpack/bindings/cli/end_program.hpp b/src/mlpack/bindings/cli/end_program.hpp index 4eed33438c..56a3809ebd 100644 --- a/src/mlpack/bindings/cli/end_program.hpp +++ b/src/mlpack/bindings/cli/end_program.hpp @@ -67,6 +67,37 @@ inline void EndProgram() CLI::GetSingleton().timer.PrintTimer(it2.first); } } + + // Lastly clean up any memory. If we are holding any pointers, then we "own" + // them. But we may hold the same pointer twice, so we have to be careful to + // not delete it multiple times. + std::unordered_map memoryAddresses; + it = parameters.begin(); + while (it != parameters.end()) + { + const util::ParamData& data = it->second; + + void* result; + CLI::GetSingleton().functionMap[data.tname]["GetAllocatedMemory"](data, + NULL, (void*) &result); + if (result != NULL && memoryAddresses.count(result) == 0) + memoryAddresses[result] = &data; + + ++it; + } + + // Now we have all the unique addresses that need to be deleted. + std::unordered_map::const_iterator it2; + it2 = memoryAddresses.begin(); + while (it2 != memoryAddresses.end()) + { + const util::ParamData& data = *(it2->second); + + CLI::GetSingleton().functionMap[data.tname]["DeleteAllocatedMemory"](data, + NULL, NULL); + + ++it2; + } } } // namespace cli diff --git a/src/mlpack/bindings/cli/get_allocated_memory.hpp b/src/mlpack/bindings/cli/get_allocated_memory.hpp new file mode 100644 index 0000000000..2dda83e212 --- /dev/null +++ b/src/mlpack/bindings/cli/get_allocated_memory.hpp @@ -0,0 +1,59 @@ +/** + * @file get_allocated_memory.hpp + * @author Ryan Curtin + * + * If the parameter has a type that may need to be deleted, return the address + * of that object. Otherwise return NULL. + */ +#ifndef MLPACK_BINDINGS_CLI_GET_ALLOCATED_MEMORY_HPP +#define MLPACK_BINDINGS_CLI_GET_ALLOCATED_MEMORY_HPP + +#include + +namespace mlpack { +namespace bindings { +namespace cli { + +template +void* GetAllocatedMemory( + const util::ParamData& /* d */, + const typename boost::disable_if>::type* = 0, + const typename boost::disable_if>::type* = 0) +{ + return NULL; +} + +template +void* GetAllocatedMemory( + const util::ParamData& /* d */, + const typename boost::enable_if>::type* = 0) +{ + return NULL; +} + +template +void* GetAllocatedMemory( + const util::ParamData& d, + const typename boost::disable_if>::type* = 0, + const typename boost::enable_if>::type* = 0) +{ + // Here we have a model, which is a tuple, and we need the address of the + // memory. + typedef std::tuple TupleType; + return std::get<0>(*boost::any_cast(&d.value)); +} + +template +void GetAllocatedMemory(const util::ParamData& d, + const void* /* input */, + void* output) +{ + *((void**) output) = + GetAllocatedMemory::type>(d); +} + +} // namespace cli +} // namespace bindings +} // namespace mlpack + +#endif diff --git a/src/mlpack/bindings/cli/get_param.hpp b/src/mlpack/bindings/cli/get_param.hpp index fca9c823fe..9c16b10f26 100644 --- a/src/mlpack/bindings/cli/get_param.hpp +++ b/src/mlpack/bindings/cli/get_param.hpp @@ -95,24 +95,25 @@ T& GetParam( * @param d ParamData object to get parameter value from. */ template -T& GetParam( +T*& GetParam( util::ParamData& d, const typename boost::disable_if>::type* = 0, const typename boost::enable_if>::type* = 0) { // If the model is an input model, we have to load it from file. 'value' // contains the filename. - typedef std::tuple TupleType; + typedef std::tuple TupleType; TupleType* tuple = boost::any_cast(&d.value); const std::string& value = std::get<1>(*tuple); - T& model = std::get<0>(*tuple); if (d.input && !d.loaded) { - data::Load(value, "model", model, true); + T* model = new T(); + data::Load(value, "model", *model, true); d.loaded = true; + std::get<0>(*tuple) = model; } - return model; + return std::get<0>(*tuple); } /** @@ -127,7 +128,8 @@ template void GetParam(const util::ParamData& d, const void* /* input */, void* output) { // Cast to the correct type. - *((T**) output) = &GetParam(const_cast(d)); + *((T**) output) = &GetParam::type>( + const_cast(d)); } } // namespace cli diff --git a/src/mlpack/bindings/cli/get_printable_param.hpp b/src/mlpack/bindings/cli/get_printable_param.hpp index 2b082b338d..5688dc2209 100644 --- a/src/mlpack/bindings/cli/get_printable_param.hpp +++ b/src/mlpack/bindings/cli/get_printable_param.hpp @@ -37,16 +37,24 @@ std::string GetPrintableParam( const typename std::enable_if::value>::type* = 0); /** - * Print a matrix option (this just prints the filename). + * Print a matrix/tuple option (this just prints the filename). */ template std::string GetPrintableParam( const util::ParamData& data, const typename std::enable_if::value || - data::HasSerialize::value || std::is_same>::value>::type* = 0); +/** + * Print a model option (this just prints the filename). + */ +template +std::string GetPrintableParam( + const util::ParamData& data, + const typename boost::disable_if>::type* = 0, + const typename boost::enable_if>::type* = 0); + /** * Print an option into a std::string. This should print a short, one-line * representation of the object. The string will be stored in the output @@ -57,7 +65,8 @@ void GetPrintableParam(const util::ParamData& data, const void* /* input */, void* output) { - *((std::string*) output) = GetPrintableParam(data); + *((std::string*) output) = + GetPrintableParam::type>(data); } } // namespace cli diff --git a/src/mlpack/bindings/cli/get_printable_param_impl.hpp b/src/mlpack/bindings/cli/get_printable_param_impl.hpp index 0e55cb649a..6fb65bc48d 100644 --- a/src/mlpack/bindings/cli/get_printable_param_impl.hpp +++ b/src/mlpack/bindings/cli/get_printable_param_impl.hpp @@ -43,12 +43,11 @@ std::string GetPrintableParam( return oss.str(); } -//! Print a matrix/model/tuple option (this just prints the filename). +//! Print a matrix/tuple option (this just prints the filename). template std::string GetPrintableParam( const util::ParamData& data, const typename std::enable_if::value || - data::HasSerialize::value || std::is_same>::value>::type* /* junk */) { @@ -61,6 +60,22 @@ std::string GetPrintableParam( return oss.str(); } +//! Print a model option (this just prints the filename). +template +std::string GetPrintableParam( + const util::ParamData& data, + const typename boost::disable_if>::type* /* junk */, + const typename boost::enable_if>::type* /* junk */) +{ + // Extract the string from the tuple that's being held. + typedef std::tuple::type> TupleType; + const TupleType* tuple = boost::any_cast(&data.value); + + std::ostringstream oss; + oss << std::get<1>(*tuple); + return oss.str(); +} + } // namespace cli } // namespace bindings } // namespace mlpack diff --git a/src/mlpack/bindings/cli/get_printable_param_name.hpp b/src/mlpack/bindings/cli/get_printable_param_name.hpp index 12c7f92cfd..e323d2378a 100644 --- a/src/mlpack/bindings/cli/get_printable_param_name.hpp +++ b/src/mlpack/bindings/cli/get_printable_param_name.hpp @@ -64,7 +64,8 @@ void GetPrintableParamName( const void* /* input */, void* output) { - *((std::string*) output) = GetPrintableParamName(d); + *((std::string*) output) = + GetPrintableParamName::type>(d); } } // namespace cli diff --git a/src/mlpack/bindings/cli/get_printable_param_value.hpp b/src/mlpack/bindings/cli/get_printable_param_value.hpp index be29934d51..0110c0dfdc 100644 --- a/src/mlpack/bindings/cli/get_printable_param_value.hpp +++ b/src/mlpack/bindings/cli/get_printable_param_value.hpp @@ -68,7 +68,8 @@ void GetPrintableParamValue( const void* input, void* output) { - *((std::string*) output) = GetPrintableParamValue(d, + *((std::string*) output) = + GetPrintableParamValue::type>(d, *((std::string*) input)); } diff --git a/src/mlpack/bindings/cli/get_raw_param.hpp b/src/mlpack/bindings/cli/get_raw_param.hpp index ae73ca0d36..a397ffff54 100644 --- a/src/mlpack/bindings/cli/get_raw_param.hpp +++ b/src/mlpack/bindings/cli/get_raw_param.hpp @@ -40,15 +40,29 @@ T& GetRawParam( const typename boost::enable_if_c< arma::is_arma_type::value || std::is_same>::value || - data::HasSerialize::value>::type* = 0) + arma::mat>>::value>::type* = 0) { - // Don't load the matrix/model. + // Don't load the matrix. typedef std::tuple TupleType; T& value = std::get<0>(*boost::any_cast(&d.value)); return value; } +/** + * Return the name of a model parameter. + */ +template +T*& GetRawParam( + util::ParamData& d, + const typename boost::disable_if>::type* = 0, + const typename boost::enable_if>::type* = 0) +{ + // Don't load the model. + typedef std::tuple TupleType; + T*& value = std::get<0>(*boost::any_cast(&d.value)); + return value; +} + /** * Return a parameter casted to the given type. Type checking does not happen * here! @@ -63,7 +77,8 @@ void GetRawParam(const util::ParamData& d, void* output) { // Cast to the correct type. - *((T**) output) = &GetRawParam(const_cast(d)); + *((T**) output) = &GetRawParam::type>( + const_cast(d)); } } // namespace cli diff --git a/src/mlpack/bindings/cli/map_parameter_name.hpp b/src/mlpack/bindings/cli/map_parameter_name.hpp index f75f3ad4af..2d37d6c384 100644 --- a/src/mlpack/bindings/cli/map_parameter_name.hpp +++ b/src/mlpack/bindings/cli/map_parameter_name.hpp @@ -61,7 +61,8 @@ void MapParameterName(const util::ParamData& d, { // Store the mapped name in the output pointer, which is actually a string // pointer. - *((std::string*) output) = MapParameterName(d.name); + *((std::string*) output) = + MapParameterName::type>(d.name); } } // namespace cli diff --git a/src/mlpack/bindings/cli/output_param.hpp b/src/mlpack/bindings/cli/output_param.hpp index c4ce9c3b5d..4ed052d55f 100644 --- a/src/mlpack/bindings/cli/output_param.hpp +++ b/src/mlpack/bindings/cli/output_param.hpp @@ -70,7 +70,7 @@ void OutputParam(const util::ParamData& data, const void* /* input */, void* /* output */) { - OutputParamImpl(data); + OutputParamImpl::type>(data); } } // namespace cli diff --git a/src/mlpack/bindings/cli/output_param_impl.hpp b/src/mlpack/bindings/cli/output_param_impl.hpp index 3e30a66ff8..a85e55a941 100644 --- a/src/mlpack/bindings/cli/output_param_impl.hpp +++ b/src/mlpack/bindings/cli/output_param_impl.hpp @@ -55,10 +55,10 @@ void OutputParamImpl( if (output.n_elem > 0 && filename != "") { - if (arma::is_Row::value || arma::is_Col::value) - data::Save(filename, output, false); - else - data::Save(filename, output, false, !data.noTranspose); + if (arma::is_Row::value || arma::is_Col::value) + data::Save(filename, output, false); + else + data::Save(filename, output, false, !data.noTranspose); } } @@ -72,14 +72,14 @@ void OutputParamImpl( // The const cast is necessary here because Serialize() can't ever be marked // const. In this case we can assume it though, since we will be saving and // not loading. - typedef std::tuple TupleType; - T& output = const_cast(std::get<0>(*boost::any_cast( + typedef std::tuple TupleType; + T*& output = const_cast(std::get<0>(*boost::any_cast( &data.value))); const std::string& filename = std::get<1>(*boost::any_cast(&data.value)); if (filename != "") - data::Save(filename, "model", output); + data::Save(filename, "model", *output); } //! Output a mapped dataset. diff --git a/src/mlpack/bindings/cli/parse_command_line.hpp b/src/mlpack/bindings/cli/parse_command_line.hpp index 3e0b959eaf..b7f7cac0de 100644 --- a/src/mlpack/bindings/cli/parse_command_line.hpp +++ b/src/mlpack/bindings/cli/parse_command_line.hpp @@ -52,7 +52,7 @@ void ParseCommandLine(int argc, char** argv) boostNameMap[boostName] = d.name; } - // TODO: we have to mark somehow that we parsed. + // Mark that we did parsing. CLI::GetSingleton().didParse = true; // Parse the command line, then place the values in the right place. diff --git a/src/mlpack/bindings/cli/set_param.hpp b/src/mlpack/bindings/cli/set_param.hpp index fb1fc0efce..64103135c5 100644 --- a/src/mlpack/bindings/cli/set_param.hpp +++ b/src/mlpack/bindings/cli/set_param.hpp @@ -54,7 +54,6 @@ void SetParam( util::ParamData& d, const boost::any& value, const typename std::enable_if::value || - data::HasSerialize::value || std::is_same>::value>::type* = 0) { @@ -64,6 +63,23 @@ void SetParam( std::get<1>(tuple) = boost::any_cast(value); } +/** + * Set a serializable object. This sets the filename referring to the + * parameter. + */ +template +void SetParam( + util::ParamData& d, + const boost::any& value, + const typename boost::disable_if>::type* = 0, + const typename boost::enable_if>::type* = 0) +{ + // We're setting the string filename. + typedef std::tuple::type> TupleType; + TupleType& tuple = *boost::any_cast(&d.value); + std::get<1>(tuple) = boost::any_cast(value); +} + /** * Return a parameter casted to the given type. Type checking does not happen * here! @@ -75,7 +91,8 @@ void SetParam( template void SetParam(const util::ParamData& d, const void* input, void* /* output */) { - SetParam(const_cast(d), *((boost::any*) input)); + SetParam::type>( + const_cast(d), *((boost::any*) input)); } } // namespace cli From 84427e1fe2bd2389aeae73c954c78e8d4617b300 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 26 Jan 2018 13:55:14 -0500 Subject: [PATCH 06/25] Fix memory handling of numpy arrays. If we make a copy during the conversion, then Armadillo should own the memory (otherwise Python will delete the temporary). --- .../bindings/python/mlpack/arma_numpy.pxd | 25 ++++---- .../bindings/python/mlpack/arma_numpy.pyx | 60 +++++++++++++++---- .../bindings/python/mlpack/matrix_utils.py | 27 +++++++-- 3 files changed, 82 insertions(+), 30 deletions(-) diff --git a/src/mlpack/bindings/python/mlpack/arma_numpy.pxd b/src/mlpack/bindings/python/mlpack/arma_numpy.pxd index 167810cb68..a0e9b8547e 100644 --- a/src/mlpack/bindings/python/mlpack/arma_numpy.pxd +++ b/src/mlpack/bindings/python/mlpack/arma_numpy.pxd @@ -19,14 +19,15 @@ import numpy numpy.import_array() cimport arma +from libcpp cimport bool """ Convert a numpy ndarray to a matrix. """ -cdef arma.Mat[double]* numpy_to_mat_d(numpy.ndarray[numpy.double_t, ndim=2] X) \ - except + -cdef arma.Mat[size_t]* numpy_to_mat_s(numpy.ndarray[numpy.npy_intp, ndim=2] X) \ - except + +cdef arma.Mat[double]* numpy_to_mat_d(numpy.ndarray[numpy.double_t, ndim=2] X, \ + bool takeOwnership) except + +cdef arma.Mat[size_t]* numpy_to_mat_s(numpy.ndarray[numpy.npy_intp, ndim=2] X, \ + bool takeOwnership) except + """ Convert an Armadillo object to a numpy ndarray of the given type. @@ -39,10 +40,10 @@ cdef numpy.ndarray[numpy.npy_intp, ndim=2] mat_to_numpy_s(arma.Mat[size_t]& X) \ """ Convert a numpy one-dimensional ndarray to a row of the given type. """ -cdef arma.Row[double]* numpy_to_row_d(numpy.ndarray[numpy.double_t, ndim=1] X) \ - except + -cdef arma.Row[size_t]* numpy_to_row_s(numpy.ndarray[numpy.npy_intp, ndim=1] X) \ - except + +cdef arma.Row[double]* numpy_to_row_d(numpy.ndarray[numpy.double_t, ndim=1] X, \ + bool takeOwnership) except + +cdef arma.Row[size_t]* numpy_to_row_s(numpy.ndarray[numpy.npy_intp, ndim=1] X, \ + bool takeOwnership) except + """ Convert an Armadillo row vector to a one-dimensional numpy ndarray of the @@ -56,10 +57,10 @@ cdef numpy.ndarray[numpy.npy_intp, ndim=1] row_to_numpy_s(arma.Row[size_t]& X) \ """ Convert a numpy one-dimensional ndarray to a column vector of the given type. """ -cdef arma.Col[double]* numpy_to_col_d(numpy.ndarray[numpy.double_t, ndim=1] X) \ - except + -cdef arma.Col[size_t]* numpy_to_col_s(numpy.ndarray[numpy.npy_intp, ndim=1] X) \ - except + +cdef arma.Col[double]* numpy_to_col_d(numpy.ndarray[numpy.double_t, ndim=1] X, \ + bool takeOwnership) except + +cdef arma.Col[size_t]* numpy_to_col_s(numpy.ndarray[numpy.npy_intp, ndim=1] X, \ + bool takeOwnership) except + """ Convert an Armadillo column vector to a one-dimensional numpy ndarray of the diff --git a/src/mlpack/bindings/python/mlpack/arma_numpy.pyx b/src/mlpack/bindings/python/mlpack/arma_numpy.pyx index 40508d17a1..dcf6908d94 100644 --- a/src/mlpack/bindings/python/mlpack/arma_numpy.pyx +++ b/src/mlpack/bindings/python/mlpack/arma_numpy.pyx @@ -24,6 +24,7 @@ import numpy numpy.import_array() cimport arma +from libcpp cimport bool cdef extern from "numpy/arrayobject.h": void PyArray_ENABLEFLAGS(numpy.ndarray arr, int flags) @@ -39,32 +40,44 @@ cdef extern from "": size_t* GetMemory(arma.Col[size_t]& m) size_t* GetMemory(arma.Row[size_t]& m) -cdef arma.Mat[double]* numpy_to_mat_d(numpy.ndarray[numpy.double_t, ndim=2] X) \ - except +: +cdef arma.Mat[double]* numpy_to_mat_d(numpy.ndarray[numpy.double_t, ndim=2] X, \ + bool takeOwnership) except +: """ Convert a numpy ndarray to a matrix. The memory will still be owned by numpy. """ if not (X.flags.c_contiguous or X.flags.owndata): # If needed, make a copy where we own the memory. X = X.copy(order="C") + takeOwnership = True cdef arma.Mat[double]* m = new arma.Mat[double]( X.data, X.shape[1],\ X.shape[0], False, False) + # Take ownership of the memory, if we need to. + if takeOwnership: + PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) + SetMemState[arma.Mat[double]](m[0], 0) + return m -cdef arma.Mat[size_t]* numpy_to_mat_s(numpy.ndarray[numpy.npy_intp, ndim=2] X) \ - except +: +cdef arma.Mat[size_t]* numpy_to_mat_s(numpy.ndarray[numpy.npy_intp, ndim=2] X, \ + bool takeOwnership) except +: """ Convert a numpy ndarray to a matrix. The memory will still be owned by numpy. """ if not (X.flags.c_contiguous or X.flags.owndata): # If needed, make a copy where we own the memory. X = X.copy(order="C") + takeOwnership = True cdef arma.Mat[size_t]* m = new arma.Mat[size_t]( X.data, X.shape[1], X.shape[0], False, False) + # Take ownership of the memory, if we need to. + if takeOwnership: + PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) + SetMemState[arma.Mat[size_t]](m[0], 0) + return m cdef numpy.ndarray[numpy.double_t, ndim=2] mat_to_numpy_d(arma.Mat[double]& X) \ @@ -105,8 +118,8 @@ cdef numpy.ndarray[numpy.npy_intp, ndim=2] mat_to_numpy_s(arma.Mat[size_t]& X) \ return output -cdef arma.Row[double]* numpy_to_row_d(numpy.ndarray[numpy.double_t, ndim=1] X) \ - except +: +cdef arma.Row[double]* numpy_to_row_d(numpy.ndarray[numpy.double_t, ndim=1] X, \ + bool takeOwnership) except +: """ Convert a numpy one-dimensional ndarray to a row. The memory will still be owned by numpy. @@ -114,14 +127,20 @@ cdef arma.Row[double]* numpy_to_row_d(numpy.ndarray[numpy.double_t, ndim=1] X) \ if not (X.flags.c_contiguous or X.flags.owndata): # If needed, make a copy where we own the memory. X = X.copy(order="C") + takeOwnership = True cdef arma.Row[double]* m = new arma.Row[double]( X.data, X.shape[0], False, False) + # Transfer memory ownership, if needed. + if takeOwnership: + PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) + SetMemState[arma.Row[double]](m[0], 0) + return m -cdef arma.Row[size_t]* numpy_to_row_s(numpy.ndarray[numpy.npy_intp, ndim=1] X) \ - except +: +cdef arma.Row[size_t]* numpy_to_row_s(numpy.ndarray[numpy.npy_intp, ndim=1] X, \ + bool takeOwnership) except +: """ Convert a numpy one-dimensional ndarray to a row. The memory will still be owned by numpy. @@ -129,10 +148,16 @@ cdef arma.Row[size_t]* numpy_to_row_s(numpy.ndarray[numpy.npy_intp, ndim=1] X) \ if not (X.flags.c_contiguous or X.flags.owndata): # If needed, make a copy where we own the memory. X = X.copy(order="C") + takeOwnership = True cdef arma.Row[size_t]* m = new arma.Row[size_t]( X.data, X.shape[0], False, False) + # Transfer memory ownership, if needed. + if takeOwnership: + PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) + SetMemState[arma.Row[size_t]](m[0], 0) + return m cdef numpy.ndarray[numpy.double_t, ndim=1] row_to_numpy_d(arma.Row[double]& X) \ @@ -169,8 +194,8 @@ cdef numpy.ndarray[numpy.npy_intp, ndim=1] row_to_numpy_s(arma.Row[size_t]& X) \ return output -cdef arma.Col[double]* numpy_to_col_d(numpy.ndarray[numpy.double_t, ndim=1] X) \ - except +: +cdef arma.Col[double]* numpy_to_col_d(numpy.ndarray[numpy.double_t, ndim=1] X, \ + bool takeOwnership) except +: """ Convert a numpy one-dimensional ndarray to a column vector. The memory will still be owned by numpy. @@ -178,14 +203,20 @@ cdef arma.Col[double]* numpy_to_col_d(numpy.ndarray[numpy.double_t, ndim=1] X) \ if not (X.flags.c_contiguous or X.flags.owndata): # If needed, make a copy where we own the memory. X = X.copy(order="C") + takeOwnership = True cdef arma.Col[double]* m = new arma.Col[double]( X.data, X.shape[0], False, True) + # Transfer memory ownership, if needed. + if takeOwnership: + PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) + SetMemState[arma.Col[double]](m[0], 0) + return m -cdef arma.Col[size_t]* numpy_to_col_s(numpy.ndarray[numpy.npy_intp, ndim=1] X) \ - except +: +cdef arma.Col[size_t]* numpy_to_col_s(numpy.ndarray[numpy.npy_intp, ndim=1] X, \ + bool takeOwnership) except +: """ Convert a numpy one-dimensional ndarray to a column vector. The memory will still be owned by numpy. @@ -197,6 +228,11 @@ cdef arma.Col[size_t]* numpy_to_col_s(numpy.ndarray[numpy.npy_intp, ndim=1] X) \ cdef arma.Col[size_t]* m = new arma.Col[size_t]( X.data, X.shape[0], False, False) + # Transfer memory ownership, if needed. + if takeOwnership: + PyArray_CLEARFLAGS(X, numpy.NPY_OWNDATA) + SetMemState[arma.Col[size_t]](m[0], 0) + return m cdef numpy.ndarray[numpy.double_t, ndim=1] col_to_numpy_d(arma.Col[double]& X) \ diff --git a/src/mlpack/bindings/python/mlpack/matrix_utils.py b/src/mlpack/bindings/python/mlpack/matrix_utils.py index 04c9d4778d..9cf0cbadda 100644 --- a/src/mlpack/bindings/python/mlpack/matrix_utils.py +++ b/src/mlpack/bindings/python/mlpack/matrix_utils.py @@ -48,9 +48,9 @@ def to_matrix(x, dtype=np.double): raise TypeError("given argument is not array-like") if (isinstance(x, np.ndarray) and x.dtype == dtype and x.flags.c_contiguous): - return x + return x, False else: - return np.array(x, copy=True, dtype=dtype, order='C') + return np.array(x, copy=True, dtype=dtype, order='C'), True def to_matrix_with_info(x, dtype): """ @@ -66,7 +66,7 @@ def to_matrix_with_info(x, dtype): if isinstance(x, np.ndarray): # It is already an ndarray, so the vector of info is all 0s (all numeric). d = np.zeros([x.shape[1]], dtype=np.bool) - return (x, d) + return (x, False, d) if isinstance(x, pd.DataFrame) or isinstance(x, pd.Series): # It's a pandas dataframe. So we need to see if any of the dtypes are @@ -79,8 +79,9 @@ def to_matrix_with_info(x, dtype): not np.dtype(str) in dtype_array and \ not np.dtype(unicode) in dtype_array: # We can just return the matrix as-is; it's all numeric. + t = to_matrix(x) d = np.zeros([x.shape[1]], dtype=np.bool) - return (to_matrix(x), d) + return (t[0], t[1], d) if np.dtype(str) in dtype_array or np.dtype(unicode) in dtype_array: raise TypeError('cannot convert matrices with string types') @@ -115,7 +116,10 @@ def to_matrix_with_info(x, dtype): catColumnIndices = [y.columns.get_loc(i) for i in catColumns] d[catColumnIndices] = 1 - return (to_matrix(y.apply(pd.to_numeric)), d) + # We'll have to force the second part of the tuple (whether or not to take + # ownership) to true. + t = to_matrix(y.apply(pd.to_numeric)) + return (t[0], True, d) if isinstance(x, list): # Get the number of dimensions. @@ -126,7 +130,18 @@ def to_matrix_with_info(x, dtype): dims = len(x) d = np.zeros([dims]) - return (np.array(x, dtype=dtype), d) + out = np.array(x, dtype=dtype, copy=False) # Try to avoid copy... + + # Since we don't have a great way to check if these are using the same + # memory location, we will probe manually (ugh). + oldval = x[0] + x[0] *= 2 + alias = False + if out[0] == x[0]: + alias = True + x[0] = oldval + + return (np.array(x, dtype=dtype), not alias, d) # If we got here, the type is not known. raise TypeError("given matrix is not a numpy ndarray or pandas DataFrame or "\ From 7903a0ed398dd08cc789ce3ffacde3aa606373ac Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 26 Jan 2018 13:56:48 -0500 Subject: [PATCH 07/25] Make Python bindings hold pointers. This refactors the Python bindings to deal with holding pointers to serializable types. Some minor extra work is necessary to prevent Python from accidentally deleting models multiple times. --- .../bindings/python/get_cython_type.hpp | 2 +- src/mlpack/bindings/python/get_param.hpp | 12 ++- .../bindings/python/get_printable_param.hpp | 5 +- src/mlpack/bindings/python/import_decl.hpp | 2 +- src/mlpack/bindings/python/mlpack/cli.pxd | 7 +- .../bindings/python/mlpack/cli_util.hpp | 25 +++++ src/mlpack/bindings/python/mlpack/move.hpp | 30 ------ .../bindings/python/print_class_defn.hpp | 2 +- src/mlpack/bindings/python/print_doc.hpp | 3 +- .../python/print_input_processing.hpp | 61 ++++++------ .../python/print_output_processing.hpp | 94 ++++++++++++++++--- src/mlpack/bindings/python/print_pyx.cpp | 4 +- 12 files changed, 163 insertions(+), 84 deletions(-) delete mode 100644 src/mlpack/bindings/python/mlpack/move.hpp diff --git a/src/mlpack/bindings/python/get_cython_type.hpp b/src/mlpack/bindings/python/get_cython_type.hpp index f5ae807abe..8e95dc2d9d 100644 --- a/src/mlpack/bindings/python/get_cython_type.hpp +++ b/src/mlpack/bindings/python/get_cython_type.hpp @@ -113,7 +113,7 @@ inline std::string GetCythonType( const typename boost::disable_if>::type* = 0, const typename boost::enable_if>::type* = 0) { - return d.cppType; + return d.cppType + "*"; } } // namespace python diff --git a/src/mlpack/bindings/python/get_param.hpp b/src/mlpack/bindings/python/get_param.hpp index 62c898a15e..e2c1c33108 100644 --- a/src/mlpack/bindings/python/get_param.hpp +++ b/src/mlpack/bindings/python/get_param.hpp @@ -22,7 +22,17 @@ void GetParam(const util::ParamData& d, const void* /* input */, void* output) { - *((T**) output) = const_cast(boost::any_cast(&d.value)); +// typedef typename std::remove_pointer::type TRaw; +// if (std::is_pointer::value) // If true, this is a model. +// { +// std::cout << "get a raw pointer for " << d.name << ": " << boost::any_cast(d.value) << +//"\n"; +// *((TRaw***) output) = const_cast(boost::any_cast(&d.value)); +// } +// else + { + *((T**) output) = const_cast(boost::any_cast(&d.value)); + } } } // namespace python diff --git a/src/mlpack/bindings/python/get_printable_param.hpp b/src/mlpack/bindings/python/get_printable_param.hpp index b7258bc7c8..0c21dd5a61 100644 --- a/src/mlpack/bindings/python/get_printable_param.hpp +++ b/src/mlpack/bindings/python/get_printable_param.hpp @@ -73,7 +73,7 @@ std::string GetPrintableParam( const typename boost::enable_if>::type* = 0) { std::ostringstream oss; - oss << data.cppType << " model"; + oss << data.cppType << " model at " << boost::any_cast(data.value); return oss.str(); } @@ -110,7 +110,8 @@ void GetPrintableParam(const util::ParamData& data, const void* /* input */, void* output) { - *((std::string*) output) = GetPrintableParam(data); + *((std::string*) output) = + GetPrintableParam::type>(data); } } // namespace python diff --git a/src/mlpack/bindings/python/import_decl.hpp b/src/mlpack/bindings/python/import_decl.hpp index b638ecde42..52d1026340 100644 --- a/src/mlpack/bindings/python/import_decl.hpp +++ b/src/mlpack/bindings/python/import_decl.hpp @@ -79,7 +79,7 @@ void ImportDecl(const util::ParamData& d, const void* indent, void* /* output */) { - ImportDecl(d, *((size_t*) indent)); + ImportDecl::type>(d, *((size_t*) indent)); } } // namespace python diff --git a/src/mlpack/bindings/python/mlpack/cli.pxd b/src/mlpack/bindings/python/mlpack/cli.pxd index 860bc4a756..f6c6435c18 100644 --- a/src/mlpack/bindings/python/mlpack/cli.pxd +++ b/src/mlpack/bindings/python/mlpack/cli.pxd @@ -38,15 +38,12 @@ cdef extern from "" namespace "mlpack" nogil: cdef extern from "" \ namespace "mlpack::util" nogil: void SetParam[T](string, T&) nogil except + + void SetParamPtr[T](string, T*) nogil except + void SetParamWithInfo[T](string, T&, const bool*) nogil except + + (T*) GetParamPtr[T](string) nogil except + (T&) GetParamWithInfo[T](string) nogil except + void EnableVerbose() nogil except + void DisableVerbose() nogil except + void DisableBacktrace() nogil except + void ResetTimers() nogil except + void EnableTimers() nogil except + - -cdef extern from "" \ - namespace "mlpack::util" nogil: - void MoveFromPtr[T](T&, T*) nogil except + - void MoveToPtr[T](T*, T&) nogil except + diff --git a/src/mlpack/bindings/python/mlpack/cli_util.hpp b/src/mlpack/bindings/python/mlpack/cli_util.hpp index e1fdcc0c55..6c4ef5ff7b 100644 --- a/src/mlpack/bindings/python/mlpack/cli_util.hpp +++ b/src/mlpack/bindings/python/mlpack/cli_util.hpp @@ -34,6 +34,21 @@ inline void SetParam(const std::string& identifier, T& value) CLI::GetParam(identifier) = std::move(value); } +/** + * Set the parameter to the given value, given that the type is a pointer. + * + * This function exists to work around both Cython's lack of support for lvalue + * references and also its seeming lack of support for template pointer types. + * + * @param identifier Name of parameter. + * @param value Value to set parameter to. + */ +template +inline void SetParamPtr(const std::string& identifier, T* value) +{ + CLI::GetParam(identifier) = value; +} + /** * Set the parameter (which is a matrix/DatasetInfo tuple) to the given value. */ @@ -83,6 +98,16 @@ inline void SetParamWithInfo(const std::string& identifier, } } +/** + * Return a pointer. This function exists to work around Cython's seeming lack + * of support for template pointer types. + */ +template +T* GetParamPtr(const std::string& paramName) +{ + return CLI::GetParam(paramName); +} + /** * Return the matrix part of a matrix + dataset info parameter. */ diff --git a/src/mlpack/bindings/python/mlpack/move.hpp b/src/mlpack/bindings/python/mlpack/move.hpp deleted file mode 100644 index b4a145b2d7..0000000000 --- a/src/mlpack/bindings/python/mlpack/move.hpp +++ /dev/null @@ -1,30 +0,0 @@ -/** - * @file move.hpp - * @author Ryan Curtin - * - * Utility function for Cython to use std::move. - */ -#ifndef MLPACK_BINDINGS_PYTHON_CYTHON_MOVE_HPP -#define MLPACK_BINDINGS_PYTHON_CYTHON_MOVE_HPP - -#include - -namespace mlpack { -namespace util { - -template -void MoveToPtr(T* dest, T& src) -{ - *(dest) = std::move(src); -} - -template -void MoveFromPtr(T& dest, T* src) -{ - dest = std::move(*src); -} - -} // namespace util -} // namespace mlpack - -#endif diff --git a/src/mlpack/bindings/python/print_class_defn.hpp b/src/mlpack/bindings/python/print_class_defn.hpp index 77148531b9..8bb4e0fdc2 100644 --- a/src/mlpack/bindings/python/print_class_defn.hpp +++ b/src/mlpack/bindings/python/print_class_defn.hpp @@ -108,7 +108,7 @@ void PrintClassDefn(const util::ParamData& d, const void* /* input */, void* /* output */) { - PrintClassDefn(d); + PrintClassDefn::type>(d); } } // namespace python diff --git a/src/mlpack/bindings/python/print_doc.hpp b/src/mlpack/bindings/python/print_doc.hpp index 08cbebdf26..353e558a9a 100644 --- a/src/mlpack/bindings/python/print_doc.hpp +++ b/src/mlpack/bindings/python/print_doc.hpp @@ -39,7 +39,8 @@ void PrintDoc(const util::ParamData& d, oss << d.name << "_ ("; else oss << d.name << " ("; - oss << GetPythonType(d) << "): " << d.desc; + oss << GetPythonType::type>(d) << "): " + << d.desc; // Print a default, if possible. if (!d.required) diff --git a/src/mlpack/bindings/python/print_input_processing.hpp b/src/mlpack/bindings/python/print_input_processing.hpp index 9982449fa9..14d487b3fb 100644 --- a/src/mlpack/bindings/python/print_input_processing.hpp +++ b/src/mlpack/bindings/python/print_input_processing.hpp @@ -104,7 +104,9 @@ void PrintInputProcessing( * * # Detect if the parameter was passed; set if so. * if param_name is not None: - * param_name_mat = arma_numpy.numpy_to_mat_d(param_name) + * param_name_tuple = to_matrix(param_name) + * param_name_mat = arma_numpy.numpy_to_mat_d(param_name_tuple[0], + * param_name_tuple[1]) * SetParam[mat]( 'param_name', dereference(param_name_mat)) * CLI.SetPassed( 'param_name') */ @@ -114,10 +116,12 @@ void PrintInputProcessing( { std::cout << prefix << "if " << d.name << " is not None:" << std::endl; + std::cout << prefix << " " << d.name << "_tuple = to_matrix(" << d.name + << ", dtype=" << GetNumpyType() << ")" + << std::endl; std::cout << prefix << " " << d.name << "_mat = arma_numpy.numpy_to_" - << GetArmaType() << "_" << GetNumpyTypeChar() << "(to_matrix(" - << d.name << ", " << "dtype=" << GetNumpyType() - << "))" << std::endl; + << GetArmaType() << "_" << GetNumpyTypeChar() << "(" << d.name + << "_tuple[0], " << d.name << "_tuple[1])" << std::endl; std::cout << prefix << " SetParam[" << GetCythonType(d) << "]( '" << d.name << "', dereference(" << d.name << "_mat))" << std::endl; @@ -127,10 +131,12 @@ void PrintInputProcessing( } else { + std::cout << prefix << d.name << "_tuple = to_matrix(" << d.name + << ", dtype=" << GetNumpyType() << ")" + << std::endl; std::cout << prefix << d.name << "_mat = arma_numpy.numpy_to_" - << GetArmaType() << "_" << GetNumpyTypeChar() << "(to_matrix(" - << d.name << ", " << "dtype=" << GetNumpyType() - << "))" << std::endl; + << GetArmaType() << "_" << GetNumpyTypeChar() << "(" << d.name + << "_tuple[0], " << d.name << "_tuple[1])" << std::endl; std::cout << prefix << "SetParam[" << GetCythonType(d) << "]( '" << d.name << "', dereference(" << d.name << "_mat))" << std::endl; @@ -163,12 +169,10 @@ void PrintInputProcessing( * # Detect if the parameter was passed; set if so. * if param_name is not None: * try: - * MoveFromPtr[Model](CLI.GetParam[Model]('param_name'), - * ( param_name).modelptr) + * SetParamPtr[Model]('param_name', ( param_name).modelptr) * except TypeError as e: * if type(param_name).__name__ == "ModelType": - * MoveFromPtr[Model](CLI.GetParam[Model]('param_name'), - * ( param_name).modelptr) + * SetParamPtr[Model]('param_name', ( param_name).modelptr) * else: * raise e * CLI.SetPassed( 'param_name') @@ -179,15 +183,15 @@ void PrintInputProcessing( { std::cout << prefix << "if " << d.name << " is not None:" << std::endl; std::cout << prefix << " try:" << std::endl; - std::cout << prefix << " MoveFromPtr[" << strippedType - << "](CLI.GetParam[" << strippedType << "]('" << d.name << "'), (<" - << strippedType << "Type?> " << d.name << ").modelptr)" << std::endl; + std::cout << prefix << " SetParamPtr[" << strippedType << "]('" << d.name + << "', (<" << strippedType << "Type?> " << d.name << ").modelptr)" + << std::endl; std::cout << prefix << " except TypeError as e:" << std::endl; std::cout << prefix << " if type(" << d.name << ").__name__ == '" << strippedType << "Type':" << std::endl; - std::cout << prefix << " MoveFromPtr[" << strippedType - << "](CLI.GetParam[" << strippedType << "]('" << d.name << "'), (<" - << strippedType << "Type> " << d.name << ").modelptr)" << std::endl; + std::cout << prefix << " SetParamPtr[" << strippedType << "]('" << d.name + << "', (<" << strippedType << "Type> " << d.name << ").modelptr)" + << std::endl; std::cout << prefix << " else:" << std::endl; std::cout << prefix << " raise e" << std::endl; std::cout << prefix << " CLI.SetPassed( '" << d.name << "')" @@ -196,15 +200,15 @@ void PrintInputProcessing( else { std::cout << prefix << "try:" << std::endl; - std::cout << prefix << " MoveFromPtr[" << strippedType << "](CLI.GetParam[" - << strippedType << "]('" << d.name << "'), (<" << strippedType - << "Type?> " << d.name << ").modelptr)" << std::endl; + std::cout << prefix << " SetParamPtr[" << strippedType << "]('" << d.name + << "', (<" << strippedType << "Type?> " << d.name << ").modelptr)" + << std::endl; std::cout << prefix << "except TypeError as e:" << std::endl; std::cout << prefix << " if type(" << d.name << ").__name__ == '" << strippedType << "Type':" << std::endl; - std::cout << prefix << " MoveFromPtr[" << strippedType - << "](CLI.GetParam[" << strippedType << "]('" << d.name << "'), (<" - << strippedType << "Type> " << d.name << ").modelptr)" << std::endl; + std::cout << prefix << " SetParamPtr[" << strippedType << "]('" << d.name + << "', (<" << strippedType << "Type> " << d.name << ").modelptr)" + << std::endl; std::cout << prefix << " else:" << std::endl; std::cout << prefix << " raise e" << std::endl; std::cout << prefix << "CLI.SetPassed( '" << d.name << "')" @@ -244,8 +248,8 @@ void PrintInputProcessing( std::cout << prefix << " " << d.name << "_tuple = to_matrix_with_info(" << d.name << ", dtype=np.double)" << std::endl; std::cout << prefix << " " << d.name << "_mat = arma_numpy.numpy_to_mat_d(" - << d.name << "_tuple[0])" << std::endl; - std::cout << prefix << " " << d.name << "_dims = " << d.name << "_tuple[1]" + << d.name << "_tuple[0], " << d.name << "_tuple[1])" << std::endl; + std::cout << prefix << " " << d.name << "_dims = " << d.name << "_tuple[2]" << std::endl; std::cout << prefix << " SetParamWithInfo[arma.Mat[double]](" << " '" << d.name << "', dereference(" << d.name << "_mat), " << " '" << d.name << "', dereference(" << d.name << "_mat), (d, *((size_t*) input)); + PrintInputProcessing::type>(d, + *((size_t*) input)); } } // namespace python diff --git a/src/mlpack/bindings/python/print_output_processing.hpp b/src/mlpack/bindings/python/print_output_processing.hpp index d20da67f4e..00833ceceb 100644 --- a/src/mlpack/bindings/python/print_output_processing.hpp +++ b/src/mlpack/bindings/python/print_output_processing.hpp @@ -180,13 +180,47 @@ void PrintOutputProcessing( * This gives us code like: * * result = ModelType() - * MoveToPtr[Model](( model).modelptr), - * CLI.GetParam[Model]('name')) + * ( result).modelptr = GetParamPtr[Model]('name') */ std::cout << prefix << "result = " << strippedType << "Type()" << std::endl; - std::cout << prefix << "MoveToPtr[" << strippedType << "]((<" - << strippedType << "Type?> result).modelptr, CLI.GetParam[" - << strippedType << "]('" << d.name << "'))" << std::endl; + std::cout << prefix << "(<" << strippedType << "Type?> result).modelptr = " + << "GetParamPtr[" << strippedType << "]('" << d.name << "')" + << std::endl; + + /** + * But we also have to check to ensure there aren't any input model + * parameters of the same type that could have the same model pointer. + * So we need to loop through all input parameters that have the same type, + * and double-check. + */ + std::map& parameters = CLI::Parameters(); + for (auto it = parameters.begin(); it != parameters.end(); ++it) + { + // Is it an input parameter of the same type? + const util::ParamData& data = it->second; + if (data.input && data.cppType == d.cppType && data.required) + { + std::cout << prefix << "if (<" << strippedType + << "Type> result).modelptr" << d.name << " == (<" << strippedType + << "Type> " << data.name << ").modelptr:" << std::endl; + std::cout << prefix << " (<" << strippedType + << "Type> result).modelptr = <" << strippedType << "*> 0" + << std::endl; + std::cout << prefix << " result = " << data.name << std::endl; + } + else if (data.input && data.cppType == d.cppType) + { + std::cout << prefix << "if " << data.name << " is not None:" + << std::endl; + std::cout << prefix << " if (<" << strippedType + << "Type> result).modelptr" << d.name << " == (<" << strippedType + << "Type> " << data.name << ").modelptr:" << std::endl; + std::cout << prefix << " (<" << strippedType + << "Type> result).modelptr = <" << strippedType << "*> 0" + << std::endl; + std::cout << prefix << " result = " << data.name << std::endl; + } + } } else { @@ -194,15 +228,50 @@ void PrintOutputProcessing( * This gives us code like: * * result['name'] = ModelType() - * MoveToPtr[Model*](( result['name']).modelptr), - * CLI.GetParam[Model]('name')) + * ( result['name']).modelptr = GetParamPtr[Model]('name')) */ std::cout << prefix << "result['" << d.name << "'] = " << strippedType << "Type()" << std::endl; - std::cout << prefix << "MoveToPtr[" << strippedType << "]((<" - << strippedType << "Type?>" << " result['" << d.name - << "']).modelptr, CLI.GetParam[" << strippedType << "]('" - << d.name << "'))" << std::endl; + std::cout << prefix << "(<" << strippedType << "Type?> result['" << d.name + << "']).modelptr = GetParamPtr[" << strippedType << "]('" << d.name + << "')" << std::endl; + + /** + * But we also have to check to ensure there aren't any input model + * parameters of the same type that could have the same model pointer. + * So we need to loop through all input parameters that have the same type, + * and double-check. + */ + std::map& parameters = CLI::Parameters(); + for (auto it = parameters.begin(); it != parameters.end(); ++it) + { + // Is it an input parameter of the same type? + const util::ParamData& data = it->second; + if (data.input && data.cppType == d.cppType && data.required) + { + std::cout << prefix << "if (<" << strippedType << "Type> result['" + << d.name << "']).modelptr == (<" << strippedType << "Type> " + << data.name << ").modelptr:" << std::endl; + std::cout << prefix << " (<" << strippedType << "Type> result['" + << d.name << "']).modelptr = <" << strippedType << "*> 0" + << std::endl; + std::cout << prefix << " result['" << d.name << "'] = " << data.name + << std::endl; + } + else if (data.input && data.cppType == d.cppType) + { + std::cout << prefix << "if " << data.name << " is not None:" + << std::endl; + std::cout << prefix << " if (<" << strippedType << "Type> result['" + << d.name << "']).modelptr == (<" << strippedType << "Type> " + << data.name << ").modelptr:" << std::endl; + std::cout << prefix << " (<" << strippedType << "Type> result['" + << d.name << "']).modelptr = <" << strippedType << "*> 0" + << std::endl; + std::cout << prefix << " result['" << d.name << "'] = " << data.name + << std::endl; + } + } } } @@ -227,7 +296,8 @@ void PrintOutputProcessing(const util::ParamData& d, { std::tuple* tuple = (std::tuple*) input; - PrintOutputProcessing(d, std::get<0>(*tuple), std::get<1>(*tuple)); + PrintOutputProcessing::type>(d, + std::get<0>(*tuple), std::get<1>(*tuple)); } } // namespace python diff --git a/src/mlpack/bindings/python/print_pyx.cpp b/src/mlpack/bindings/python/print_pyx.cpp index cb92389366..423de61a41 100644 --- a/src/mlpack/bindings/python/print_pyx.cpp +++ b/src/mlpack/bindings/python/print_pyx.cpp @@ -71,10 +71,10 @@ void PrintPYX(const ProgramDoc& programInfo, cout << "cimport arma" << endl; cout << "cimport arma_numpy" << endl; cout << "from cli cimport CLI" << endl; - cout << "from cli cimport SetParam, SetParamWithInfo" << endl; + cout << "from cli cimport SetParam, SetParamPtr, SetParamWithInfo, " + << "GetParamPtr" << endl; cout << "from cli cimport EnableVerbose, DisableVerbose, DisableBacktrace, " << "ResetTimers, EnableTimers" << endl; - cout << "from cli cimport MoveFromPtr, MoveToPtr" << endl; cout << "from matrix_utils import to_matrix, to_matrix_with_info" << endl; cout << "from serialization cimport SerializeIn, SerializeOut" << endl; cout << endl; From 7e2dcf9563d02cc7eff208bf4b36d0474f74658c Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 26 Jan 2018 13:59:20 -0500 Subject: [PATCH 08/25] Update test bindings to hold model pointers. --- src/mlpack/bindings/tests/get_printable_param.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mlpack/bindings/tests/get_printable_param.hpp b/src/mlpack/bindings/tests/get_printable_param.hpp index e95234549b..5720a0433c 100644 --- a/src/mlpack/bindings/tests/get_printable_param.hpp +++ b/src/mlpack/bindings/tests/get_printable_param.hpp @@ -72,7 +72,8 @@ void GetPrintableParam(const util::ParamData& data, const void* /* input */, void* output) { - *((std::string*) output) = GetPrintableParam(data); + *((std::string*) output) = + GetPrintableParam::type>(data); } } // namespace tests From 7c53e3e79a50032c79c0748e0f884a5062e6fb8a Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 26 Jan 2018 13:59:38 -0500 Subject: [PATCH 09/25] Update PARAM_MODEL_IN() and PARAM_MODEL_OUT() macros to hold pointers. --- src/mlpack/core/util/param.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mlpack/core/util/param.hpp b/src/mlpack/core/util/param.hpp index 2683d59b77..b812887fbd 100644 --- a/src/mlpack/core/util/param.hpp +++ b/src/mlpack/core/util/param.hpp @@ -1058,9 +1058,9 @@ using DatasetInfo = DatasetMapper; // There are no uses of required models, so that is not an option to this // macro (it would be easy to add). #define PARAM_MODEL(TYPE, ID, DESC, ALIAS, REQ, IN) \ - static mlpack::util::Option \ + static mlpack::util::Option \ JOIN(cli_option_dummy_model_, __COUNTER__) \ - (TYPE(), ID, DESC, ALIAS, #TYPE, REQ, IN, false, testName); + (nullptr, ID, DESC, ALIAS, #TYPE, REQ, IN, false, testName); #else // We have to do some really bizarre stuff since __COUNTER__ isn't defined. I // don't think we can absolutely guarantee success, but it should be "good @@ -1113,9 +1113,9 @@ using DatasetInfo = DatasetMapper; !TRANS, testName); #define PARAM_MODEL(TYPE, ID, DESC, ALIAS, REQ, IN) \ - static mlpack::util::Option \ + static mlpack::util::Option \ JOIN(JOIN(cli_option_dummy_object_model_, __LINE__), opt) \ - (TYPE(), ID, DESC, ALIAS, #TYPE, REQ, IN, false, \ + (nullptr, ID, DESC, ALIAS, #TYPE, REQ, IN, false, \ testName); #endif From ede36869753ba38e35f84f7fa8096303bb427b80 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 26 Jan 2018 14:01:41 -0500 Subject: [PATCH 10/25] Refactor all programs to work with model pointers. --- src/mlpack/methods/adaboost/adaboost_main.cpp | 30 +++++----- .../methods/approx_kfn/approx_kfn_main.cpp | 29 ++++----- src/mlpack/methods/cf/cf_main.cpp | 22 ++++--- .../decision_stump/decision_stump_main.cpp | 20 +++---- .../decision_tree/decision_tree_main.cpp | 26 ++++---- src/mlpack/methods/det/det_main.cpp | 14 ++--- src/mlpack/methods/fastmks/fastmks_main.cpp | 55 ++++++++--------- src/mlpack/methods/gmm/gmm_generate_main.cpp | 9 ++- .../methods/gmm/gmm_probability_main.cpp | 7 +-- src/mlpack/methods/gmm/gmm_train_main.cpp | 27 +++++---- src/mlpack/methods/hmm/hmm_loglik_main.cpp | 2 +- src/mlpack/methods/hmm/hmm_train_main.cpp | 12 ++-- src/mlpack/methods/hmm/hmm_viterbi_main.cpp | 5 +- .../hoeffding_trees/hoeffding_tree_main.cpp | 34 +++++------ src/mlpack/methods/lars/lars_main.cpp | 23 ++++---- .../linear_regression_main.cpp | 18 +++--- .../local_coordinate_coding_main.cpp | 51 ++++++++-------- .../logistic_regression_main.cpp | 23 ++++---- src/mlpack/methods/lsh/lsh_main.cpp | 24 ++++---- src/mlpack/methods/naive_bayes/nbc_main.cpp | 27 ++++----- .../methods/neighbor_search/kfn_main.cpp | 47 ++++++++------- .../methods/neighbor_search/knn_main.cpp | 53 ++++++++--------- .../methods/perceptron/perceptron_main.cpp | 47 ++++++++------- .../random_forest/random_forest_main.cpp | 25 ++++---- .../range_search/range_search_main.cpp | 29 ++++----- src/mlpack/methods/rann/krann_main.cpp | 52 ++++++++-------- .../softmax_regression_main.cpp | 21 +++---- .../sparse_coding/sparse_coding_main.cpp | 59 +++++++++---------- 28 files changed, 377 insertions(+), 414 deletions(-) diff --git a/src/mlpack/methods/adaboost/adaboost_main.cpp b/src/mlpack/methods/adaboost/adaboost_main.cpp index e5209efb46..2634ff08a1 100644 --- a/src/mlpack/methods/adaboost/adaboost_main.cpp +++ b/src/mlpack/methods/adaboost/adaboost_main.cpp @@ -145,10 +145,11 @@ static void mlpackMain() ReportIgnoredParam({{ "test", false }}, "output"); - AdaBoostModel m; + AdaBoostModel* m; if (CLI::HasParam("training")) { mat trainingData = std::move(CLI::GetParam("training")); + m = new AdaBoostModel(); // Load labels. arma::Row labelsIn; @@ -172,28 +173,28 @@ static void mlpackMain() Row labels; // Normalize the labels. - data::NormalizeLabels(labelsIn, labels, m.Mappings()); + data::NormalizeLabels(labelsIn, labels, m->Mappings()); // Get other training parameters. const double tolerance = CLI::GetParam("tolerance"); const size_t iterations = (size_t) CLI::GetParam("iterations"); const string weakLearner = CLI::GetParam("weak_learner"); if (weakLearner == "decision_stump") - m.WeakLearnerType() = AdaBoostModel::WeakLearnerTypes::DECISION_STUMP; + m->WeakLearnerType() = AdaBoostModel::WeakLearnerTypes::DECISION_STUMP; else if (weakLearner == "perceptron") - m.WeakLearnerType() = AdaBoostModel::WeakLearnerTypes::PERCEPTRON; + m->WeakLearnerType() = AdaBoostModel::WeakLearnerTypes::PERCEPTRON; - const size_t numClasses = m.Mappings().n_elem; + const size_t numClasses = m->Mappings().n_elem; Log::Info << numClasses << " classes in dataset." << endl; Timer::Start("adaboost_training"); - m.Train(trainingData, labels, numClasses, iterations, tolerance); + m->Train(trainingData, labels, numClasses, iterations, tolerance); Timer::Stop("adaboost_training"); } else { // We have a specified input model. - m = std::move(CLI::GetParam("input_model")); + m = CLI::GetParam("input_model"); } // Perform classification, if desired. @@ -201,24 +202,21 @@ static void mlpackMain() { mat testingData = std::move(CLI::GetParam("test")); - if (testingData.n_rows != m.Dimensionality()) + if (testingData.n_rows != m->Dimensionality()) Log::Fatal << "Test data dimensionality (" << testingData.n_rows << ") " << "must be the same as the model dimensionality (" - << m.Dimensionality() << ")!" << endl; + << m->Dimensionality() << ")!" << endl; Row predictedLabels(testingData.n_cols); Timer::Start("adaboost_classification"); - m.Classify(testingData, predictedLabels); + m->Classify(testingData, predictedLabels); Timer::Stop("adaboost_classification"); Row results; - data::RevertLabels(predictedLabels, m.Mappings(), results); + data::RevertLabels(predictedLabels, m->Mappings(), results); - if (CLI::HasParam("output")) - CLI::GetParam>("output") = std::move(results); + CLI::GetParam>("output") = std::move(results); } - // Should we save the model, too? - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(m); + CLI::GetParam("output_model") = m; } diff --git a/src/mlpack/methods/approx_kfn/approx_kfn_main.cpp b/src/mlpack/methods/approx_kfn/approx_kfn_main.cpp index f73008583d..030fb90132 100644 --- a/src/mlpack/methods/approx_kfn/approx_kfn_main.cpp +++ b/src/mlpack/methods/approx_kfn/approx_kfn_main.cpp @@ -190,11 +190,12 @@ static void mlpackMain() } // Do the building of a model, if necessary. - ApproxKFNModel m; + ApproxKFNModel* m; arma::mat referenceSet; // This may be used at query time. if (CLI::HasParam("reference")) { referenceSet = std::move(CLI::GetParam("reference")); + m = new ApproxKFNModel(); const size_t numTables = (size_t) CLI::GetParam("num_tables"); const size_t numProjections = @@ -205,16 +206,16 @@ static void mlpackMain() { Timer::Start("drusilla_select_construct"); Log::Info << "Building DrusillaSelect model..." << endl; - m.type = 0; - m.ds = DrusillaSelect<>(referenceSet, numTables, numProjections); + m->type = 0; + m->ds = DrusillaSelect<>(referenceSet, numTables, numProjections); Timer::Stop("drusilla_select_construct"); } else { Timer::Start("qdafn_construct"); Log::Info << "Building QDAFN model..." << endl; - m.type = 1; - m.qdafn = QDAFN<>(referenceSet, numTables, numProjections); + m->type = 1; + m->qdafn = QDAFN<>(referenceSet, numTables, numProjections); Timer::Stop("qdafn_construct"); } Log::Info << "Model built." << endl; @@ -222,7 +223,7 @@ static void mlpackMain() else { // We must load the model from what was passed. - m = std::move(CLI::GetParam("input_model")); + m = CLI::GetParam("input_model"); } // Now, do we need to do any queries? @@ -238,12 +239,12 @@ static void mlpackMain() if (CLI::HasParam("query")) querySet = std::move(CLI::GetParam("query")); - if (m.type == 0) + if (m->type == 0) { Timer::Start("drusilla_select_search"); Log::Info << "Searching for " << k << " furthest neighbors with " << "DrusillaSelect..." << endl; - m.ds.Search(set, k, neighbors, distances); + m->ds.Search(set, k, neighbors, distances); Timer::Stop("drusilla_select_search"); } else @@ -251,7 +252,7 @@ static void mlpackMain() Timer::Start("qdafn_search"); Log::Info << "Searching for " << k << " furthest neighbors with " << "QDAFN..." << endl; - m.qdafn.Search(set, k, neighbors, distances); + m->qdafn.Search(set, k, neighbors, distances); Timer::Stop("qdafn_search"); } Log::Info << "Search complete." << endl; @@ -288,13 +289,9 @@ static void mlpackMain() } // Save results, if desired. - if (CLI::HasParam("neighbors")) - CLI::GetParam>("neighbors") = std::move(neighbors); - if (CLI::HasParam("distances")) - CLI::GetParam("distances") = std::move(distances); + CLI::GetParam>("neighbors") = std::move(neighbors); + CLI::GetParam("distances") = std::move(distances); } - // Should we save the model? - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(m); + CLI::GetParam("output_model") = m; } diff --git a/src/mlpack/methods/cf/cf_main.cpp b/src/mlpack/methods/cf/cf_main.cpp index 2fedefb4ed..f09280dacd 100644 --- a/src/mlpack/methods/cf/cf_main.cpp +++ b/src/mlpack/methods/cf/cf_main.cpp @@ -115,7 +115,7 @@ PARAM_INT_IN("recommendations", "Number of recommendations to generate for each" PARAM_INT_IN("seed", "Set the random seed (0 uses std::time(NULL)).", "s", 0); -void ComputeRecommendations(CF& cf, +void ComputeRecommendations(CF* cf, const size_t numRecs, arma::Mat& recommendations) { @@ -132,16 +132,16 @@ void ComputeRecommendations(CF& cf, Log::Info << "Generating recommendations for " << users.n_elem << " users." << endl; - cf.GetRecommendations(numRecs, recommendations, users.row(0).t()); + cf->GetRecommendations(numRecs, recommendations, users.row(0).t()); } else { Log::Info << "Generating recommendations for all users." << endl; - cf.GetRecommendations(numRecs, recommendations); + cf->GetRecommendations(numRecs, recommendations); } } -void ComputeRMSE(CF& cf) +void ComputeRMSE(CF* cf) { // Now, compute each test point. arma::mat testData = std::move(CLI::GetParam("test")); @@ -156,7 +156,7 @@ void ComputeRMSE(CF& cf) // Now compute the RMSE. arma::vec predictions; - cf.Predict(combinations, predictions); + cf->Predict(combinations, predictions); // Compute the root of the sum of the squared errors, divide by the number of // points to get the RMSE. It turns out this is just the L2-norm divided by @@ -168,7 +168,7 @@ void ComputeRMSE(CF& cf) Log::Info << "RMSE is " << rmse << "." << endl; } -void PerformAction(CF& c) +void PerformAction(CF* c) { if (CLI::HasParam("query") || CLI::HasParam("all_user_recommendations")) { @@ -180,15 +180,13 @@ void PerformAction(CF& c) ComputeRecommendations(c, numRecs, recommendations); // Save the output. - if (CLI::HasParam("output")) - CLI::GetParam>("output") = recommendations; + CLI::GetParam>("output") = recommendations; } if (CLI::HasParam("test")) ComputeRMSE(c); - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(c); + CLI::GetParam("output_model") = c; } template @@ -198,7 +196,7 @@ void PerformAction(Factorizer&& factorizer, { // Parameters for generating the CF object. const size_t neighborhood = (size_t) CLI::GetParam("neighborhood"); - CF c(dataset, factorizer, neighborhood, rank); + CF* c = new CF(dataset, factorizer, neighborhood, rank); PerformAction(c); } @@ -323,7 +321,7 @@ static void mlpackMain() else { // Load an input model. - CF c = std::move(CLI::GetParam("input_model")); + CF* c = std::move(CLI::GetParam("input_model")); PerformAction(c); } diff --git a/src/mlpack/methods/decision_stump/decision_stump_main.cpp b/src/mlpack/methods/decision_stump/decision_stump_main.cpp index 2bfe230043..aa4d9a67d8 100644 --- a/src/mlpack/methods/decision_stump/decision_stump_main.cpp +++ b/src/mlpack/methods/decision_stump/decision_stump_main.cpp @@ -116,9 +116,10 @@ static void mlpackMain() ReportIgnoredParam({{ "test", false }}, "predictions"); // We must either load a model, or train a new stump. - DSModel model; + DSModel* model; if (CLI::HasParam("training")) { + model = new DSModel(); mat trainingData = std::move(CLI::GetParam("training")); // Load labels, if necessary. @@ -140,18 +141,18 @@ static void mlpackMain() // Normalize the labels. Row labels; - data::NormalizeLabels(labelsIn, labels, model.mappings); + data::NormalizeLabels(labelsIn, labels, model->mappings); const size_t bucketSize = CLI::GetParam("bucket_size"); const size_t classes = labels.max() + 1; Timer::Start("training"); - model.stump.Train(trainingData, labels, classes, bucketSize); + model->stump.Train(trainingData, labels, classes, bucketSize); Timer::Stop("training"); } else { - model = std::move(CLI::GetParam("input_model")); + model = CLI::GetParam("input_model"); } // Now, do we need to do any testing? @@ -160,21 +161,21 @@ static void mlpackMain() // Load the test file. mat testingData = std::move(CLI::GetParam("test")); - if (testingData.n_rows <= model.stump.SplitDimension()) + if (testingData.n_rows <= model->stump.SplitDimension()) Log::Fatal << "Test data dimensionality (" << testingData.n_rows << ") " << "is too low; the trained stump requires at least " - << model.stump.SplitDimension() << " dimensions!" << endl; + << model->stump.SplitDimension() << " dimensions!" << endl; Row predictedLabels(testingData.n_cols); Timer::Start("testing"); - model.stump.Classify(testingData, predictedLabels); + model->stump.Classify(testingData, predictedLabels); Timer::Stop("testing"); // Denormalize predicted labels, if we want to save them. if (CLI::HasParam("predictions")) { Row actualLabels; - data::RevertLabels(predictedLabels, model.mappings, actualLabels); + data::RevertLabels(predictedLabels, model->mappings, actualLabels); // Save the predicted labels as output. CLI::GetParam>("predictions") = std::move(actualLabels); @@ -182,6 +183,5 @@ static void mlpackMain() } // Save the model, if desired. - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(model); + CLI::GetParam("output_model") = model; } diff --git a/src/mlpack/methods/decision_tree/decision_tree_main.cpp b/src/mlpack/methods/decision_tree/decision_tree_main.cpp index e86f34042e..e4ee8fcedc 100644 --- a/src/mlpack/methods/decision_tree/decision_tree_main.cpp +++ b/src/mlpack/methods/decision_tree/decision_tree_main.cpp @@ -137,13 +137,14 @@ static void mlpackMain() "leaf size must be positive"); // Load the model or build the tree. - DecisionTreeModel model; + DecisionTreeModel* model; arma::mat trainingSet; arma::Row labels; if (CLI::HasParam("training")) { - model.info = std::move(std::get<0>(CLI::GetParam("training"))); + model = new DecisionTreeModel(); + model->info = std::move(std::get<0>(CLI::GetParam("training"))); trainingSet = std::move(std::get<1>(CLI::GetParam("training"))); if (CLI::HasParam("labels")) { @@ -169,12 +170,12 @@ static void mlpackMain() { arma::Row weights = std::move(CLI::GetParam>("weights")); - model.tree = DecisionTree<>(trainingSet, model.info, labels, + model->tree = DecisionTree<>(trainingSet, model->info, labels, numClasses, weights, minLeafSize); } else { - model.tree = DecisionTree<>(trainingSet, model.info, labels, + model->tree = DecisionTree<>(trainingSet, model->info, labels, numClasses, minLeafSize); } @@ -184,7 +185,7 @@ static void mlpackMain() arma::Row predictions; arma::mat probabilities; - model.tree.Classify(trainingSet, predictions, probabilities); + model->tree.Classify(trainingSet, predictions, probabilities); size_t correct = 0; for (size_t i = 0; i < trainingSet.n_cols; ++i) @@ -199,19 +200,19 @@ static void mlpackMain() } else { - model = std::move(CLI::GetParam("input_model")); + model = CLI::GetParam("input_model"); } // Do we need to get predictions? if (CLI::HasParam("test")) { - std::get<0>(CLI::GetRawParam("test")) = model.info; + std::get<0>(CLI::GetRawParam("test")) = model->info; arma::mat testPoints = std::get<1>(CLI::GetParam("test")); arma::Row predictions; arma::mat probabilities; - model.tree.Classify(testPoints, predictions, probabilities); + model->tree.Classify(testPoints, predictions, probabilities); // Do we need to calculate accuracy? if (CLI::HasParam("test_labels")) @@ -231,13 +232,10 @@ static void mlpackMain() } // Do we need to save outputs? - if (CLI::HasParam("predictions")) - CLI::GetParam>("predictions") = std::move(predictions); - if (CLI::HasParam("probabilities")) - CLI::GetParam("probabilities") = std::move(probabilities); + CLI::GetParam>("predictions") = predictions; + CLI::GetParam("probabilities") = probabilities; } // Do we need to save the model? - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(model); + CLI::GetParam("output_model") = model; } diff --git a/src/mlpack/methods/det/det_main.cpp b/src/mlpack/methods/det/det_main.cpp index 29ecd56c99..a9cb4ec47c 100644 --- a/src/mlpack/methods/det/det_main.cpp +++ b/src/mlpack/methods/det/det_main.cpp @@ -168,7 +168,7 @@ static void mlpackMain() } else { - tree = &CLI::GetParam>("input_model"); + tree = CLI::GetParam*>("input_model"); } // Compute the density at the provided test points and output the density in @@ -213,8 +213,7 @@ static void mlpackMain() if (!ofs.is_open()) { Log::Warn << "Unable to open file '" << tagFile - << "' to save tag membership info." - << std::endl; + << "' to save tag membership info." << std::endl; } else if (CLI::HasParam("path_format")) { @@ -231,7 +230,7 @@ static void mlpackMain() else { Log::Warn << "Unknown path format specified: '" << pathFormat - << "'. Valid are: lr | lr-id | id-lr. Defaults to 'lr'." << endl; + << "'. Valid are: lr | lr-id | id-lr. Defaults to 'lr'." << endl; theFormat = PathCacher::FormatLR; } @@ -284,10 +283,5 @@ static void mlpackMain() } // Save the model, if desired. - if (CLI::HasParam("output_model")) - CLI::GetParam>("output_model") = std::move(*tree); - - // Clean up memory, if we need to. - if (!CLI::HasParam("input_model") && !CLI::HasParam("output_model")) - delete tree; + CLI::GetParam*>("output_model") = tree; } diff --git a/src/mlpack/methods/fastmks/fastmks_main.cpp b/src/mlpack/methods/fastmks/fastmks_main.cpp index f3b7333ebe..1d8e7e7fb6 100644 --- a/src/mlpack/methods/fastmks/fastmks_main.cpp +++ b/src/mlpack/methods/fastmks/fastmks_main.cpp @@ -111,10 +111,11 @@ static void mlpackMain() // Naive mode overrides single mode. ReportIgnoredParam({{ "naive", true }}, "single"); - FastMKSModel model; + FastMKSModel* model; arma::mat referenceData; if (CLI::HasParam("reference")) { + model = new FastMKSModel(); referenceData = std::move(CLI::GetParam("reference")); Log::Info << "Loaded reference data (" << referenceData.n_rows << " x " @@ -137,55 +138,55 @@ static void mlpackMain() if (kernelType == "linear") { LinearKernel lk; - model.KernelType() = FastMKSModel::LINEAR_KERNEL; - model.BuildModel(referenceData, lk, single, naive, base); + model->KernelType() = FastMKSModel::LINEAR_KERNEL; + model->BuildModel(referenceData, lk, single, naive, base); } else if (kernelType == "polynomial") { PolynomialKernel pk(degree, offset); - model.KernelType() = FastMKSModel::POLYNOMIAL_KERNEL; - model.BuildModel(referenceData, pk, single, naive, base); + model->KernelType() = FastMKSModel::POLYNOMIAL_KERNEL; + model->BuildModel(referenceData, pk, single, naive, base); } else if (kernelType == "cosine") { CosineDistance cd; - model.KernelType() = FastMKSModel::COSINE_DISTANCE; - model.BuildModel(referenceData, cd, single, naive, base); + model->KernelType() = FastMKSModel::COSINE_DISTANCE; + model->BuildModel(referenceData, cd, single, naive, base); } else if (kernelType == "gaussian") { GaussianKernel gk(bandwidth); - model.KernelType() = FastMKSModel::GAUSSIAN_KERNEL; - model.BuildModel(referenceData, gk, single, naive, base); + model->KernelType() = FastMKSModel::GAUSSIAN_KERNEL; + model->BuildModel(referenceData, gk, single, naive, base); } else if (kernelType == "epanechnikov") { EpanechnikovKernel ek(bandwidth); - model.KernelType() = FastMKSModel::EPANECHNIKOV_KERNEL; - model.BuildModel(referenceData, ek, single, naive, base); + model->KernelType() = FastMKSModel::EPANECHNIKOV_KERNEL; + model->BuildModel(referenceData, ek, single, naive, base); } else if (kernelType == "triangular") { TriangularKernel tk(bandwidth); - model.KernelType() = FastMKSModel::TRIANGULAR_KERNEL; - model.BuildModel(referenceData, tk, single, naive, base); + model->KernelType() = FastMKSModel::TRIANGULAR_KERNEL; + model->BuildModel(referenceData, tk, single, naive, base); } else if (kernelType == "hyptan") { HyperbolicTangentKernel htk(scale, offset); - model.KernelType() = FastMKSModel::HYPTAN_KERNEL; - model.BuildModel(referenceData, htk, single, naive, base); + model->KernelType() = FastMKSModel::HYPTAN_KERNEL; + model->BuildModel(referenceData, htk, single, naive, base); } } else { // Load model from file, then do whatever is necessary. - model = std::move(CLI::GetParam("input_model")); + model = CLI::GetParam("input_model"); } // Set search preferences. - model.Naive() = CLI::HasParam("naive"); - model.SingleMode() = CLI::HasParam("single"); + model->Naive() = CLI::HasParam("naive"); + model->SingleMode() = CLI::HasParam("single"); // Should we do search? if (CLI::HasParam("k")) @@ -202,23 +203,19 @@ static void mlpackMain() Log::Info << "Loaded query data (" << queryData.n_rows << " x " << queryData.n_cols << ")." << endl; - model.Search(queryData, (size_t) CLI::GetParam("k"), indices, + model->Search(queryData, (size_t) CLI::GetParam("k"), indices, kernels, base); } else { - model.Search((size_t) CLI::GetParam("k"), indices, kernels); + model->Search((size_t) CLI::GetParam("k"), indices, kernels); } - // Save output, if we were asked to. - if (CLI::HasParam("kernels")) - CLI::GetParam("kernels") = std::move(kernels); - - if (CLI::HasParam("indices")) - CLI::GetParam>("indices") = std::move(indices); + // Save output. + CLI::GetParam("kernels") = std::move(kernels); + CLI::GetParam>("indices") = std::move(indices); } - // Save the model, if requested. - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(model); + // Save the model. + CLI::GetParam("output_model") = model; } diff --git a/src/mlpack/methods/gmm/gmm_generate_main.cpp b/src/mlpack/methods/gmm/gmm_generate_main.cpp index a6ed7d6ba4..7842349414 100644 --- a/src/mlpack/methods/gmm/gmm_generate_main.cpp +++ b/src/mlpack/methods/gmm/gmm_generate_main.cpp @@ -55,15 +55,14 @@ static void mlpackMain() RequireParamValue("samples", [](int x) { return x > 0; }, true, "number of samples must be greater than 0"); - GMM gmm = std::move(CLI::GetParam("input_model")); + GMM* gmm = CLI::GetParam("input_model"); size_t length = (size_t) CLI::GetParam("samples"); Log::Info << "Generating " << length << " samples..." << endl; - arma::mat samples(gmm.Dimensionality(), length); + arma::mat samples(gmm->Dimensionality(), length); for (size_t i = 0; i < length; ++i) - samples.col(i) = gmm.Random(); + samples.col(i) = gmm->Random(); // Save, if the user asked for it. - if (CLI::HasParam("output")) - CLI::GetParam("output") = std::move(samples); + CLI::GetParam("output") = std::move(samples); } diff --git a/src/mlpack/methods/gmm/gmm_probability_main.cpp b/src/mlpack/methods/gmm/gmm_probability_main.cpp index ac1669aaac..f40c866284 100644 --- a/src/mlpack/methods/gmm/gmm_probability_main.cpp +++ b/src/mlpack/methods/gmm/gmm_probability_main.cpp @@ -46,16 +46,15 @@ static void mlpackMain() RequireAtLeastOnePassed({ "output" }, false, "no results will be saved"); // Get the GMM and the points. - GMM gmm = std::move(CLI::GetParam("input_model")); + GMM* gmm = CLI::GetParam("input_model"); arma::mat dataset = std::move(CLI::GetParam("input")); // Now calculate the probabilities. arma::rowvec probabilities(dataset.n_cols); for (size_t i = 0; i < dataset.n_cols; ++i) - probabilities[i] = gmm.Probability(dataset.unsafe_col(i)); + probabilities[i] = gmm->Probability(dataset.unsafe_col(i)); // And save the result. - if (CLI::HasParam("output")) - CLI::GetParam("output") = std::move(probabilities); + CLI::GetParam("output") = std::move(probabilities); } diff --git a/src/mlpack/methods/gmm/gmm_train_main.cpp b/src/mlpack/methods/gmm/gmm_train_main.cpp index 82694fb17c..c331d04dc0 100644 --- a/src/mlpack/methods/gmm/gmm_train_main.cpp +++ b/src/mlpack/methods/gmm/gmm_train_main.cpp @@ -153,17 +153,21 @@ static void mlpackMain() } // Initialize GMM. - GMM gmm(size_t(gaussians), dataPoints.n_rows); + GMM* gmm; if (CLI::HasParam("input_model")) { - gmm = std::move(CLI::GetParam("input_model")); + gmm = CLI::GetParam("input_model"); - if (gmm.Dimensionality() != dataPoints.n_rows) + if (gmm->Dimensionality() != dataPoints.n_rows) Log::Fatal << "Given input data (with " << PRINT_PARAM_STRING("input") << ") has dimensionality " << dataPoints.n_rows << ", but the initial" << " model (given with " << PRINT_PARAM_STRING("input_model") - << " has dimensionality " << gmm.Dimensionality() << "!" << endl; + << " has dimensionality " << gmm->Dimensionality() << "!" << endl; + } + else + { + gmm = new GMM(size_t(gaussians), dataPoints.n_rows); } // Gather parameters for EMFit object. @@ -199,7 +203,7 @@ static void mlpackMain() // Compute the parameters of the model using the EM algorithm. Timer::Start("em"); EMFit em(maxIterations, tolerance, k); - likelihood = gmm.Train(dataPoints, CLI::GetParam("trials"), false, + likelihood = gmm->Train(dataPoints, CLI::GetParam("trials"), false, em); Timer::Stop("em"); } @@ -208,7 +212,7 @@ static void mlpackMain() // Compute the parameters of the model using the EM algorithm. Timer::Start("em"); EMFit em(maxIterations, tolerance, k); - likelihood = gmm.Train(dataPoints, CLI::GetParam("trials"), false, + likelihood = gmm->Train(dataPoints, CLI::GetParam("trials"), false, em); Timer::Stop("em"); } @@ -217,7 +221,7 @@ static void mlpackMain() // Compute the parameters of the model using the EM algorithm. Timer::Start("em"); EMFit em(maxIterations, tolerance, k); - likelihood = gmm.Train(dataPoints, CLI::GetParam("trials"), false, + likelihood = gmm->Train(dataPoints, CLI::GetParam("trials"), false, em); Timer::Stop("em"); } @@ -231,7 +235,7 @@ static void mlpackMain() // Compute the parameters of the model using the EM algorithm. Timer::Start("em"); EMFit, DiagonalConstraint> em(maxIterations, tolerance); - likelihood = gmm.Train(dataPoints, CLI::GetParam("trials"), false, + likelihood = gmm->Train(dataPoints, CLI::GetParam("trials"), false, em); Timer::Stop("em"); } @@ -240,7 +244,7 @@ static void mlpackMain() // Compute the parameters of the model using the EM algorithm. Timer::Start("em"); EMFit<> em(maxIterations, tolerance); - likelihood = gmm.Train(dataPoints, CLI::GetParam("trials"), false, + likelihood = gmm->Train(dataPoints, CLI::GetParam("trials"), false, em); Timer::Stop("em"); } @@ -249,7 +253,7 @@ static void mlpackMain() // Compute the parameters of the model using the EM algorithm. Timer::Start("em"); EMFit, NoConstraint> em(maxIterations, tolerance); - likelihood = gmm.Train(dataPoints, CLI::GetParam("trials"), false, + likelihood = gmm->Train(dataPoints, CLI::GetParam("trials"), false, em); Timer::Stop("em"); } @@ -257,6 +261,5 @@ static void mlpackMain() Log::Info << "Log-likelihood of estimate: " << likelihood << "." << endl; - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(gmm); + CLI::GetParam("output_model") = gmm; } diff --git a/src/mlpack/methods/hmm/hmm_loglik_main.cpp b/src/mlpack/methods/hmm/hmm_loglik_main.cpp index 1a3d51a8cf..6ebb3cbef1 100644 --- a/src/mlpack/methods/hmm/hmm_loglik_main.cpp +++ b/src/mlpack/methods/hmm/hmm_loglik_main.cpp @@ -79,5 +79,5 @@ struct Loglik static void mlpackMain() { // Load model, and calculate the log-likelihood of the sequence. - CLI::GetParam("input_model").PerformAction((void*) NULL); + CLI::GetParam("input_model")->PerformAction((void*) NULL); } diff --git a/src/mlpack/methods/hmm/hmm_train_main.cpp b/src/mlpack/methods/hmm/hmm_train_main.cpp index 2983c2528b..06cf01db0e 100644 --- a/src/mlpack/methods/hmm/hmm_train_main.cpp +++ b/src/mlpack/methods/hmm/hmm_train_main.cpp @@ -436,21 +436,21 @@ static void mlpackMain() typeId = HMMType::GaussianMixtureModelHMM; // If we have a model file, we can autodetect the type. - HMMModel hmm(typeId); + HMMModel* hmm; if (CLI::HasParam("input_model")) { - hmm = std::move(CLI::GetParam("input_model")); + hmm = CLI::GetParam("input_model"); } else { // We need to initialize the model. - hmm.PerformAction>(&trainSeq); + hmm = new HMMModel(typeId); + hmm->PerformAction>(&trainSeq); } // Train the model. - hmm.PerformAction>(&trainSeq); + hmm->PerformAction>(&trainSeq); // If necessary, save the output. - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(hmm); + CLI::GetParam("output_model") = hmm; } diff --git a/src/mlpack/methods/hmm/hmm_viterbi_main.cpp b/src/mlpack/methods/hmm/hmm_viterbi_main.cpp index 323bc066cc..d0e1168b3d 100644 --- a/src/mlpack/methods/hmm/hmm_viterbi_main.cpp +++ b/src/mlpack/methods/hmm/hmm_viterbi_main.cpp @@ -77,8 +77,7 @@ struct Viterbi hmm.Predict(dataSeq, sequence); // Save output. - if (CLI::HasParam("output")) - CLI::GetParam>("output") = std::move(sequence); + CLI::GetParam>("output") = std::move(sequence); } }; @@ -86,5 +85,5 @@ static void mlpackMain() { RequireAtLeastOnePassed({ "output" }, false, "no results will be saved"); - CLI::GetParam("input_model").PerformAction((void*) NULL); + CLI::GetParam("input_model")->PerformAction((void*) NULL); } diff --git a/src/mlpack/methods/hoeffding_trees/hoeffding_tree_main.cpp b/src/mlpack/methods/hoeffding_trees/hoeffding_tree_main.cpp index 73012a6265..2c4576337b 100644 --- a/src/mlpack/methods/hoeffding_trees/hoeffding_tree_main.cpp +++ b/src/mlpack/methods/hoeffding_trees/hoeffding_tree_main.cpp @@ -140,25 +140,25 @@ static void mlpackMain() true, "unrecognized numeric split strategy"); // Do we need to load a model or do we already have one? - HoeffdingTreeModel model; + HoeffdingTreeModel* model; DatasetInfo datasetInfo; arma::mat trainingSet; arma::Row labels; if (CLI::HasParam("input_model")) { - model = std::move(CLI::GetParam("input_model")); + model = CLI::GetParam("input_model"); } else { // Initialize a model. if (!CLI::HasParam("info_gain") && (numericSplitStrategy == "domingos")) - model = HoeffdingTreeModel(HoeffdingTreeModel::GINI_HOEFFDING); + model = new HoeffdingTreeModel(HoeffdingTreeModel::GINI_HOEFFDING); else if (!CLI::HasParam("info_gain") && (numericSplitStrategy == "binary")) - model = HoeffdingTreeModel(HoeffdingTreeModel::GINI_BINARY); + model = new HoeffdingTreeModel(HoeffdingTreeModel::GINI_BINARY); else if (CLI::HasParam("info_gain") && (numericSplitStrategy == "domingos")) - model = HoeffdingTreeModel(HoeffdingTreeModel::INFO_HOEFFDING); + model = new HoeffdingTreeModel(HoeffdingTreeModel::INFO_HOEFFDING); else if (CLI::HasParam("info_gain") && (numericSplitStrategy == "binary")) - model = HoeffdingTreeModel(HoeffdingTreeModel::INFO_BINARY); + model = new HoeffdingTreeModel(HoeffdingTreeModel::INFO_BINARY); } // Now, do we need to train? @@ -207,7 +207,7 @@ static void mlpackMain() if (!CLI::HasParam("input_model")) { // Build the model. - model.BuildModel(trainingSet, datasetInfo, labels, + model->BuildModel(trainingSet, datasetInfo, labels, arma::max(labels) + 1, batchTraining, confidence, maxSamples, 100, minSamples, bins, observationsBeforeBinning); --passes; // This model-building takes one pass. @@ -219,12 +219,12 @@ static void mlpackMain() // We only need to do batch training if we've not already called // BuildModel. if (CLI::HasParam("input_model")) - model.Train(trainingSet, labels, true); + model->Train(trainingSet, labels, true); } else { for (size_t p = 0; p < passes; ++p) - model.Train(trainingSet, labels, false); + model->Train(trainingSet, labels, false); } Timer::Stop("tree_training"); @@ -235,7 +235,7 @@ static void mlpackMain() { // Get training error. arma::Row predictions; - model.Classify(trainingSet, predictions); + model->Classify(trainingSet, predictions); size_t correct = 0; for (size_t i = 0; i < labels.n_elem; ++i) @@ -248,7 +248,7 @@ static void mlpackMain() } // Get the number of nodes in the tree. - Log::Info << model.NumNodes() << " nodes in the tree." << endl; + Log::Info << model->NumNodes() << " nodes in the tree." << endl; // The tree is trained or loaded. Now do any testing if we need. if (CLI::HasParam("test")) @@ -262,7 +262,7 @@ static void mlpackMain() arma::rowvec probabilities; Timer::Start("tree_testing"); - model.Classify(testSet, predictions, probabilities); + model->Classify(testSet, predictions, probabilities); Timer::Stop("tree_testing"); if (CLI::HasParam("test_labels")) @@ -281,14 +281,10 @@ static void mlpackMain() 100.0 << ")." << endl; } - if (CLI::HasParam("predictions")) - CLI::GetParam>("predictions") = std::move(predictions); - - if (CLI::HasParam("probabilities")) - CLI::GetParam("probabilities") = std::move(probabilities); + CLI::GetParam>("predictions") = std::move(predictions); + CLI::GetParam("probabilities") = std::move(probabilities); } // Check the accuracy on the training set. - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(model); + CLI::GetParam("output_model") = model; } diff --git a/src/mlpack/methods/lars/lars_main.cpp b/src/mlpack/methods/lars/lars_main.cpp index 0c6d1a28e5..040c5f0c3e 100644 --- a/src/mlpack/methods/lars/lars_main.cpp +++ b/src/mlpack/methods/lars/lars_main.cpp @@ -120,11 +120,12 @@ static void mlpackMain() "no results will be saved"); ReportIgnoredParam({{ "test", true }}, "output_predictions"); - // Initialize the object. - LARS lars(useCholesky, lambda1, lambda2); - + LARS* lars; if (CLI::HasParam("input")) { + // Initialize the object. + lars = new LARS(useCholesky, lambda1, lambda2); + // Load covariates. We can avoid LARS transposing our data by choosing to // not transpose this data (that's why we used PARAM_TMATRIX_IN). mat matX = std::move(CLI::GetParam("input")); @@ -146,11 +147,11 @@ static void mlpackMain() vec beta; arma::rowvec y = std::move(matY); - lars.Train(matX, y, beta, false /* do not transpose */); + lars->Train(matX, y, beta, false /* do not transpose */); } else // We must have --input_model_file. { - lars = std::move(CLI::GetParam("input_model")); + lars = CLI::GetParam("input_model"); } if (CLI::HasParam("test")) @@ -162,19 +163,17 @@ static void mlpackMain() // Make sure the dimensionality is right. We haven't transposed, so, we // check n_cols not n_rows. - if (testPoints.n_cols != lars.BetaPath().back().n_elem) + if (testPoints.n_cols != lars->BetaPath().back().n_elem) Log::Fatal << "Dimensionality of test set (" << testPoints.n_cols << ") " << "is not equal to the dimensionality of the model (" - << lars.BetaPath().back().n_elem << ")!" << endl; + << lars->BetaPath().back().n_elem << ")!" << endl; arma::rowvec predictions; - lars.Predict(testPoints.t(), predictions, false); + lars->Predict(testPoints.t(), predictions, false); // Save test predictions (one per line). - if (CLI::HasParam("output_predictions")) - CLI::GetParam("output_predictions") = predictions.t(); + CLI::GetParam("output_predictions") = predictions.t(); } - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(lars); + CLI::GetParam("output_model") = lars; } diff --git a/src/mlpack/methods/linear_regression/linear_regression_main.cpp b/src/mlpack/methods/linear_regression/linear_regression_main.cpp index 19c418151c..d98a385f9c 100644 --- a/src/mlpack/methods/linear_regression/linear_regression_main.cpp +++ b/src/mlpack/methods/linear_regression/linear_regression_main.cpp @@ -96,7 +96,7 @@ static void mlpackMain() mat regressors; rowvec responses; - LinearRegression lr; + LinearRegression* lr; const bool computeModel = !CLI::HasParam("input_model"); const bool computePrediction = CLI::HasParam("test"); @@ -148,14 +148,14 @@ static void mlpackMain() } Timer::Start("regression"); - lr = LinearRegression(regressors, responses, lambda); + lr = new LinearRegression(regressors, responses, lambda); Timer::Stop("regression"); } else { // A model file was passed in, so load it. Timer::Start("load_model"); - lr = std::move(CLI::GetParam("input_model")); + lr = CLI::GetParam("input_model"); Timer::Stop("load_model"); } @@ -168,9 +168,9 @@ static void mlpackMain() Timer::Stop("load_test_points"); // Ensure that test file data has the right number of features. - if ((lr.Parameters().n_elem - 1) != points.n_rows) + if ((lr->Parameters().n_elem - 1) != points.n_rows) { - Log::Fatal << "The model was trained on " << lr.Parameters().n_elem - 1 + Log::Fatal << "The model was trained on " << lr->Parameters().n_elem - 1 << "-dimensional data, but the test points in '" << CLI::GetPrintableParam("test") << "' are " << points.n_rows << "-dimensional!" << endl; @@ -179,15 +179,13 @@ static void mlpackMain() // Perform the predictions using our model. rowvec predictions; Timer::Start("prediction"); - lr.Predict(points, predictions); + lr->Predict(points, predictions); Timer::Stop("prediction"); // Save predictions. - if (CLI::HasParam("output_predictions")) - CLI::GetParam("output_predictions") = std::move(predictions); + CLI::GetParam("output_predictions") = std::move(predictions); } // Save the model if needed. - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(lr); + CLI::GetParam("output_model") = lr; } diff --git a/src/mlpack/methods/local_coordinate_coding/local_coordinate_coding_main.cpp b/src/mlpack/methods/local_coordinate_coding/local_coordinate_coding_main.cpp index cdfc34c258..f0ee7e6dc5 100644 --- a/src/mlpack/methods/local_coordinate_coding/local_coordinate_coding_main.cpp +++ b/src/mlpack/methods/local_coordinate_coding/local_coordinate_coding_main.cpp @@ -120,9 +120,11 @@ static void mlpackMain() ReportIgnoredParam({{ "training", false }}, "tolerance"); // Do we have an existing model? - LocalCoordinateCoding lcc(0, 0.0); + LocalCoordinateCoding* lcc; if (CLI::HasParam("input_model")) - lcc = std::move(CLI::GetParam("input_model")); + lcc = CLI::GetParam("input_model"); + else + lcc = new LocalCoordinateCoding(0, 0.0); if (CLI::HasParam("training")) { @@ -136,10 +138,10 @@ static void mlpackMain() matX.col(i) /= norm(matX.col(i), 2); } - lcc.Lambda() = CLI::GetParam("lambda"); - lcc.Atoms() = (size_t) CLI::GetParam("atoms"); - lcc.MaxIterations() = (size_t) CLI::GetParam("max_iterations"); - lcc.Tolerance() = CLI::GetParam("tolerance"); + lcc->Lambda() = CLI::GetParam("lambda"); + lcc->Atoms() = (size_t) CLI::GetParam("atoms"); + lcc->MaxIterations() = (size_t) CLI::GetParam("max_iterations"); + lcc->Tolerance() = CLI::GetParam("tolerance"); // Inform the user if we are overwriting their model. if (CLI::HasParam("input_model")) @@ -147,35 +149,35 @@ static void mlpackMain() Log::Info << "Using dictionary from existing model in '" << CLI::GetPrintableParam("input_model") << "' as initial " << "dictionary for training." << endl; - lcc.Train(matX); + lcc->Train(matX); } else if (CLI::HasParam("initial_dictionary")) { // Load initial dictionary directly into LCC object. - lcc.Dictionary() = std::move(CLI::GetParam("initial_dictionary")); + lcc->Dictionary() = std::move(CLI::GetParam("initial_dictionary")); // Validate the size of the initial dictionary. - if (lcc.Dictionary().n_cols != lcc.Atoms()) + if (lcc->Dictionary().n_cols != lcc->Atoms()) { - Log::Fatal << "The initial dictionary has " << lcc.Dictionary().n_cols + Log::Fatal << "The initial dictionary has " << lcc->Dictionary().n_cols << " atoms, but the number of atoms was specified to be " - << lcc.Atoms() << "!" << endl; + << lcc->Atoms() << "!" << endl; } - if (lcc.Dictionary().n_rows != matX.n_rows) + if (lcc->Dictionary().n_rows != matX.n_rows) { - Log::Fatal << "The initial dictionary has " << lcc.Dictionary().n_rows + Log::Fatal << "The initial dictionary has " << lcc->Dictionary().n_rows << " dimensions, but the data has " << matX.n_rows << " dimensions!" << endl; } // Train the model. - lcc.Train(matX); + lcc->Train(matX); } else { // Run with the default initialization. - lcc.Train(matX); + lcc->Train(matX); } } @@ -184,9 +186,9 @@ static void mlpackMain() { mat matY = std::move(CLI::GetParam("test")); - if (matY.n_rows != lcc.Dictionary().n_rows) + if (matY.n_rows != lcc->Dictionary().n_rows) Log::Fatal << "Model was trained with a dimensionality of " - << lcc.Dictionary().n_rows << ", but data in test file " + << lcc->Dictionary().n_rows << ", but data in test file " << CLI::GetPrintableParam("test") << " has a dimensionality of " << matY.n_rows << "!" << endl; @@ -199,17 +201,12 @@ static void mlpackMain() } mat codes; - lcc.Encode(matY, codes); + lcc->Encode(matY, codes); - if (CLI::HasParam("codes")) - CLI::GetParam("codes") = std::move(codes); + CLI::GetParam("codes") = std::move(codes); } - // Did the user want to save the dictionary? - if (CLI::HasParam("dictionary")) - CLI::GetParam("dictionary") = std::move(lcc.Dictionary()); - - // Did the user want to save the model? - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(lcc); + // Save the dictionary and the model. + CLI::GetParam("dictionary") = lcc->Dictionary(); + CLI::GetParam("output_model") = lcc; } diff --git a/src/mlpack/methods/logistic_regression/logistic_regression_main.cpp b/src/mlpack/methods/logistic_regression/logistic_regression_main.cpp index 02a12022e3..0720dd028e 100644 --- a/src/mlpack/methods/logistic_regression/logistic_regression_main.cpp +++ b/src/mlpack/methods/logistic_regression/logistic_regression_main.cpp @@ -206,16 +206,18 @@ static void mlpackMain() regressors = std::move(CLI::GetParam("training")); // Load the model, if necessary. - LogisticRegression<> model(0, 0); // Empty model. + LogisticRegression<>* model; if (CLI::HasParam("input_model")) - model = std::move(CLI::GetParam>("input_model")); + model = CLI::GetParam*>("input_model"); else { + model = new LogisticRegression<>(0, 0); + // Set the size of the parameters vector, if necessary. if (!CLI::HasParam("labels")) - model.Parameters() = arma::zeros(regressors.n_rows); + model->Parameters() = arma::zeros(regressors.n_rows); else - model.Parameters() = arma::zeros(regressors.n_rows + 1); + model->Parameters() = arma::zeros(regressors.n_rows + 1); } // Check if the responses are in a separate file. @@ -242,7 +244,7 @@ static void mlpackMain() // Now, do the training. if (CLI::HasParam("training")) { - model.Lambda() = lambda; + model->Lambda() = lambda; if (optimizerType == "sgd") { @@ -254,7 +256,7 @@ static void mlpackMain() Log::Info << "Training model with SGD optimizer." << endl; // This will train the model. - model.Train(regressors, responses, sgdOpt); + model->Train(regressors, responses, sgdOpt); } else if (optimizerType == "lbfgs") { @@ -264,7 +266,7 @@ static void mlpackMain() Log::Info << "Training model with L-BFGS optimizer." << endl; // This will train the model. - model.Train(regressors, responses, lbfgsOpt); + model->Train(regressors, responses, lbfgsOpt); } } @@ -278,7 +280,7 @@ static void mlpackMain() { Log::Info << "Predicting classes of points in '" << CLI::GetPrintableParam("test") << "'." << endl; - model.Classify(testSet, predictions, decisionBoundary); + model->Classify(testSet, predictions, decisionBoundary); CLI::GetParam>("output") = std::move(predictions); } @@ -288,13 +290,12 @@ static void mlpackMain() Log::Info << "Calculating class probabilities of points in '" << CLI::GetPrintableParam("test") << "'." << endl; arma::mat probabilities; - model.Classify(testSet, probabilities); + model->Classify(testSet, probabilities); CLI::GetParam("output_probabilities") = std::move(probabilities); } } - if (CLI::HasParam("output_model")) - CLI::GetParam>("output_model") = std::move(model); + CLI::GetParam*>("output_model") = model; } diff --git a/src/mlpack/methods/lsh/lsh_main.cpp b/src/mlpack/methods/lsh/lsh_main.cpp index f9ce2de8b5..eec2049daa 100644 --- a/src/mlpack/methods/lsh/lsh_main.cpp +++ b/src/mlpack/methods/lsh/lsh_main.cpp @@ -154,9 +154,10 @@ static void mlpackMain() Log::Info << "Using LSH with " << numProj << " projections (K) and " << numTables << " tables (L) with hash width(r): " << hashWidth << endl; - LSHSearch<> allkann; + LSHSearch<>* allkann; if (CLI::HasParam("reference")) { + allkann = new LSHSearch<>(); referenceData = std::move(CLI::GetParam("reference")); Log::Info << "Using reference data from '" << CLI::GetPrintableParam("reference") << "' (" @@ -164,13 +165,13 @@ static void mlpackMain() << endl; Timer::Start("hash_building"); - allkann.Train(std::move(referenceData), numProj, numTables, hashWidth, + allkann->Train(std::move(referenceData), numProj, numTables, hashWidth, secondHashSize, bucketSize); Timer::Stop("hash_building"); } else if (CLI::HasParam("input_model")) { - allkann = std::move(CLI::GetParam>("input_model")); + allkann = CLI::GetParam*>("input_model"); } if (CLI::HasParam("k")) @@ -184,11 +185,11 @@ static void mlpackMain() << CLI::GetPrintableParam("query") << "' (" << queryData.n_rows << " x " << queryData.n_cols << ")." << endl; - allkann.Search(queryData, k, neighbors, distances, 0, numProbes); + allkann->Search(queryData, k, neighbors, distances, 0, numProbes); } else { - allkann.Search(k, neighbors, distances, 0, numProbes); + allkann->Search(k, neighbors, distances, 0, numProbes); } Log::Info << "Neighbors computed." << endl; @@ -205,20 +206,17 @@ static void mlpackMain() << endl; // Compute recall and print it. - double recallPercentage = 100 * allkann.ComputeRecall(neighbors, + double recallPercentage = 100 * allkann->ComputeRecall(neighbors, trueNeighbors); Log::Info << "Recall: " << recallPercentage << endl; } - // Save output, if desired. + // Save output, if we did a search.. if (CLI::HasParam("k")) { - if (CLI::HasParam("distances")) - CLI::GetParam("distances") = std::move(distances); - if (CLI::HasParam("neighbors")) - CLI::GetParam>("neighbors") = std::move(neighbors); + CLI::GetParam("distances") = std::move(distances); + CLI::GetParam>("neighbors") = std::move(neighbors); } - if (CLI::HasParam("output_model")) - CLI::GetParam>("output_model") = std::move(allkann); + CLI::GetParam*>("output_model") = allkann; } diff --git a/src/mlpack/methods/naive_bayes/nbc_main.cpp b/src/mlpack/methods/naive_bayes/nbc_main.cpp index 99b76cb7e7..68a4a70e04 100644 --- a/src/mlpack/methods/naive_bayes/nbc_main.cpp +++ b/src/mlpack/methods/naive_bayes/nbc_main.cpp @@ -118,9 +118,10 @@ static void mlpackMain() Log::Warn << "No test set given; no task will be performed!" << std::endl; // Either we have to train a model, or load a model. - NBCModel model; + NBCModel* model; if (CLI::HasParam("training")) { + model = new NBCModel(); mat trainingData = std::move(CLI::GetParam("training")); Row labels; @@ -130,7 +131,7 @@ static void mlpackMain() { // Load labels. Row rawLabels = std::move(CLI::GetParam>("labels")); - data::NormalizeLabels(rawLabels, labels, model.mappings); + data::NormalizeLabels(rawLabels, labels, model->mappings); } else { @@ -138,7 +139,7 @@ static void mlpackMain() Log::Info << "Using last dimension of training data as training labels." << endl; data::NormalizeLabels(trainingData.row(trainingData.n_rows - 1), labels, - model.mappings); + model->mappings); // Remove the label row. trainingData.shed_row(trainingData.n_rows - 1); } @@ -146,14 +147,14 @@ static void mlpackMain() const bool incrementalVariance = CLI::HasParam("incremental_variance"); Timer::Start("nbc_training"); - model.nbc = NaiveBayesClassifier<>(trainingData, labels, - model.mappings.n_elem, incrementalVariance); + model->nbc = NaiveBayesClassifier<>(trainingData, labels, + model->mappings.n_elem, incrementalVariance); Timer::Stop("nbc_training"); } else { // Load the model from file. - model = std::move(CLI::GetParam("input_model")); + model = CLI::GetParam("input_model"); } // Do we need to do testing? @@ -161,10 +162,10 @@ static void mlpackMain() { mat testingData = std::move(CLI::GetParam("test")); - if (testingData.n_rows != model.nbc.Means().n_rows) + if (testingData.n_rows != model->nbc.Means().n_rows) { Log::Fatal << "Test data dimensionality (" << testingData.n_rows << ") " - << "must be the same as training data (" << model.nbc.Means().n_rows + << "must be the same as training data (" << model->nbc.Means().n_rows << ")!" << std::endl; } @@ -172,23 +173,21 @@ static void mlpackMain() Row predictions; mat probabilities; Timer::Start("nbc_testing"); - model.nbc.Classify(testingData, predictions, probabilities); + model->nbc.Classify(testingData, predictions, probabilities); Timer::Stop("nbc_testing"); if (CLI::HasParam("output")) { // Un-normalize labels to prepare output. Row rawResults; - data::RevertLabels(predictions, model.mappings, rawResults); + data::RevertLabels(predictions, model->mappings, rawResults); // Output results. CLI::GetParam>("output") = std::move(rawResults); } - if (CLI::HasParam("output_probs")) - CLI::GetParam("output_probs") = probabilities; + CLI::GetParam("output_probs") = probabilities; } - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(model); + CLI::GetParam("output_model") = model; } diff --git a/src/mlpack/methods/neighbor_search/kfn_main.cpp b/src/mlpack/methods/neighbor_search/kfn_main.cpp index d6a5d3d0d7..7488be23bd 100644 --- a/src/mlpack/methods/neighbor_search/kfn_main.cpp +++ b/src/mlpack/methods/neighbor_search/kfn_main.cpp @@ -157,7 +157,7 @@ static void mlpackMain() epsilon = 1 - percentage; // We either have to load the reference data, or we have to load the model. - NSModel kfn; + NSModel* kfn; const string algorithm = CLI::GetParam("algorithm"); RequireParamInSet("algorithm", { "naive", "single_tree", "dual_tree", @@ -175,6 +175,8 @@ static void mlpackMain() if (CLI::HasParam("reference")) { + kfn = new KFNModel(); + // Get all the parameters. RequireParamInSet("tree_type", { "kd", "cover", "r", "r-star", "ball", "x", "hilbert-r", "r-plus", "r-plus-plus", "vp", "rp", "max-rp", @@ -212,8 +214,8 @@ static void mlpackMain() else if (treeType == "oct") tree = KFNModel::OCTREE; - kfn.TreeType() = tree; - kfn.RandomBasis() = randomBasis; + kfn->TreeType() = tree; + kfn->RandomBasis() = randomBasis; arma::mat referenceSet = std::move(CLI::GetParam("reference")); @@ -221,27 +223,27 @@ static void mlpackMain() << CLI::GetPrintableParam("reference") << "' (" << referenceSet.n_rows << "x" << referenceSet.n_cols << ")." << endl; - kfn.BuildModel(std::move(referenceSet), size_t(lsInt), searchMode, epsilon); + kfn->BuildModel(std::move(referenceSet), size_t(lsInt), searchMode, epsilon); } else { // Load the model from file. - kfn = std::move(CLI::GetParam("input_model")); + kfn = CLI::GetParam("input_model"); // Adjust search mode. - kfn.SearchMode() = searchMode; - kfn.Epsilon() = epsilon; + kfn->SearchMode() = searchMode; + kfn->Epsilon() = epsilon; // If leaf_size wasn't provided, let's consider the current value in the // loaded model. Else, update it (only considered when building the query // tree). if (CLI::HasParam("leaf_size")) - kfn.LeafSize() = size_t(lsInt); + kfn->LeafSize() = size_t(lsInt); Log::Info << "Using kFN model from '" - << CLI::GetPrintableParam("input_model") << "' (trained on " - << kfn.Dataset().n_rows << "x" << kfn.Dataset().n_cols << " dataset)." - << endl; + << CLI::GetPrintableParam("input_model") << "' (trained on " + << kfn->Dataset().n_rows << "x" << kfn->Dataset().n_cols + << " dataset)." << endl; } // Perform search, if desired. @@ -261,11 +263,11 @@ static void mlpackMain() // Sanity check on k value: must be greater than 0, must be less than the // number of reference points. Since it is unsigned, we only test the upper // bound. - if (k > kfn.Dataset().n_cols) + if (k > kfn->Dataset().n_cols) { Log::Fatal << "Invalid k: " << k << "; must be greater than 0 and less " << "than or equal to the number of reference points (" - << kfn.Dataset().n_cols << ")." << endl; + << kfn->Dataset().n_cols << ")." << endl; } // Now run the search. @@ -273,21 +275,19 @@ static void mlpackMain() arma::mat distances; if (CLI::HasParam("query")) - kfn.Search(std::move(queryData), k, neighbors, distances); + kfn->Search(std::move(queryData), k, neighbors, distances); else - kfn.Search(k, neighbors, distances); + kfn->Search(k, neighbors, distances); Log::Info << "Search complete." << endl; - // Save output, if desired. - if (CLI::HasParam("neighbors")) - CLI::GetParam>("neighbors") = std::move(neighbors); - if (CLI::HasParam("distances")) - CLI::GetParam("distances") = std::move(distances); + // Save output. + CLI::GetParam>("neighbors") = std::move(neighbors); + CLI::GetParam("distances") = std::move(distances); // Calculate the effective error, if desired. if (CLI::HasParam("true_distances")) { - if (kfn.Epsilon() == 0) + if (kfn->Epsilon() == 0) Log::Warn << PRINT_PARAM_STRING("true_distances") << " specified, but " << "the search is exact, so there is no need to calculate the " << "error!" << endl; @@ -307,7 +307,7 @@ static void mlpackMain() // Calculate the recall, if desired. if (CLI::HasParam("true_neighbors")) { - if (kfn.Epsilon() == 0) + if (kfn->Epsilon() == 0) Log::Warn << PRINT_PARAM_STRING("true_neighbors") << " specified, but " << "the search is exact, so there is no need to calculate the " << "recall!" << endl; @@ -324,6 +324,5 @@ static void mlpackMain() } } - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(kfn); + CLI::GetParam("output_model") = kfn; } diff --git a/src/mlpack/methods/neighbor_search/knn_main.cpp b/src/mlpack/methods/neighbor_search/knn_main.cpp index 9c43abceaf..aae9ff699a 100644 --- a/src/mlpack/methods/neighbor_search/knn_main.cpp +++ b/src/mlpack/methods/neighbor_search/knn_main.cpp @@ -166,7 +166,7 @@ static void mlpackMain() "epsilon must be positive"); // We either have to load the reference data, or we have to load the model. - KNNModel knn; + KNNModel* knn; const string algorithm = CLI::GetParam("algorithm"); RequireParamInSet("algorithm", { "naive", "single_tree", "dual_tree", @@ -184,6 +184,8 @@ static void mlpackMain() if (CLI::HasParam("reference")) { + knn = new KNNModel(); + // Get all the parameters. const string treeType = CLI::GetParam("tree_type"); const bool randomBasis = CLI::HasParam("random_basis"); @@ -223,11 +225,11 @@ static void mlpackMain() else if (treeType == "oct") tree = KNNModel::OCTREE; - knn.TreeType() = tree; - knn.RandomBasis() = randomBasis; - knn.LeafSize() = size_t(lsInt); - knn.Tau() = tau; - knn.Rho() = rho; + knn->TreeType() = tree; + knn->RandomBasis() = randomBasis; + knn->LeafSize() = size_t(lsInt); + knn->Tau() = tau; + knn->Rho() = rho; arma::mat referenceSet = std::move(CLI::GetParam("reference")); @@ -236,27 +238,27 @@ static void mlpackMain() << referenceSet.n_rows << " x " << referenceSet.n_cols << ")." << endl; - knn.BuildModel(std::move(referenceSet), size_t(lsInt), searchMode, epsilon); + knn->BuildModel(std::move(referenceSet), size_t(lsInt), searchMode, epsilon); } else { // Load the model from file. - knn = std::move(CLI::GetParam("input_model")); + knn = CLI::GetParam("input_model"); // Adjust search mode. - knn.SearchMode() = searchMode; - knn.Epsilon() = epsilon; + knn->SearchMode() = searchMode; + knn->Epsilon() = epsilon; // If leaf_size wasn't provided, let's consider the current value in the // loaded model. Else, update it (only considered when building the query // tree). if (CLI::HasParam("leaf_size")) - knn.LeafSize() = size_t(lsInt); + knn->LeafSize() = size_t(lsInt); Log::Info << "Loaded kNN model from '" - << CLI::GetPrintableParam("input_model") << "' (trained on " - << knn.Dataset().n_rows << "x" << knn.Dataset().n_cols << " dataset)." - << endl; + << CLI::GetPrintableParam("input_model") << "' (trained on " + << knn->Dataset().n_rows << "x" << knn->Dataset().n_cols + << " dataset)." << endl; } // Perform search, if desired. @@ -276,11 +278,11 @@ static void mlpackMain() // Sanity check on k value: must be greater than 0, must be less than the // number of reference points. Since it is unsigned, we only test the upper // bound. - if (k > knn.Dataset().n_cols) + if (k > knn->Dataset().n_cols) { Log::Fatal << "Invalid k: " << k << "; must be greater than 0 and less "; Log::Fatal << "than or equal to the number of reference points ("; - Log::Fatal << knn.Dataset().n_cols << ")." << endl; + Log::Fatal << knn->Dataset().n_cols << ")." << endl; } // Now run the search. @@ -288,21 +290,19 @@ static void mlpackMain() arma::mat distances; if (CLI::HasParam("query")) - knn.Search(std::move(queryData), k, neighbors, distances); + knn->Search(std::move(queryData), k, neighbors, distances); else - knn.Search(k, neighbors, distances); + knn->Search(k, neighbors, distances); Log::Info << "Search complete." << endl; - // Save output, if desired. - if (CLI::HasParam("neighbors")) - CLI::GetParam>("neighbors") = std::move(neighbors); - if (CLI::HasParam("distances")) - CLI::GetParam("distances") = std::move(distances); + // Save output. + CLI::GetParam>("neighbors") = std::move(neighbors); + CLI::GetParam("distances") = std::move(distances); // Calculate the effective error, if desired. if (CLI::HasParam("true_distances")) { - if (knn.TreeType() != KNNModel::SPILL_TREE && knn.Epsilon() == 0) + if (knn->TreeType() != KNNModel::SPILL_TREE && knn->Epsilon() == 0) Log::Warn << PRINT_PARAM_STRING("true_distances") << "specified, but " << "the search is exact, so there is no need to calculate the " << "error!" << endl; @@ -322,7 +322,7 @@ static void mlpackMain() // Calculate the recall, if desired. if (CLI::HasParam("true_neighbors")) { - if (knn.TreeType() != KNNModel::SPILL_TREE && knn.Epsilon() == 0) + if (knn->TreeType() != KNNModel::SPILL_TREE && knn->Epsilon() == 0) Log::Warn << PRINT_PARAM_STRING("true_neighbors") << " specified, but " << " the search is exact, so there is no need to calculate the " << "recall!" << endl; @@ -339,6 +339,5 @@ static void mlpackMain() } } - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(knn); + CLI::GetParam("output_model") = knn; } diff --git a/src/mlpack/methods/perceptron/perceptron_main.cpp b/src/mlpack/methods/perceptron/perceptron_main.cpp index c7a5b17796..0f2e18e0f8 100644 --- a/src/mlpack/methods/perceptron/perceptron_main.cpp +++ b/src/mlpack/methods/perceptron/perceptron_main.cpp @@ -137,14 +137,18 @@ static void mlpackMain() true, "maximum number of iterations must be nonnegative"); // Now, load our model, if there is one. - PerceptronModel p; + PerceptronModel* p; if (CLI::HasParam("input_model")) { Log::Info << "Using saved perceptron from " - << CLI::GetPrintableParam("input_model") << "." + << CLI::GetPrintableParam("input_model") << "." << endl; - p = std::move(CLI::GetParam("input_model")); + p = CLI::GetParam("input_model"); + } + else + { + p = new PerceptronModel(); } // Next, load the training data and labels (if they have been given). @@ -186,8 +190,8 @@ static void mlpackMain() // Normalize the labels. Row labels; - data::NormalizeLabels(labelsIn, labels, p.Map()); - const size_t numClasses = p.Map().n_elem; + data::NormalizeLabels(labelsIn, labels, p->Map()); + const size_t numClasses = p->Map().n_elem; // Now, if we haven't already created a perceptron, do it. Otherwise, make // sure the dimensions are right, then continue training. @@ -195,35 +199,35 @@ static void mlpackMain() { // Create and train the classifier. Timer::Start("training"); - p.P() = Perceptron<>(trainingData, labels, numClasses, maxIterations); + p->P() = Perceptron<>(trainingData, labels, numClasses, maxIterations); Timer::Stop("training"); } else { // Check dimensionality. - if (p.P().Weights().n_rows != trainingData.n_rows) + if (p->P().Weights().n_rows != trainingData.n_rows) { Log::Fatal << "Perceptron from '" - << CLI::GetPrintableParam("input_model") - << "' is built on data with " << p.P().Weights().n_rows + << CLI::GetPrintableParam("input_model") + << "' is built on data with " << p->P().Weights().n_rows << " dimensions, but data in '" << CLI::GetPrintableParam("training") << "' has " << trainingData.n_rows << "dimensions!" << endl; } // Check the number of labels. - if (numClasses > p.P().Weights().n_cols) + if (numClasses > p->P().Weights().n_cols) { Log::Fatal << "Perceptron from '" - << CLI::GetPrintableParam("input_model") << "' " - << "has " << p.P().Weights().n_cols << " classes, but the training " - << "data has " << numClasses + 1 << " classes!" << endl; + << CLI::GetPrintableParam("input_model") << "' " + << "has " << p->P().Weights().n_cols << " classes, but the training" + << " data has " << numClasses + 1 << " classes!" << endl; } // Now train. Timer::Start("training"); - p.P().MaxIterations() = maxIterations; - p.P().Train(trainingData, labels.t(), numClasses); + p->P().MaxIterations() = maxIterations; + p->P().Train(trainingData, labels.t(), numClasses); Timer::Stop("training"); } } @@ -235,29 +239,28 @@ static void mlpackMain() << CLI::GetPrintableParam("test") << "'." << endl; mat testData = std::move(CLI::GetParam("test")); - if (testData.n_rows != p.P().Weights().n_rows) + if (testData.n_rows != p->P().Weights().n_rows) { Log::Fatal << "Test data dimensionality (" << testData.n_rows << ") must " << "be the same as the dimensionality of the perceptron (" - << p.P().Weights().n_rows << ")!" << endl; + << p->P().Weights().n_rows << ")!" << endl; } // Time the running of the perceptron classifier. Row predictedLabels(testData.n_cols); Timer::Start("testing"); - p.P().Classify(testData, predictedLabels); + p->P().Classify(testData, predictedLabels); Timer::Stop("testing"); // Un-normalize labels to prepare output. Row results; - data::RevertLabels(predictedLabels, p.Map(), results); + data::RevertLabels(predictedLabels, p->Map(), results); // Save the predicted labels. if (CLI::HasParam("output")) CLI::GetParam>("output") = std::move(results); } - // Lastly, do we need to save the output model? - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(p); + // Lastly, save the output model. + CLI::GetParam("output_model") = p; } diff --git a/src/mlpack/methods/random_forest/random_forest_main.cpp b/src/mlpack/methods/random_forest/random_forest_main.cpp index e6cdfd5aa3..df38516fb9 100644 --- a/src/mlpack/methods/random_forest/random_forest_main.cpp +++ b/src/mlpack/methods/random_forest/random_forest_main.cpp @@ -104,9 +104,11 @@ static void mlpackMain() ReportIgnoredParam({{ "training", false }}, "num_trees"); ReportIgnoredParam({{ "training", false }}, "minimum_leaf_size"); - RandomForestModel rfModel; + RandomForestModel* rfModel; if (CLI::HasParam("training")) { + rfModel = new RandomForestModel(); + // Train the model on the given input data. arma::mat data = std::move(CLI::GetParam("training")); arma::Row labels = @@ -121,13 +123,13 @@ static void mlpackMain() const size_t numClasses = arma::max(labels) + 1; // Train the model. - rfModel.rf.Train(data, labels, numClasses, numTrees, minimumLeafSize); + rfModel->rf.Train(data, labels, numClasses, numTrees, minimumLeafSize); // Did we want training accuracy? if (CLI::HasParam("print_training_accuracy")) { arma::Row predictions; - rfModel.rf.Classify(data, predictions); + rfModel->rf.Classify(data, predictions); const size_t correct = arma::accu(predictions == labels); @@ -139,7 +141,7 @@ static void mlpackMain() else { // Then we must be loading a model. - rfModel = std::move(CLI::GetParam("input_model")); + rfModel = CLI::GetParam("input_model"); } if (CLI::HasParam("test")) @@ -149,7 +151,7 @@ static void mlpackMain() // Get predictions and probabilities. arma::Row predictions; arma::mat probabilities; - rfModel.rf.Classify(testData, predictions, probabilities); + rfModel->rf.Classify(testData, predictions, probabilities); // Did we want to calculate test accuracy? if (CLI::HasParam("test_labels")) @@ -164,14 +166,11 @@ static void mlpackMain() << ")." << endl; } - // Should we save the outputs? - if (CLI::HasParam("probabilities")) - CLI::GetParam("probabilities") = std::move(probabilities); - if (CLI::HasParam("predictions")) - CLI::GetParam>("predictions") = std::move(predictions); + // Save the outputs. + CLI::GetParam("probabilities") = std::move(probabilities); + CLI::GetParam>("predictions") = std::move(predictions); } - // Did the user want to save the output model? - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(rfModel); + // Save the output model. + CLI::GetParam("output_model") = rfModel; } diff --git a/src/mlpack/methods/range_search/range_search_main.cpp b/src/mlpack/methods/range_search/range_search_main.cpp index 66a070e6d6..27e49caf19 100644 --- a/src/mlpack/methods/range_search/range_search_main.cpp +++ b/src/mlpack/methods/range_search/range_search_main.cpp @@ -136,11 +136,13 @@ static void mlpackMain() "leaf size must be greater than 0"); // We either have to load the reference data, or we have to load the model. - RSModel rs; + RSModel* rs; const bool naive = CLI::HasParam("naive"); const bool singleMode = CLI::HasParam("single_mode"); if (CLI::HasParam("reference")) { + rs = new RSModel(); + // Get all the parameters. const string treeType = CLI::GetParam("tree_type"); RequireParamInSet("tree_type", { "kd", "cover", "r", "r-star", @@ -178,8 +180,8 @@ static void mlpackMain() else if (treeType == "oct") tree = RSModel::OCTREE; - rs.TreeType() = tree; - rs.RandomBasis() = randomBasis; + rs->TreeType() = tree; + rs->RandomBasis() = randomBasis; arma::mat referenceSet = std::move(CLI::GetParam("reference")); @@ -189,22 +191,22 @@ static void mlpackMain() const size_t leafSize = size_t(lsInt); - rs.BuildModel(std::move(referenceSet), leafSize, naive, singleMode); + rs->BuildModel(std::move(referenceSet), leafSize, naive, singleMode); } else { // Load the model from file. - rs = std::move(CLI::GetParam("input_model")); + rs = CLI::GetParam("input_model"); Log::Info << "Using range search model from '" << CLI::GetPrintableParam("input_model") << "' (" - << "trained on " << rs.Dataset().n_rows << "x" << rs.Dataset().n_cols + << "trained on " << rs->Dataset().n_rows << "x" << rs->Dataset().n_cols << " dataset)." << endl; // Adjust singleMode and naive if necessary. - rs.SingleMode() = CLI::HasParam("single_mode"); - rs.Naive() = CLI::HasParam("naive"); - rs.LeafSize() = size_t(lsInt); + rs->SingleMode() = CLI::HasParam("single_mode"); + rs->Naive() = CLI::HasParam("naive"); + rs->LeafSize() = size_t(lsInt); } // Perform search, if desired. @@ -235,9 +237,9 @@ static void mlpackMain() vector> distances; if (CLI::HasParam("query")) - rs.Search(std::move(queryData), r, neighbors, distances); + rs->Search(std::move(queryData), r, neighbors, distances); else - rs.Search(r, neighbors, distances); + rs->Search(r, neighbors, distances); Log::Info << "Search complete." << endl; @@ -301,7 +303,6 @@ static void mlpackMain() } } - // Save the output model, if desired. - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(rs); + // Save the output model. + CLI::GetParam("output_model") = rs; } diff --git a/src/mlpack/methods/rann/krann_main.cpp b/src/mlpack/methods/rann/krann_main.cpp index dd7fdef538..703fdd0850 100644 --- a/src/mlpack/methods/rann/krann_main.cpp +++ b/src/mlpack/methods/rann/krann_main.cpp @@ -135,11 +135,13 @@ static void mlpackMain() "leaf size must be greater than 0"); // We either have to load the reference data, or we have to load the model. - RANNModel rann; + RANNModel* rann; const bool naive = CLI::HasParam("naive"); const bool singleMode = CLI::HasParam("single_mode"); if (CLI::HasParam("reference")) { + rann = new RANNModel(); + // Get all the parameters. const string treeType = CLI::GetParam("tree_type"); RequireParamInSet("tree_type", { "kd", "cover", "r", "r-star", "x", @@ -169,8 +171,8 @@ static void mlpackMain() else if (treeType == "oct") tree = RANNModel::OCTREE; - rann.TreeType() = tree; - rann.RandomBasis() = randomBasis; + rann->TreeType() = tree; + rann->RandomBasis() = randomBasis; arma::mat referenceSet = std::move(CLI::GetParam("reference")); @@ -179,33 +181,33 @@ static void mlpackMain() << referenceSet.n_rows << " x " << referenceSet.n_cols << ")." << endl; - rann.BuildModel(std::move(referenceSet), size_t(lsInt), naive, singleMode); + rann->BuildModel(std::move(referenceSet), size_t(lsInt), naive, singleMode); } else { // Load the model from file. - rann = std::move(CLI::GetParam("input_model")); + rann = CLI::GetParam("input_model"); Log::Info << "Using rank-approximate kNN model from '" << CLI::GetPrintableParam("input_model") << "' (trained on " - << rann.Dataset().n_rows << "x" << rann.Dataset().n_cols << " dataset)." - << endl; + << rann->Dataset().n_rows << "x" << rann->Dataset().n_cols + << " dataset)." << endl; // Adjust singleMode and naive if necessary. - rann.SingleMode() = CLI::HasParam("single_mode"); - rann.Naive() = CLI::HasParam("naive"); - rann.LeafSize() = size_t(lsInt); + rann->SingleMode() = CLI::HasParam("single_mode"); + rann->Naive() = CLI::HasParam("naive"); + rann->LeafSize() = size_t(lsInt); } // Apply the parameters for search. if (CLI::HasParam("tau")) - rann.Tau() = CLI::GetParam("tau"); + rann->Tau() = CLI::GetParam("tau"); if (CLI::HasParam("alpha")) - rann.Alpha() = CLI::GetParam("alpha"); + rann->Alpha() = CLI::GetParam("alpha"); if (CLI::HasParam("single_sample_limit")) - rann.SingleSampleLimit() = CLI::GetParam("single_sample_limit"); - rann.SampleAtLeaves() = CLI::HasParam("sample_at_leaves"); - rann.FirstLeafExact() = CLI::HasParam("sample_at_leaves"); + rann->SingleSampleLimit() = CLI::GetParam("single_sample_limit"); + rann->SampleAtLeaves() = CLI::HasParam("sample_at_leaves"); + rann->FirstLeafExact() = CLI::HasParam("sample_at_leaves"); // Perform search, if desired. if (CLI::HasParam("k")) @@ -224,28 +226,26 @@ static void mlpackMain() // Sanity check on k value: must be greater than 0, must be less than the // number of reference points. Since it is unsigned, we only test the upper // bound. - if (k > rann.Dataset().n_cols) + if (k > rann->Dataset().n_cols) { Log::Fatal << "Invalid k: " << k << "; must be greater than 0 and less "; Log::Fatal << "than or equal to the number of reference points ("; - Log::Fatal << rann.Dataset().n_cols << ")." << endl; + Log::Fatal << rann->Dataset().n_cols << ")." << endl; } arma::Mat neighbors; arma::mat distances; if (CLI::HasParam("query")) - rann.Search(std::move(queryData), k, neighbors, distances); + rann->Search(std::move(queryData), k, neighbors, distances); else - rann.Search(k, neighbors, distances); + rann->Search(k, neighbors, distances); Log::Info << "Search complete." << endl; - // Save output, if desired. - if (CLI::HasParam("neighbors")) - CLI::GetParam>("neighbors") = std::move(neighbors); - if (CLI::HasParam("distances")) - CLI::GetParam("distances") = std::move(distances); + // Save output. + CLI::GetParam>("neighbors") = std::move(neighbors); + CLI::GetParam("distances") = std::move(distances); } - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(rann); + // Save the output model. + CLI::GetParam("output_model") = rann; } diff --git a/src/mlpack/methods/softmax_regression/softmax_regression_main.cpp b/src/mlpack/methods/softmax_regression/softmax_regression_main.cpp index 78e8c96245..d61723143e 100644 --- a/src/mlpack/methods/softmax_regression/softmax_regression_main.cpp +++ b/src/mlpack/methods/softmax_regression/softmax_regression_main.cpp @@ -113,7 +113,7 @@ void TestClassifyAcc(const size_t numClasses, const Model& model); // Build the softmax model given the parameters. template -unique_ptr TrainSoftmax(const size_t maxIterations); +Model* TrainSoftmax(const size_t maxIterations); static void mlpackMain() { @@ -141,13 +141,11 @@ static void mlpackMain() RequireAtLeastOnePassed({ "output_model", "predictions" }, false, "no results" " will be saved"); - using SM = SoftmaxRegression; - unique_ptr sm = TrainSoftmax(maxIterations); + SoftmaxRegression* sm = TrainSoftmax(maxIterations); TestClassifyAcc(sm->NumClasses(), *sm); - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(*sm); + CLI::GetParam("output_model") = sm; } size_t CalculateNumberOfClasses(const size_t numClasses, @@ -230,17 +228,14 @@ void TestClassifyAcc(size_t numClasses, const Model& model) } template -unique_ptr TrainSoftmax(const size_t maxIterations) +Model* TrainSoftmax(const size_t maxIterations) { using namespace mlpack; - using SRF = regression::SoftmaxRegressionFunction; - - unique_ptr sm; + Model* sm; if (CLI::HasParam("input_model")) { - sm.reset(new Model(0, 0, false)); - *sm = std::move(CLI::GetParam("input_model")); + sm = CLI::GetParam("input_model"); } else { @@ -259,8 +254,8 @@ unique_ptr TrainSoftmax(const size_t maxIterations) const size_t numBasis = 5; optimization::L_BFGS optimizer(numBasis, maxIterations); - sm.reset(new Model(trainData, trainLabels, numClasses, - CLI::GetParam("lambda"), intercept, std::move(optimizer))); + sm = new Model(trainData, trainLabels, numClasses, + CLI::GetParam("lambda"), intercept, std::move(optimizer)); } return sm; diff --git a/src/mlpack/methods/sparse_coding/sparse_coding_main.cpp b/src/mlpack/methods/sparse_coding/sparse_coding_main.cpp index 0ce5bf8336..87a2cd278f 100644 --- a/src/mlpack/methods/sparse_coding/sparse_coding_main.cpp +++ b/src/mlpack/methods/sparse_coding/sparse_coding_main.cpp @@ -146,9 +146,11 @@ static void mlpackMain() "Newton method tolerance must be nonnegative"); // Do we have an existing model? - SparseCoding sc(0, 0.0); + SparseCoding* sc; if (CLI::HasParam("input_model")) - sc = std::move(CLI::GetParam("input_model")); + sc = CLI::GetParam("input_model"); + else + sc = new SparseCoding(0, 0.0); if (CLI::HasParam("training")) { @@ -162,12 +164,12 @@ static void mlpackMain() matX.col(i) /= norm(matX.col(i), 2); } - sc.Lambda1() = CLI::GetParam("lambda1"); - sc.Lambda2() = CLI::GetParam("lambda2"); - sc.MaxIterations() = (size_t) CLI::GetParam("max_iterations"); - sc.Atoms() = (size_t) CLI::GetParam("atoms"); - sc.ObjTolerance() = CLI::GetParam("objective_tolerance"); - sc.NewtonTolerance() = CLI::GetParam("newton_tolerance"); + sc->Lambda1() = CLI::GetParam("lambda1"); + sc->Lambda2() = CLI::GetParam("lambda2"); + sc->MaxIterations() = (size_t) CLI::GetParam("max_iterations"); + sc->Atoms() = (size_t) CLI::GetParam("atoms"); + sc->ObjTolerance() = CLI::GetParam("objective_tolerance"); + sc->NewtonTolerance() = CLI::GetParam("newton_tolerance"); // Inform the user if we are overwriting their model. if (CLI::HasParam("input_model")) @@ -175,36 +177,36 @@ static void mlpackMain() Log::Info << "Using dictionary from existing model in '" << CLI::GetPrintableParam("input_model") << "' as initial dictionary for training." << endl; - sc.Train(matX); + sc->Train(matX); } else if (CLI::HasParam("initial_dictionary")) { // Load initial dictionary directly into sparse coding object. - sc.Dictionary() = + sc->Dictionary() = std::move(CLI::GetParam("initial_dictionary")); // Validate size of initial dictionary. - if (sc.Dictionary().n_cols != sc.Atoms()) + if (sc->Dictionary().n_cols != sc->Atoms()) { - Log::Fatal << "The initial dictionary has " << sc.Dictionary().n_cols + Log::Fatal << "The initial dictionary has " << sc->Dictionary().n_cols << " atoms, but the number of atoms was specified to be " - << sc.Atoms() << "!" << endl; + << sc->Atoms() << "!" << endl; } - if (sc.Dictionary().n_rows != matX.n_rows) + if (sc->Dictionary().n_rows != matX.n_rows) { - Log::Fatal << "The initial dictionary has " << sc.Dictionary().n_rows + Log::Fatal << "The initial dictionary has " << sc->Dictionary().n_rows << " dimensions, but the data has " << matX.n_rows << " dimensions!" << endl; } // Run sparse coding. - sc.Train(matX); + sc->Train(matX); } else { // Run sparse coding with the default initialization. - sc.Train(matX); + sc->Train(matX); } } @@ -213,9 +215,9 @@ static void mlpackMain() { mat matY = std::move(CLI::GetParam("test")); - if (matY.n_rows != sc.Dictionary().n_rows) + if (matY.n_rows != sc->Dictionary().n_rows) Log::Fatal << "Model was trained with a dimensionality of " - << sc.Dictionary().n_rows << ", but test data '" + << sc->Dictionary().n_rows << ", but test data '" << CLI::GetPrintableParam("test") << "' have a " << "dimensionality of " << matY.n_rows << "!" << endl; @@ -228,20 +230,15 @@ static void mlpackMain() } mat codes; - sc.Encode(matY, codes); + sc->Encode(matY, codes); - if (CLI::HasParam("codes")) - CLI::GetParam("codes") = std::move(codes); + CLI::GetParam("codes") = std::move(codes); } - // Did the user want to save the dictionary? If so we can move that, but only - // if we are not also saving an output model. - if (CLI::HasParam("dictionary") && !CLI::HasParam("output_model")) - CLI::GetParam("dictionary") = std::move(sc.Dictionary()); - else if (CLI::HasParam("dictionary")) - CLI::GetParam("dictionary") = sc.Dictionary(); + // Did the user want to save the dictionary? Use an alias for the dictionary. + CLI::GetParam("dictionary") = arma::mat(sc->Dictionary().memptr(), + sc->Dictionary().n_rows, sc->Dictionary().n_cols, false, false); - // Did the user want to save the model? - if (CLI::HasParam("output_model")) - CLI::GetParam("output_model") = std::move(sc); + // Save the model. + CLI::GetParam("output_model") = sc; } From 6b954ebf4c76d97b1a8ce41ca58a1ec9da319131 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 26 Jan 2018 14:02:39 -0500 Subject: [PATCH 11/25] Oops, this snuck in somehow. --- src/mlpack/tests/gmm_test.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mlpack/tests/gmm_test.cpp b/src/mlpack/tests/gmm_test.cpp index 9a694cd12a..f1945c9836 100644 --- a/src/mlpack/tests/gmm_test.cpp +++ b/src/mlpack/tests/gmm_test.cpp @@ -761,7 +761,6 @@ BOOST_AUTO_TEST_CASE(UseExistingModelTest) */ BOOST_AUTO_TEST_CASE(DiagonalGMMTrainTest) { - Log::Warn.ignoreInput = false; // We'll have three diagonal-covariance Gaussian distributions from this // mixture. distribution::GaussianDistribution d1("0.0 1.0 0.0", "1.0 0.0 0.0;" From 6e3f0cb4376715fd866ea05f80649e709d48ae81 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 26 Jan 2018 17:25:05 -0500 Subject: [PATCH 12/25] Remove move.hpp from build configuration. --- src/mlpack/bindings/python/CMakeLists.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mlpack/bindings/python/CMakeLists.txt b/src/mlpack/bindings/python/CMakeLists.txt index be49a1f238..88ec98a949 100644 --- a/src/mlpack/bindings/python/CMakeLists.txt +++ b/src/mlpack/bindings/python/CMakeLists.txt @@ -79,7 +79,6 @@ set(CYTHON_SOURCES mlpack/arma_util.hpp mlpack/cli.pxd mlpack/cli_util.hpp - mlpack/move.hpp mlpack/matrix_utils.py mlpack/serialization.hpp mlpack/serialization.pxd @@ -148,7 +147,6 @@ add_custom_command(TARGET python POST_BUILD mlpack/arma_util.hpp mlpack/cli.pxd mlpack/cli_util.hpp - mlpack/move.hpp mlpack/matrix_utils.py mlpack WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/) From b773276c1799a72fec9e9744be6c87d0380f3548 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 26 Jan 2018 17:26:08 -0500 Subject: [PATCH 13/25] Adapt tests to use pointers to models. --- src/mlpack/bindings/cli/get_param.hpp | 1 - src/mlpack/tests/cli_binding_test.cpp | 27 ++++++++++--------- src/mlpack/tests/cli_test.cpp | 15 ++++++----- .../tests/main_tests/decision_tree_test.cpp | 4 +-- .../main_tests/linear_regression_test.cpp | 6 ++--- 5 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/mlpack/bindings/cli/get_param.hpp b/src/mlpack/bindings/cli/get_param.hpp index 9c16b10f26..4ddfe7f72b 100644 --- a/src/mlpack/bindings/cli/get_param.hpp +++ b/src/mlpack/bindings/cli/get_param.hpp @@ -112,7 +112,6 @@ T*& GetParam( d.loaded = true; std::get<0>(*tuple) = model; } - return std::get<0>(*tuple); } diff --git a/src/mlpack/tests/cli_binding_test.cpp b/src/mlpack/tests/cli_binding_test.cpp index 526bb00545..0aba4f49e4 100644 --- a/src/mlpack/tests/cli_binding_test.cpp +++ b/src/mlpack/tests/cli_binding_test.cpp @@ -238,20 +238,21 @@ BOOST_AUTO_TEST_CASE(GetParamModelTest) data::Save("kernel.bin", "model", gk); // Create tuple. - gk.Bandwidth(2.0); - tuple t = make_tuple(gk, filename); + tuple t = make_tuple((GaussianKernel*) NULL, + filename); d.value = boost::any(t); // Make sure it is not loaded yet. d.input = true; d.loaded = false; - GaussianKernel* output = NULL; - GetParam((const util::ParamData&) d, (void*) NULL, + GaussianKernel** output = NULL; + GetParam((const util::ParamData&) d, (void*) NULL, (void*) &output); - BOOST_REQUIRE_EQUAL(output->Bandwidth(), 5.0); + BOOST_REQUIRE_EQUAL((*output)->Bandwidth(), 5.0); remove("kernel.bin"); + delete *output; } BOOST_AUTO_TEST_CASE(RawParamDoubleTest) @@ -300,17 +301,17 @@ BOOST_AUTO_TEST_CASE(GetRawParamModelTest) kernel::GaussianKernel gk(5.0); // Create tuple. - tuple t = make_tuple(gk, filename); + tuple t = make_tuple(&gk, filename); d.value = boost::any(t); // Make sure it is not loaded yet. d.input = true; d.loaded = false; - tuple* output = NULL; - GetRawParam>((const util::ParamData&) d, + tuple* output = NULL; + GetRawParam>((const util::ParamData&) d, (void*) NULL, (void*) &output); - BOOST_REQUIRE_EQUAL(get<0>(*output).Bandwidth(), 5.0); + BOOST_REQUIRE_EQUAL(get<0>(*output)->Bandwidth(), 5.0); } BOOST_AUTO_TEST_CASE(GetRawParamDatasetInfoTest) @@ -403,7 +404,7 @@ BOOST_AUTO_TEST_CASE(OutputParamModelTest) // Create value. string filename = "kernel.bin"; GaussianKernel gk(5.0); - tuple t = make_tuple(gk, filename); + tuple t = make_tuple(&gk, filename); d.value = boost::any(t); d.input = false; @@ -491,7 +492,7 @@ BOOST_AUTO_TEST_CASE(SetParamModelTest) // Create initial value. string filename = "kernel.bin"; GaussianKernel gk(2.0); - d.value = boost::any(make_tuple(gk, filename)); + d.value = boost::any(make_tuple(&gk, filename)); // Get a new string. string newFilename = "new_kernel.bin"; @@ -501,8 +502,8 @@ BOOST_AUTO_TEST_CASE(SetParamModelTest) (void*) NULL); // Make sure the change went through. - tuple& t = - *boost::any_cast>(&d.value); + tuple& t = + *boost::any_cast>(&d.value); BOOST_REQUIRE_EQUAL(get<1>(t), "new_kernel.bin"); } diff --git a/src/mlpack/tests/cli_test.cpp b/src/mlpack/tests/cli_test.cpp index 144e8c9967..5b44385a6f 100644 --- a/src/mlpack/tests/cli_test.cpp +++ b/src/mlpack/tests/cli_test.cpp @@ -866,9 +866,9 @@ BOOST_AUTO_TEST_CASE(UnmappedParamTest) BOOST_REQUIRE_EQUAL(CLI::GetPrintableParam("matrix"), "file1.csv"); BOOST_REQUIRE_EQUAL(CLI::GetPrintableParam("matrix2"), "file2.csv"); - BOOST_REQUIRE_EQUAL(CLI::GetPrintableParam("kernel"), + BOOST_REQUIRE_EQUAL(CLI::GetPrintableParam("kernel"), "kernel.txt"); - BOOST_REQUIRE_EQUAL(CLI::GetPrintableParam("kernel2"), + BOOST_REQUIRE_EQUAL(CLI::GetPrintableParam("kernel2"), "kernel2.txt"); remove("kernel.txt"); @@ -894,9 +894,9 @@ BOOST_AUTO_TEST_CASE(SerializationTest) ParseCommandLine(argc, const_cast(argv)); // Create the kernel we'll save. - GaussianKernel gk(0.5); + GaussianKernel* gk = new GaussianKernel(0.5); - CLI::GetParam("kernel") = move(gk); + CLI::GetParam("kernel") = gk; // Save it. EndProgram(); @@ -910,9 +910,12 @@ BOOST_AUTO_TEST_CASE(SerializationTest) ParseCommandLine(argc, const_cast(argv)); // Load the kernel from file. - GaussianKernel gk2 = move(CLI::GetParam("kernel")); + GaussianKernel* gk2 = CLI::GetParam("kernel"); - BOOST_REQUIRE_CLOSE(gk2.Bandwidth(), 0.5, 1e-5); + BOOST_REQUIRE_CLOSE(gk2->Bandwidth(), 0.5, 1e-5); + + // Clean up the memory... + delete gk2; // Now remove the file we made. remove("kernel.txt"); diff --git a/src/mlpack/tests/main_tests/decision_tree_test.cpp b/src/mlpack/tests/main_tests/decision_tree_test.cpp index 6ed311689a..3cf1f0761e 100644 --- a/src/mlpack/tests/main_tests/decision_tree_test.cpp +++ b/src/mlpack/tests/main_tests/decision_tree_test.cpp @@ -210,7 +210,7 @@ BOOST_AUTO_TEST_CASE(DecisionModelReuseTest) // Input trained model. SetInputParam("test", std::move(std::make_tuple(info, testData))); SetInputParam("input_model", - std::move(CLI::GetParam("output_model"))); + std::move(CLI::GetParam("output_model"))); mlpackMain(); @@ -281,7 +281,7 @@ BOOST_AUTO_TEST_CASE(DecisionModelCategoricalReuseTest) // Input trained model. SetInputParam("test", std::move(std::make_tuple(info, testData))); SetInputParam("input_model", - std::move(CLI::GetParam("output_model"))); + std::move(CLI::GetParam("output_model"))); mlpackMain(); diff --git a/src/mlpack/tests/main_tests/linear_regression_test.cpp b/src/mlpack/tests/main_tests/linear_regression_test.cpp index e3d0d7993d..28a205aacd 100644 --- a/src/mlpack/tests/main_tests/linear_regression_test.cpp +++ b/src/mlpack/tests/main_tests/linear_regression_test.cpp @@ -135,12 +135,12 @@ BOOST_AUTO_TEST_CASE(LRModelReload) mlpackMain(); - LinearRegression model = CLI::GetParam("output_model"); + LinearRegression* model = CLI::GetParam("output_model"); const arma::rowvec testY1 = CLI::GetParam("output_predictions"); ResetSettings(); - SetInputParam("input_model", std::move(model)); + SetInputParam("input_model", model); SetInputParam("test", std::move(testX)); mlpackMain(); @@ -209,7 +209,7 @@ BOOST_AUTO_TEST_CASE(LRWrongDimOfDataTest2) mlpackMain(); - LinearRegression model = CLI::GetParam("output_model"); + LinearRegression* model = CLI::GetParam("output_model"); ResetSettings(); From 934682ecc54c09c574ddf2ffe96bd41ff2b7a95d Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 26 Jan 2018 18:23:24 -0500 Subject: [PATCH 14/25] Remove code that wasn't needed in the end. --- src/mlpack/bindings/python/get_param.hpp | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/mlpack/bindings/python/get_param.hpp b/src/mlpack/bindings/python/get_param.hpp index e2c1c33108..62c898a15e 100644 --- a/src/mlpack/bindings/python/get_param.hpp +++ b/src/mlpack/bindings/python/get_param.hpp @@ -22,17 +22,7 @@ void GetParam(const util::ParamData& d, const void* /* input */, void* output) { -// typedef typename std::remove_pointer::type TRaw; -// if (std::is_pointer::value) // If true, this is a model. -// { -// std::cout << "get a raw pointer for " << d.name << ": " << boost::any_cast(d.value) << -//"\n"; -// *((TRaw***) output) = const_cast(boost::any_cast(&d.value)); -// } -// else - { - *((T**) output) = const_cast(boost::any_cast(&d.value)); - } + *((T**) output) = const_cast(boost::any_cast(&d.value)); } } // namespace python From cb829ac3046a217b475091993fae0b550988d271 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Fri, 26 Jan 2018 18:24:40 -0500 Subject: [PATCH 15/25] Fix too-long lines. --- src/mlpack/bindings/python/print_input_processing.hpp | 6 +++--- src/mlpack/methods/neighbor_search/kfn_main.cpp | 3 ++- src/mlpack/methods/neighbor_search/knn_main.cpp | 3 ++- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/mlpack/bindings/python/print_input_processing.hpp b/src/mlpack/bindings/python/print_input_processing.hpp index 14d487b3fb..290366f185 100644 --- a/src/mlpack/bindings/python/print_input_processing.hpp +++ b/src/mlpack/bindings/python/print_input_processing.hpp @@ -189,9 +189,9 @@ void PrintInputProcessing( std::cout << prefix << " except TypeError as e:" << std::endl; std::cout << prefix << " if type(" << d.name << ").__name__ == '" << strippedType << "Type':" << std::endl; - std::cout << prefix << " SetParamPtr[" << strippedType << "]('" << d.name - << "', (<" << strippedType << "Type> " << d.name << ").modelptr)" - << std::endl; + std::cout << prefix << " SetParamPtr[" << strippedType << "]('" + << d.name << "', (<" << strippedType << "Type> " << d.name + << ").modelptr)" << std::endl; std::cout << prefix << " else:" << std::endl; std::cout << prefix << " raise e" << std::endl; std::cout << prefix << " CLI.SetPassed( '" << d.name << "')" diff --git a/src/mlpack/methods/neighbor_search/kfn_main.cpp b/src/mlpack/methods/neighbor_search/kfn_main.cpp index 7488be23bd..9d1ebb35bb 100644 --- a/src/mlpack/methods/neighbor_search/kfn_main.cpp +++ b/src/mlpack/methods/neighbor_search/kfn_main.cpp @@ -223,7 +223,8 @@ static void mlpackMain() << CLI::GetPrintableParam("reference") << "' (" << referenceSet.n_rows << "x" << referenceSet.n_cols << ")." << endl; - kfn->BuildModel(std::move(referenceSet), size_t(lsInt), searchMode, epsilon); + kfn->BuildModel(std::move(referenceSet), size_t(lsInt), searchMode, + epsilon); } else { diff --git a/src/mlpack/methods/neighbor_search/knn_main.cpp b/src/mlpack/methods/neighbor_search/knn_main.cpp index aae9ff699a..2fe257700b 100644 --- a/src/mlpack/methods/neighbor_search/knn_main.cpp +++ b/src/mlpack/methods/neighbor_search/knn_main.cpp @@ -238,7 +238,8 @@ static void mlpackMain() << referenceSet.n_rows << " x " << referenceSet.n_cols << ")." << endl; - knn->BuildModel(std::move(referenceSet), size_t(lsInt), searchMode, epsilon); + knn->BuildModel(std::move(referenceSet), size_t(lsInt), searchMode, + epsilon); } else { From 787e0904edcceef887767448002b0404c84ba74b Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sat, 27 Jan 2018 13:13:52 -0500 Subject: [PATCH 16/25] Add a 'copy_all_inputs' option to Python bindings. This is hand-tested as working, but I need to write actual tests still. --- src/mlpack/bindings/python/mlpack/cli.pxd | 5 ++- .../bindings/python/mlpack/cli_util.hpp | 7 +++- .../bindings/python/mlpack/matrix_utils.py | 20 +++++++--- .../python/print_input_processing.hpp | 39 ++++++++++++------- src/mlpack/bindings/python/print_pyx.cpp | 9 ++++- src/mlpack/bindings/python/py_option.hpp | 4 +- src/mlpack/core/util/mlpack_main.hpp | 4 ++ 7 files changed, 61 insertions(+), 27 deletions(-) diff --git a/src/mlpack/bindings/python/mlpack/cli.pxd b/src/mlpack/bindings/python/mlpack/cli.pxd index f6c6435c18..20380817f5 100644 --- a/src/mlpack/bindings/python/mlpack/cli.pxd +++ b/src/mlpack/bindings/python/mlpack/cli.pxd @@ -20,6 +20,9 @@ cdef extern from "" namespace "mlpack" nogil: @staticmethod (T&) GetParam[T](string) nogil except + + @staticmethod + bool HasParam(string) nogil except + + @staticmethod void SetPassed(string) nogil except + @@ -38,7 +41,7 @@ cdef extern from "" namespace "mlpack" nogil: cdef extern from "" \ namespace "mlpack::util" nogil: void SetParam[T](string, T&) nogil except + - void SetParamPtr[T](string, T*) nogil except + + void SetParamPtr[T](string, T*, bool) nogil except + void SetParamWithInfo[T](string, T&, const bool*) nogil except + (T*) GetParamPtr[T](string) nogil except + (T&) GetParamWithInfo[T](string) nogil except + diff --git a/src/mlpack/bindings/python/mlpack/cli_util.hpp b/src/mlpack/bindings/python/mlpack/cli_util.hpp index 6c4ef5ff7b..c2873fcf36 100644 --- a/src/mlpack/bindings/python/mlpack/cli_util.hpp +++ b/src/mlpack/bindings/python/mlpack/cli_util.hpp @@ -42,11 +42,14 @@ inline void SetParam(const std::string& identifier, T& value) * * @param identifier Name of parameter. * @param value Value to set parameter to. + * @param copy Whether or not the object should be copied. */ template -inline void SetParamPtr(const std::string& identifier, T* value) +inline void SetParamPtr(const std::string& identifier, + T* value, + const bool copy) { - CLI::GetParam(identifier) = value; + CLI::GetParam(identifier) = copy ? new T(*value) : value; } /** diff --git a/src/mlpack/bindings/python/mlpack/matrix_utils.py b/src/mlpack/bindings/python/mlpack/matrix_utils.py index 9cf0cbadda..5529315ac6 100644 --- a/src/mlpack/bindings/python/mlpack/matrix_utils.py +++ b/src/mlpack/bindings/python/mlpack/matrix_utils.py @@ -37,7 +37,7 @@ try: except: buffer = memoryview -def to_matrix(x, dtype=np.double): +def to_matrix(x, dtype=np.double, copy=False): """ Given some array-like X, return a numpy ndarray of the same type. """ @@ -48,11 +48,14 @@ def to_matrix(x, dtype=np.double): raise TypeError("given argument is not array-like") if (isinstance(x, np.ndarray) and x.dtype == dtype and x.flags.c_contiguous): - return x, False + if copy: # Copy the matrix if required. + return x.copy("C"), True + else: + return x, False else: return np.array(x, copy=True, dtype=dtype, order='C'), True -def to_matrix_with_info(x, dtype): +def to_matrix_with_info(x, dtype, copy=False): """ Given some array-like X (which should be either a numpy ndarray or a pandas DataFrame, convert into a numpy matrix of the given dtype. @@ -66,7 +69,12 @@ def to_matrix_with_info(x, dtype): if isinstance(x, np.ndarray): # It is already an ndarray, so the vector of info is all 0s (all numeric). d = np.zeros([x.shape[1]], dtype=np.bool) - return (x, False, d) + + # Copy the matrix if needed. + if copy: + return (x.copy(order="C"), True, d) + else: + return (x, False, d) if isinstance(x, pd.DataFrame) or isinstance(x, pd.Series): # It's a pandas dataframe. So we need to see if any of the dtypes are @@ -79,7 +87,7 @@ def to_matrix_with_info(x, dtype): not np.dtype(str) in dtype_array and \ not np.dtype(unicode) in dtype_array: # We can just return the matrix as-is; it's all numeric. - t = to_matrix(x) + t = to_matrix(x, copy) d = np.zeros([x.shape[1]], dtype=np.bool) return (t[0], t[1], d) @@ -130,7 +138,7 @@ def to_matrix_with_info(x, dtype): dims = len(x) d = np.zeros([dims]) - out = np.array(x, dtype=dtype, copy=False) # Try to avoid copy... + out = np.array(x, dtype=dtype, copy=copy) # Try to avoid copy... # Since we don't have a great way to check if these are using the same # memory location, we will probe manually (ugh). diff --git a/src/mlpack/bindings/python/print_input_processing.hpp b/src/mlpack/bindings/python/print_input_processing.hpp index 290366f185..bb688ad463 100644 --- a/src/mlpack/bindings/python/print_input_processing.hpp +++ b/src/mlpack/bindings/python/print_input_processing.hpp @@ -31,6 +31,11 @@ void PrintInputProcessing( const typename boost::disable_if>>::type* = 0) { + // The copy_all_inputs parameter must be handled first, and so is outside the + // scope of this code. + if (d.name == "copy_all_inputs") + return; + const std::string prefix(indent, ' '); std::string def = "None"; @@ -117,8 +122,8 @@ void PrintInputProcessing( std::cout << prefix << "if " << d.name << " is not None:" << std::endl; std::cout << prefix << " " << d.name << "_tuple = to_matrix(" << d.name - << ", dtype=" << GetNumpyType() << ")" - << std::endl; + << ", dtype=" << GetNumpyType() << ", " + << "copy=CLI.HasParam('copy_all_inputs'))" << std::endl; std::cout << prefix << " " << d.name << "_mat = arma_numpy.numpy_to_" << GetArmaType() << "_" << GetNumpyTypeChar() << "(" << d.name << "_tuple[0], " << d.name << "_tuple[1])" << std::endl; @@ -132,8 +137,8 @@ void PrintInputProcessing( else { std::cout << prefix << d.name << "_tuple = to_matrix(" << d.name - << ", dtype=" << GetNumpyType() << ")" - << std::endl; + << ", dtype=" << GetNumpyType() << ", " + << "copy=CLI.HasParam('copy_all_inputs'))" << std::endl; std::cout << prefix << d.name << "_mat = arma_numpy.numpy_to_" << GetArmaType() << "_" << GetNumpyTypeChar() << "(" << d.name << "_tuple[0], " << d.name << "_tuple[1])" << std::endl; @@ -169,10 +174,12 @@ void PrintInputProcessing( * # Detect if the parameter was passed; set if so. * if param_name is not None: * try: - * SetParamPtr[Model]('param_name', ( param_name).modelptr) + * SetParamPtr[Model]('param_name', ( param_name).modelptr, + * CLI.HasParam('copy_all_inputs')) * except TypeError as e: * if type(param_name).__name__ == "ModelType": - * SetParamPtr[Model]('param_name', ( param_name).modelptr) + * SetParamPtr[Model]('param_name', ( param_name).modelptr, + * CLI.HasParam('copy_all_inputs')) TODO * else: * raise e * CLI.SetPassed( 'param_name') @@ -184,14 +191,14 @@ void PrintInputProcessing( std::cout << prefix << "if " << d.name << " is not None:" << std::endl; std::cout << prefix << " try:" << std::endl; std::cout << prefix << " SetParamPtr[" << strippedType << "]('" << d.name - << "', (<" << strippedType << "Type?> " << d.name << ").modelptr)" - << std::endl; + << "', (<" << strippedType << "Type?> " << d.name << ").modelptr, " + << "CLI.HasParam('copy_all_inputs'))" << std::endl; std::cout << prefix << " except TypeError as e:" << std::endl; std::cout << prefix << " if type(" << d.name << ").__name__ == '" << strippedType << "Type':" << std::endl; std::cout << prefix << " SetParamPtr[" << strippedType << "]('" << d.name << "', (<" << strippedType << "Type> " << d.name - << ").modelptr)" << std::endl; + << ").modelptr, CLI.HasParam('copy_all_inputs'))" << std::endl; std::cout << prefix << " else:" << std::endl; std::cout << prefix << " raise e" << std::endl; std::cout << prefix << " CLI.SetPassed( '" << d.name << "')" @@ -201,14 +208,14 @@ void PrintInputProcessing( { std::cout << prefix << "try:" << std::endl; std::cout << prefix << " SetParamPtr[" << strippedType << "]('" << d.name - << "', (<" << strippedType << "Type?> " << d.name << ").modelptr)" - << std::endl; + << "', (<" << strippedType << "Type?> " << d.name << ").modelptr, " + << "CLI.HasParam('copy_all_inputs'))" << std::endl; std::cout << prefix << "except TypeError as e:" << std::endl; std::cout << prefix << " if type(" << d.name << ").__name__ == '" << strippedType << "Type':" << std::endl; std::cout << prefix << " SetParamPtr[" << strippedType << "]('" << d.name - << "', (<" << strippedType << "Type> " << d.name << ").modelptr)" - << std::endl; + << "', (<" << strippedType << "Type> " << d.name << ").modelptr, " + << "CLI.HasParam('copy_all_inputs'))" << std::endl; std::cout << prefix << " else:" << std::endl; std::cout << prefix << " raise e" << std::endl; std::cout << prefix << "CLI.SetPassed( '" << d.name << "')" @@ -246,7 +253,8 @@ void PrintInputProcessing( { std::cout << prefix << "if " << d.name << " is not None:" << std::endl; std::cout << prefix << " " << d.name << "_tuple = to_matrix_with_info(" - << d.name << ", dtype=np.double)" << std::endl; + << d.name << ", dtype=np.double, copy=CLI.HasParam('copy_all_inputs'))" + << std::endl; std::cout << prefix << " " << d.name << "_mat = arma_numpy.numpy_to_mat_d(" << d.name << "_tuple[0], " << d.name << "_tuple[1])" << std::endl; std::cout << prefix << " " << d.name << "_dims = " << d.name << "_tuple[2]" @@ -261,7 +269,8 @@ void PrintInputProcessing( else { std::cout << prefix << d.name << "_tuple = to_matrix_with_info(" << d.name - << ", dtype=np.double)" << std::endl; + << ", dtype=np.double, copy=CLI.HasParam('copy_all_inputs'))" + << std::endl; std::cout << prefix << d.name << "_mat = arma_numpy.numpy_to_mat_d(" << d.name << "_tuple[0], " << d.name << "_tuple[1])" << std::endl; std::cout << prefix << d.name << "_dims = " << d.name << "_tuple[2]" diff --git a/src/mlpack/bindings/python/print_pyx.cpp b/src/mlpack/bindings/python/print_pyx.cpp index 423de61a41..498bd07940 100644 --- a/src/mlpack/bindings/python/print_pyx.cpp +++ b/src/mlpack/bindings/python/print_pyx.cpp @@ -177,7 +177,14 @@ void PrintPYX(const ProgramDoc& programInfo, cout << " DisableVerbose()" << endl; // Restore the parameters. - cout << " CLI.RestoreSettings(\"" << programInfo.programName << "\")"; + cout << " CLI.RestoreSettings(\"" << programInfo.programName << "\")" + << endl; + + // Determine whether or not we need to copy parameters. + cout << " if copy_all_inputs:" << endl; + cout << " SetParam[bool]( 'copy_all_inputs', " + << "copy_all_inputs)" << endl; + cout << " CLI.SetPassed( 'copy_all_inputs')" << endl; // Do any input processing. for (size_t i = 0; i < inputOptions.size(); ++i) diff --git a/src/mlpack/bindings/python/py_option.hpp b/src/mlpack/bindings/python/py_option.hpp index 60eaf3a1c4..b25be68a4b 100644 --- a/src/mlpack/bindings/python/py_option.hpp +++ b/src/mlpack/bindings/python/py_option.hpp @@ -58,8 +58,8 @@ class PyOption data.required = required; data.input = input; data.loaded = false; - // Only "verbose" will be persistent. - if (identifier == "verbose") + // Only "verbose" and "copy_all_inputs" will be persistent. + if (identifier == "verbose" || identifier == "copy_all_inputs") data.persistent = true; else data.persistent = false; diff --git a/src/mlpack/core/util/mlpack_main.hpp b/src/mlpack/core/util/mlpack_main.hpp index a26a5c6ff7..e675d3d1b0 100644 --- a/src/mlpack/core/util/mlpack_main.hpp +++ b/src/mlpack/core/util/mlpack_main.hpp @@ -138,6 +138,10 @@ static const std::string testName = ""; PARAM_FLAG("verbose", "Display informational messages and the full list of " "parameters and timers at the end of execution.", "v"); +PARAM_FLAG("copy_all_inputs", "If specified, all input parameters will be deep" + " copied before the method is run. This is useful for debugging problems " + "where the input parameters are being modified by the algorithm, but can " + "slow down the code.", ""); // Nothing else needs to be defined---the binding will use mlpackMain() as-is. From 6f8828859241a0b363b3ddceedb086ca18ae3853 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sat, 27 Jan 2018 23:16:33 -0500 Subject: [PATCH 17/25] Add tests for Python bindings and update for new to_matrix() API. --- .../bindings/python/mlpack/matrix_utils.py | 6 +- .../python/tests/dataset_info_test.py | 26 +- .../python/tests/test_python_binding.py | 255 +++++++++++++++++- .../python/tests/test_python_binding_main.cpp | 4 +- 4 files changed, 269 insertions(+), 22 deletions(-) diff --git a/src/mlpack/bindings/python/mlpack/matrix_utils.py b/src/mlpack/bindings/python/mlpack/matrix_utils.py index 5529315ac6..58f39b61ed 100644 --- a/src/mlpack/bindings/python/mlpack/matrix_utils.py +++ b/src/mlpack/bindings/python/mlpack/matrix_utils.py @@ -87,7 +87,7 @@ def to_matrix_with_info(x, dtype, copy=False): not np.dtype(str) in dtype_array and \ not np.dtype(unicode) in dtype_array: # We can just return the matrix as-is; it's all numeric. - t = to_matrix(x, copy) + t = to_matrix(x, dtype=dtype, copy=copy) d = np.zeros([x.shape[1]], dtype=np.bool) return (t[0], t[1], d) @@ -126,7 +126,7 @@ def to_matrix_with_info(x, dtype, copy=False): # We'll have to force the second part of the tuple (whether or not to take # ownership) to true. - t = to_matrix(y.apply(pd.to_numeric)) + t = to_matrix(y.apply(pd.to_numeric), dtype=dtype) return (t[0], True, d) if isinstance(x, list): @@ -149,7 +149,7 @@ def to_matrix_with_info(x, dtype, copy=False): alias = True x[0] = oldval - return (np.array(x, dtype=dtype), not alias, d) + return (out, not alias, d) # If we got here, the type is not known. raise TypeError("given matrix is not a numpy ndarray or pandas DataFrame or "\ diff --git a/src/mlpack/bindings/python/tests/dataset_info_test.py b/src/mlpack/bindings/python/tests/dataset_info_test.py index dd14d73071..e72c529d00 100644 --- a/src/mlpack/bindings/python/tests/dataset_info_test.py +++ b/src/mlpack/bindings/python/tests/dataset_info_test.py @@ -23,7 +23,7 @@ class TestToMatrix(unittest.TestCase): """ d = pd.DataFrame(np.random.randn(100, 4), columns=list('abcd')) - m = to_matrix(d) + m, _ = to_matrix(d) self.assertTrue(isinstance(m, np.ndarray)) self.assertEqual(m.shape[0], 100) @@ -40,7 +40,7 @@ class TestToMatrix(unittest.TestCase): """ d = pd.DataFrame({'a': range(5)}) - m = to_matrix(d) + m, _ = to_matrix(d) self.assertTrue(isinstance(m, np.ndarray)) self.assertEqual(m.shape[0], 5) @@ -58,7 +58,7 @@ class TestToMatrix(unittest.TestCase): self.assertEqual(d['a'].dtype, int) self.assertEqual(d['b'].dtype, np.dtype(np.double)) - m = to_matrix(d) + m, _ = to_matrix(d) self.assertTrue(isinstance(m, np.ndarray)) self.assertEqual(m.dtype, np.dtype(np.double)) @@ -79,7 +79,7 @@ class TestToMatrix(unittest.TestCase): [0.07, 0.08, 0.09], [0.10, 0.11, 0.12]] - m = to_matrix(a) + m, _ = to_matrix(a) self.assertTrue(isinstance(m, np.ndarray)) self.assertEqual(m.dtype, np.dtype(np.double)) @@ -100,7 +100,7 @@ class TestToMatrix(unittest.TestCase): [0.07, 0.08, 9], [0.10, 0.11, 12]] - m = to_matrix(a) + m, _ = to_matrix(a) self.assertTrue(isinstance(m, np.ndarray)) self.assertEqual(m.dtype, np.dtype(np.double)) @@ -116,7 +116,7 @@ class TestToMatrix(unittest.TestCase): Make sure we can convert a numpy matrix without copying anything. """ m1 = np.random.randn(100, 5) - m2 = to_matrix(m1) + m2, _ = to_matrix(m1) self.assertTrue(isinstance(m2, np.ndarray)) self.assertEqual(m2.dtype, np.dtype(np.double)) @@ -144,7 +144,7 @@ class TestToMatrixWithInfo(unittest.TestCase): """ d = pd.DataFrame(np.random.randn(100, 4), columns=list('abcd')) - m, dims = to_matrix_with_info(d, np.double) + m, _, dims = to_matrix_with_info(d, np.double) self.assertTrue(isinstance(m, np.ndarray)) self.assertEqual(m.shape[0], 100) @@ -167,7 +167,7 @@ class TestToMatrixWithInfo(unittest.TestCase): """ d = pd.DataFrame({'a': range(5)}) - m, dims = to_matrix_with_info(d, np.double) + m, _, dims = to_matrix_with_info(d, np.double) self.assertTrue(isinstance(m, np.ndarray)) self.assertEqual(m.shape[0], 5) @@ -188,7 +188,7 @@ class TestToMatrixWithInfo(unittest.TestCase): self.assertEqual(d['a'].dtype, int) self.assertEqual(d['b'].dtype, np.dtype(np.double)) - m, dims = to_matrix_with_info(d, np.double) + m, _, dims = to_matrix_with_info(d, np.double) self.assertTrue(isinstance(m, np.ndarray)) self.assertEqual(m.dtype, np.dtype(np.double)) @@ -213,7 +213,7 @@ class TestToMatrixWithInfo(unittest.TestCase): [0.07, 0.08, 0.09], [0.10, 0.11, 0.12]] - m, dims = to_matrix_with_info(a, np.double) + m, _, dims = to_matrix_with_info(a, np.double) self.assertTrue(isinstance(m, np.ndarray)) self.assertEqual(m.dtype, np.dtype(np.double)) @@ -239,7 +239,7 @@ class TestToMatrixWithInfo(unittest.TestCase): [0.07, 0.08, 9], [0.10, 0.11, 12]] - m, dims = to_matrix_with_info(a, np.double) + m, _, dims = to_matrix_with_info(a, np.double) self.assertTrue(isinstance(m, np.ndarray)) self.assertEqual(m.dtype, np.dtype(np.double)) @@ -260,7 +260,7 @@ class TestToMatrixWithInfo(unittest.TestCase): Make sure we can convert a numpy matrix without copying anything. """ m1 = np.random.randn(100, 5) - m2, dims = to_matrix_with_info(m1, np.double) + m2, _, dims = to_matrix_with_info(m1, np.double) self.assertTrue(isinstance(m2, np.ndarray)) self.assertEqual(m2.dtype, np.dtype(np.double)) @@ -284,7 +284,7 @@ class TestToMatrixWithInfo(unittest.TestCase): d = pd.DataFrame({"A": ["a", "b", "c", "a"] }) d["A"] = d["A"].astype('category') # Convert to categorical. - m, dims = to_matrix_with_info(d, np.double) + m, _, dims = to_matrix_with_info(d, np.double) self.assertTrue(isinstance(m, np.ndarray)) self.assertEqual(m.dtype, np.dtype(np.double)) diff --git a/src/mlpack/bindings/python/tests/test_python_binding.py b/src/mlpack/bindings/python/tests/test_python_binding.py index 93ac2615c6..bd10b83a2e 100644 --- a/src/mlpack/bindings/python/tests/test_python_binding.py +++ b/src/mlpack/bindings/python/tests/test_python_binding.py @@ -112,6 +112,29 @@ class TestPythonBinding(unittest.TestCase): for j in range(100): self.assertEqual(2 * x[j, 2], output['matrix_out'][j, 2]) + def testNumpyMatrixForceCopy(self): + """ + The matrix we pass in, we should get back with the third dimension doubled + and the fifth forgotten. + """ + x = np.random.rand(100, 5); + + output = test_python_binding(string_in='hello', + int_in=12, + double_in=4.0, + matrix_in=x, + copy_all_inputs=True) + + self.assertEqual(output['matrix_out'].shape[0], 100) + self.assertEqual(output['matrix_out'].shape[1], 4) + self.assertEqual(output['matrix_out'].dtype, np.double) + for i in [0, 1, 3]: + for j in range(100): + self.assertEqual(x[j, i], output['matrix_out'][j, i]) + + for j in range(100): + self.assertEqual(2 * x[j, 2], output['matrix_out'][j, 2]) + def testArraylikeMatrix(self): """ Test that we can pass an arraylike matrix. @@ -119,12 +142,11 @@ class TestPythonBinding(unittest.TestCase): x = [[1, 2, 3, 4, 5], [6, 7, 8, 9, 10], [11, 12, 13, 14, 15]] - z = copy.copy(x) output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - matrix_in=z) + matrix_in=x) self.assertEqual(output['matrix_out'].shape[0], 3) self.assertEqual(output['matrix_out'].shape[1], 4) @@ -142,6 +164,38 @@ class TestPythonBinding(unittest.TestCase): self.assertEqual(output['matrix_out'][2, 2], 26) self.assertEqual(output['matrix_out'][2, 3], 14) + def testArraylikeMatrixForceCopy(self): + """ + Test that we can pass an arraylike matrix. + """ + x = [[1, 2, 3, 4, 5], + [6, 7, 8, 9, 10], + [11, 12, 13, 14, 15]] + + output = test_python_binding(string_in='hello', + int_in=12, + double_in=4.0, + matrix_in=x, + copy_all_inputs=True) + + self.assertEqual(output['matrix_out'].shape[0], 3) + self.assertEqual(output['matrix_out'].shape[1], 4) + self.assertEqual(len(x), 3) + self.assertEqual(len(x[0]), 5) + self.assertEqual(output['matrix_out'].dtype, np.double) + self.assertEqual(output['matrix_out'][0, 0], 1) + self.assertEqual(output['matrix_out'][0, 1], 2) + self.assertEqual(output['matrix_out'][0, 2], 6) + self.assertEqual(output['matrix_out'][0, 3], 4) + self.assertEqual(output['matrix_out'][1, 0], 6) + self.assertEqual(output['matrix_out'][1, 1], 7) + self.assertEqual(output['matrix_out'][1, 2], 16) + self.assertEqual(output['matrix_out'][1, 3], 9) + self.assertEqual(output['matrix_out'][2, 0], 11) + self.assertEqual(output['matrix_out'][2, 1], 12) + self.assertEqual(output['matrix_out'][2, 2], 26) + self.assertEqual(output['matrix_out'][2, 3], 14) + def testNumpyUmatrix(self): """ Same as testNumpyMatrix() but with an unsigned matrix. @@ -164,6 +218,28 @@ class TestPythonBinding(unittest.TestCase): for j in range(100): self.assertEqual(2 * x[j, 2], output['umatrix_out'][j, 2]) + def testNumpyUmatrixForceCopy(self): + """ + Same as testNumpyMatrix() but with an unsigned matrix. + """ + x = np.random.randint(0, high=500, size=[100, 5]) + + output = test_python_binding(string_in='hello', + int_in=12, + double_in=4.0, + umatrix_in=x, + copy_all_inputs=True) + + self.assertEqual(output['umatrix_out'].shape[0], 100) + self.assertEqual(output['umatrix_out'].shape[1], 4) + self.assertEqual(output['umatrix_out'].dtype, np.long) + for i in [0, 1, 3]: + for j in range(100): + self.assertEqual(x[j, i], output['umatrix_out'][j, i]) + + for j in range(100): + self.assertEqual(2 * x[j, 2], output['umatrix_out'][j, 2]) + def testArraylikeUmatrix(self): """ Test that we can pass an arraylike unsigned matrix. @@ -171,12 +247,11 @@ class TestPythonBinding(unittest.TestCase): x = [[1, 2, 3, 4, 5], [6, 7, 8, 9, 10], [11, 12, 13, 14, 15]] - z = copy.copy(x) output = test_python_binding(string_in='hello', int_in=12, double_in=4.0, - umatrix_in=z) + umatrix_in=x) self.assertEqual(output['umatrix_out'].shape[0], 3) self.assertEqual(output['umatrix_out'].shape[1], 4) @@ -194,6 +269,38 @@ class TestPythonBinding(unittest.TestCase): self.assertEqual(output['umatrix_out'][2, 2], 26) self.assertEqual(output['umatrix_out'][2, 3], 14) + def testArraylikeUmatrixForceCopy(self): + """ + Test that we can pass an arraylike unsigned matrix. + """ + x = [[1, 2, 3, 4, 5], + [6, 7, 8, 9, 10], + [11, 12, 13, 14, 15]] + + output = test_python_binding(string_in='hello', + int_in=12, + double_in=4.0, + umatrix_in=x, + copy_all_inputs=True) + + self.assertEqual(output['umatrix_out'].shape[0], 3) + self.assertEqual(output['umatrix_out'].shape[1], 4) + self.assertEqual(len(x), 3) + self.assertEqual(len(x[0]), 5) + self.assertEqual(output['umatrix_out'].dtype, np.long) + self.assertEqual(output['umatrix_out'][0, 0], 1) + self.assertEqual(output['umatrix_out'][0, 1], 2) + self.assertEqual(output['umatrix_out'][0, 2], 6) + self.assertEqual(output['umatrix_out'][0, 3], 4) + self.assertEqual(output['umatrix_out'][1, 0], 6) + self.assertEqual(output['umatrix_out'][1, 1], 7) + self.assertEqual(output['umatrix_out'][1, 2], 16) + self.assertEqual(output['umatrix_out'][1, 3], 9) + self.assertEqual(output['umatrix_out'][2, 0], 11) + self.assertEqual(output['umatrix_out'][2, 1], 12) + self.assertEqual(output['umatrix_out'][2, 2], 26) + self.assertEqual(output['umatrix_out'][2, 3], 14) + def testCol(self): """ Test a column vector input parameter. @@ -212,6 +319,24 @@ class TestPythonBinding(unittest.TestCase): for i in range(100): self.assertEqual(output['col_out'][i], x[i] * 2) + def testColForceCopy(self): + """ + Test a column vector input parameter. + """ + x = np.random.rand(100) + + output = test_python_binding(string_in='hello', + int_in=12, + double_in=4.0, + col_in=x, + copy_all_inputs=True) + + self.assertEqual(output['col_out'].shape[0], 100) + self.assertEqual(output['col_out'].dtype, np.double) + + for i in range(100): + self.assertEqual(output['col_out'][i], x[i] * 2) + def testUcol(self): """ Test an unsigned column vector input parameter. @@ -229,6 +354,23 @@ class TestPythonBinding(unittest.TestCase): for i in range(100): self.assertEqual(output['ucol_out'][i], x[i] * 2) + def testUcolForceCopy(self): + """ + Test an unsigned column vector input parameter. + """ + x = np.random.randint(0, high=500, size=100) + + output = test_python_binding(string_in='hello', + int_in=12, + double_in=4.0, + ucol_in=x, + copy_all_inputs=True) + + self.assertEqual(output['ucol_out'].shape[0], 100) + self.assertEqual(output['ucol_out'].dtype, np.long) + for i in range(100): + self.assertEqual(output['ucol_out'][i], x[i] * 2) + def testRow(self): """ Test a row vector input parameter. @@ -247,6 +389,24 @@ class TestPythonBinding(unittest.TestCase): for i in range(100): self.assertEqual(output['row_out'][i], x[i] * 2) + def testRowForceCopy(self): + """ + Test a row vector input parameter. + """ + x = np.random.rand(100) + + output = test_python_binding(string_in='hello', + int_in=12, + double_in=4.0, + row_in=x, + copy_all_inputs=True) + + self.assertEqual(output['row_out'].shape[0], 100) + self.assertEqual(output['row_out'].dtype, np.double) + + for i in range(100): + self.assertEqual(output['row_out'][i], x[i] * 2) + def testUrow(self): """ Test an unsigned row vector input parameter. @@ -265,6 +425,24 @@ class TestPythonBinding(unittest.TestCase): for i in range(100): self.assertEqual(output['urow_out'][i], x[i] * 2) + def testUrowForceCopy(self): + """ + Test an unsigned row vector input parameter. + """ + x = np.random.randint(0, high=500, size=100) + + output = test_python_binding(string_in='hello', + int_in=12, + double_in=4.0, + urow_in=x, + copy_all_inputs=True) + + self.assertEqual(output['urow_out'].shape[0], 100) + self.assertEqual(output['urow_out'].dtype, np.long) + + for i in range(100): + self.assertEqual(output['urow_out'][i], x[i] * 2) + def testMatrixAndInfoNumpy(self): """ Test that we can pass a matrix with all numeric features. @@ -284,6 +462,25 @@ class TestPythonBinding(unittest.TestCase): for j in range(100): self.assertEqual(output['matrix_and_info_out'][j, i], x[j, i] * 2.0) + def testMatrixAndInfoNumpyForceCopy(self): + """ + Test that we can pass a matrix with all numeric features. + """ + x = np.random.rand(100, 10) + + output = test_python_binding(string_in='hello', + int_in=12, + double_in=4.0, + matrix_and_info_in=x, + copy_all_inputs=True) + + self.assertEqual(output['matrix_and_info_out'].shape[0], 100) + self.assertEqual(output['matrix_and_info_out'].shape[1], 10) + + for i in range(10): + for j in range(100): + self.assertEqual(output['matrix_and_info_out'][j, i], x[j, i] * 2.0) + def testMatrixAndInfoPandas(self): """ Test that we can pass a matrix with some categorical features. @@ -310,6 +507,32 @@ class TestPythonBinding(unittest.TestCase): for j in range(10): self.assertEqual(output['matrix_and_info_out'][j, 4], z[cols[4]][j]) + def testMatrixAndInfoPandasForceCopy(self): + """ + Test that we can pass a matrix with some categorical features. + """ + x = pd.DataFrame(np.random.rand(10, 4), columns=list('abcd')) + x['e'] = pd.Series(['a', 'b', 'c', 'd', 'a', 'b', 'e', 'c', 'a', 'b'], + dtype='category') + + output = test_python_binding(string_in='hello', + int_in=12, + double_in=4.0, + matrix_and_info_in=x, + copy_all_inputs=True) + + self.assertEqual(output['matrix_and_info_out'].shape[0], 10) + self.assertEqual(output['matrix_and_info_out'].shape[1], 5) + + cols = list('abcde') + + for i in range(4): + for j in range(10): + self.assertEqual(output['matrix_and_info_out'][j, i], x[cols[i]][j] * 2) + + for j in range(10): + self.assertEqual(output['matrix_and_info_out'][j, 4], x[cols[4]][j]) + def testIntVector(self): """ Test that we can pass a vector of ints and get back that same vector but @@ -356,5 +579,29 @@ class TestPythonBinding(unittest.TestCase): self.assertEqual(output2['model_bw_out'], 20.0) + def testModelForceCopy(self): + """ + First create a GaussianKernel object, then send it back and make sure we get + the right double value. + """ + output = test_python_binding(string_in='hello', + int_in=12, + double_in=4.0, + build_model=True) + + output2 = test_python_binding(string_in='hello', + int_in=12, + double_in=4.0, + model_in=output['model_out'], + copy_all_inputs=True) + + output3 = test_python_binding(string_in='hello', + int_in=12, + double_in=4.0, + model_in=output['model_out']) + + self.assertEqual(output2['model_bw_out'], 20.0) + self.assertEqual(output3['model_bw_out'], 20.0) + if __name__ == '__main__': unittest.main() diff --git a/src/mlpack/bindings/python/tests/test_python_binding_main.cpp b/src/mlpack/bindings/python/tests/test_python_binding_main.cpp index 8be2f4947a..9af5a26693 100644 --- a/src/mlpack/bindings/python/tests/test_python_binding_main.cpp +++ b/src/mlpack/bindings/python/tests/test_python_binding_main.cpp @@ -172,13 +172,13 @@ static void mlpackMain() // If we got a request to build a model, then build it. if (CLI::HasParam("build_model")) { - CLI::GetParam("model_out") = GaussianKernel(10.0); + CLI::GetParam("model_out") = new GaussianKernel(10.0); } // If we got an input model, double the bandwidth and output that. if (CLI::HasParam("model_in")) { CLI::GetParam("model_bw_out") = - CLI::GetParam("model_in").Bandwidth() * 2.0; + CLI::GetParam("model_in")->Bandwidth() * 2.0; } } From 3b756a0cf88241e4916d98625eea30ebd5109f42 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Sun, 28 Jan 2018 14:36:16 -0500 Subject: [PATCH 18/25] Update tests to use pointers. (This is probably not completely correct but I am moving systems right now.) --- .../tests/main_tests/decision_stump_test.cpp | 8 +++++-- .../tests/main_tests/decision_tree_test.cpp | 2 +- src/mlpack/tests/main_tests/nbc_test.cpp | 8 +++++-- .../tests/main_tests/random_forest_test.cpp | 23 +++++++++++++------ .../main_tests/softmax_regression_test.cpp | 4 +++- 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/mlpack/tests/main_tests/decision_stump_test.cpp b/src/mlpack/tests/main_tests/decision_stump_test.cpp index f2e07a9198..02a6212471 100644 --- a/src/mlpack/tests/main_tests/decision_stump_test.cpp +++ b/src/mlpack/tests/main_tests/decision_stump_test.cpp @@ -198,7 +198,7 @@ BOOST_AUTO_TEST_CASE(DecisionStumpModelReuseTest) // Input trained model. SetInputParam("test", std::move(testData)); SetInputParam("input_model", - std::move(CLI::GetParam("output_model"))); + std::move(CLI::GetParam("output_model"))); mlpackMain(); @@ -213,6 +213,8 @@ BOOST_AUTO_TEST_CASE(DecisionStumpModelReuseTest) // Check that initial predictions and final predicitons matrix // using saved model are same. CheckMatrices(predictions, CLI::GetParam>("predictions")); + + delete CLI::GetParam("output_model"); } /** @@ -249,11 +251,13 @@ BOOST_AUTO_TEST_CASE(DecisionStumpTrainingVerTest) // Input pre-trained model. SetInputParam("input_model", - std::move(CLI::GetParam("output_model"))); + std::move(CLI::GetParam("output_model"))); Log::Fatal.ignoreInput = true; BOOST_REQUIRE_THROW(mlpackMain(), std::runtime_error); Log::Fatal.ignoreInput = false; + + delete CLI::GetParam("output_model"); } BOOST_AUTO_TEST_SUITE_END(); diff --git a/src/mlpack/tests/main_tests/decision_tree_test.cpp b/src/mlpack/tests/main_tests/decision_tree_test.cpp index 09312c0452..3511239f40 100644 --- a/src/mlpack/tests/main_tests/decision_tree_test.cpp +++ b/src/mlpack/tests/main_tests/decision_tree_test.cpp @@ -254,7 +254,7 @@ BOOST_AUTO_TEST_CASE(DecisionTreeTrainingVerTest) // Input pre-trained model. SetInputParam("input_model", - std::move(CLI::GetParam("output_model"))); + std::move(CLI::GetParam("output_model"))); Log::Fatal.ignoreInput = true; BOOST_REQUIRE_THROW(mlpackMain(), std::runtime_error); diff --git a/src/mlpack/tests/main_tests/nbc_test.cpp b/src/mlpack/tests/main_tests/nbc_test.cpp index ebb58cab27..1e12cf9aa5 100644 --- a/src/mlpack/tests/main_tests/nbc_test.cpp +++ b/src/mlpack/tests/main_tests/nbc_test.cpp @@ -208,7 +208,7 @@ BOOST_AUTO_TEST_CASE(NBCModelReuseTest) // Input trained model. SetInputParam("test", std::move(testData)); SetInputParam("input_model", - std::move(CLI::GetParam("output_model"))); + std::move(CLI::GetParam("output_model"))); mlpackMain(); @@ -226,6 +226,8 @@ BOOST_AUTO_TEST_CASE(NBCModelReuseTest) // matrix using saved model are same. CheckMatrices(output, CLI::GetParam>("output")); CheckMatrices(output_probs, CLI::GetParam("output_probs")); + + delete CLI::GetParam("output_model"); } /** @@ -244,11 +246,13 @@ BOOST_AUTO_TEST_CASE(NBCTrainingVerTest) // Input pre-trained model. SetInputParam("input_model", - std::move(CLI::GetParam("output_model"))); + std::move(CLI::GetParam("output_model"))); Log::Fatal.ignoreInput = true; BOOST_REQUIRE_THROW(mlpackMain(), std::runtime_error); Log::Fatal.ignoreInput = false; + + delete CLI::GetParam("output_model"); } /** diff --git a/src/mlpack/tests/main_tests/random_forest_test.cpp b/src/mlpack/tests/main_tests/random_forest_test.cpp index f4460ed752..340d34f1da 100644 --- a/src/mlpack/tests/main_tests/random_forest_test.cpp +++ b/src/mlpack/tests/main_tests/random_forest_test.cpp @@ -124,7 +124,7 @@ BOOST_AUTO_TEST_CASE(RandomForestModelReuseTest) // Input trained model. SetInputParam("test", std::move(testData)); SetInputParam("input_model", - std::move(CLI::GetParam("output_model"))); + CLI::GetParam("output_model")); mlpackMain(); @@ -143,6 +143,8 @@ BOOST_AUTO_TEST_CASE(RandomForestModelReuseTest) // Check that initial predictions and predictions using saved model are same. CheckMatrices(predictions, CLI::GetParam>("predictions")); CheckMatrices(probabilities, CLI::GetParam("probabilities")); + + delete CLI::GetParam("output_model"); } /** @@ -206,11 +208,13 @@ BOOST_AUTO_TEST_CASE(RandomForestTrainingVerTest) // Input pre-trained model. SetInputParam("input_model", - std::move(CLI::GetParam("output_model"))); + CLI::GetParam("output_model")); Log::Fatal.ignoreInput = true; BOOST_REQUIRE_THROW(mlpackMain(), std::runtime_error); Log::Fatal.ignoreInput = false; + + delete CLI::GetParam("output_model"); } /** @@ -236,7 +240,7 @@ BOOST_AUTO_TEST_CASE(RandomForestDiffMinLeafSizeTest) // Calculate training accuracy. arma::Row predictions; - CLI::GetParam("output_model").rf.Classify(inputData, + CLI::GetParam("output_model")->rf.Classify(inputData, predictions); size_t correct = arma::accu(predictions == labels); @@ -252,7 +256,7 @@ BOOST_AUTO_TEST_CASE(RandomForestDiffMinLeafSizeTest) mlpackMain(); // Calculate training accuracy. - CLI::GetParam("output_model").rf.Classify(inputData, + CLI::GetParam("output_model")->rf.Classify(inputData, predictions); correct = arma::accu(predictions == labels); @@ -275,6 +279,8 @@ BOOST_AUTO_TEST_CASE(RandomForestDiffMinLeafSizeTest) double accuracy1 = (double(correct) / double(labels.n_elem) * 100); BOOST_REQUIRE(accuracy1 > accuracy10 && accuracy10 > accuracy20); + + delete CLI::GetParam("output_model"); } /** @@ -308,8 +314,9 @@ BOOST_AUTO_TEST_CASE(RandomForestDiffNumTreeTest) // Calculate training accuracy. arma::Row predictions; - CLI::GetParam("output_model").rf.Classify(testData, + CLI::GetParam("output_model")->rf.Classify(testData, predictions); + delete CLI::GetParam("output_model"); size_t correct = arma::accu(predictions == testLabels); double accuracy1 = (double(correct) / double(testLabels.n_elem) * 100); @@ -324,8 +331,9 @@ BOOST_AUTO_TEST_CASE(RandomForestDiffNumTreeTest) mlpackMain(); // Calculate training accuracy. - CLI::GetParam("output_model").rf.Classify(testData, + CLI::GetParam("output_model")->rf.Classify(testData, predictions); + delete CLI::GetParam("output_model"); correct = arma::accu(predictions == testLabels); double accuracy5 = (double(correct) / double(testLabels.n_elem) * 100); @@ -340,8 +348,9 @@ BOOST_AUTO_TEST_CASE(RandomForestDiffNumTreeTest) mlpackMain(); // Calculate training accuracy. - CLI::GetParam("output_model").rf.Classify(testData, + CLI::GetParam("output_model")->rf.Classify(testData, predictions); + delete CLI::GetParam("output_model"); correct = arma::accu(predictions == testLabels); double accuracy10 = (double(correct) / double(testLabels.n_elem) * 100); diff --git a/src/mlpack/tests/main_tests/softmax_regression_test.cpp b/src/mlpack/tests/main_tests/softmax_regression_test.cpp index 04dc10169a..358ad1b8bd 100644 --- a/src/mlpack/tests/main_tests/softmax_regression_test.cpp +++ b/src/mlpack/tests/main_tests/softmax_regression_test.cpp @@ -150,7 +150,7 @@ BOOST_AUTO_TEST_CASE(SoftmaxRegressionModelReuseTest) // Input trained model. SetInputParam("test", std::move(testData)); SetInputParam("input_model", - std::move(CLI::GetParam("output_model"))); + CLI::GetParam("output_model")); mlpackMain(); @@ -165,6 +165,8 @@ BOOST_AUTO_TEST_CASE(SoftmaxRegressionModelReuseTest) // Check that initial predictions and final predicitons matrix // using saved model are same. CheckMatrices(predictions, CLI::GetParam>("predictions")); + + delete CLI::GetParam("output_model"); } /** From c7dc28b8c213c2422077ff60d69f9621ef07ebdc Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Mon, 29 Jan 2018 12:59:37 -0500 Subject: [PATCH 19/25] Add functions to clean up after tests. --- src/mlpack/bindings/tests/CMakeLists.txt | 4 ++ src/mlpack/bindings/tests/clean_memory.cpp | 54 ++++++++++++++++++ src/mlpack/bindings/tests/clean_memory.hpp | 24 ++++++++ .../tests/delete_allocated_memory.hpp | 56 ++++++++++++++++++ .../bindings/tests/get_allocated_memory.hpp | 57 +++++++++++++++++++ src/mlpack/bindings/tests/test_option.hpp | 6 ++ 6 files changed, 201 insertions(+) create mode 100644 src/mlpack/bindings/tests/clean_memory.cpp create mode 100644 src/mlpack/bindings/tests/clean_memory.hpp create mode 100644 src/mlpack/bindings/tests/delete_allocated_memory.hpp create mode 100644 src/mlpack/bindings/tests/get_allocated_memory.hpp diff --git a/src/mlpack/bindings/tests/CMakeLists.txt b/src/mlpack/bindings/tests/CMakeLists.txt index 5955a0a514..70b9928f91 100644 --- a/src/mlpack/bindings/tests/CMakeLists.txt +++ b/src/mlpack/bindings/tests/CMakeLists.txt @@ -1,8 +1,12 @@ # Define the files we need to compile. # Anything not in this list will not be compiled into mlpack. set(SOURCES + clean_memory.hpp + clean_memory.cpp test_option.hpp ignore_check.hpp + delete_allocated_memory.hpp + get_allocated_memory.hpp get_param.hpp get_printable_param.hpp get_printable_param_impl.hpp diff --git a/src/mlpack/bindings/tests/clean_memory.cpp b/src/mlpack/bindings/tests/clean_memory.cpp new file mode 100644 index 0000000000..dd40020177 --- /dev/null +++ b/src/mlpack/bindings/tests/clean_memory.cpp @@ -0,0 +1,54 @@ +/** + * @file clean_memory.cpp + * @author Ryan Curtin + * + * Delete any pointers held by the CLI object. + */ +#include "clean_memory.hpp" + +#include + +namespace mlpack { +namespace bindings { +namespace tests { + +/** + * Delete any pointers held by the CLI object. + */ +void CleanMemory() +{ + // If we are holding any pointers, then we "own" them. But we may hold the + // same pointer twice, so we have to be careful to not delete it multiple + // times. + std::unordered_map memoryAddresses; + auto it = CLI::Parameters().begin(); + while (it != CLI::Parameters().end()) + { + const util::ParamData& data = it->second; + + void* result; + CLI::GetSingleton().functionMap[data.tname]["GetAllocatedMemory"](data, + NULL, (void*) &result); + if (result != NULL && memoryAddresses.count(result) == 0) + memoryAddresses[result] = &data; + + ++it; + } + + // Now we have all the unique addresses that need to be deleted. + std::unordered_map::const_iterator it2; + it2 = memoryAddresses.begin(); + while (it2 != memoryAddresses.end()) + { + const util::ParamData& data = *(it2->second); + + CLI::GetSingleton().functionMap[data.tname]["DeleteAllocatedMemory"](data, + NULL, NULL); + + ++it2; + } +} + +} // namespace tests +} // namespace bindings +} // namespace mlpack diff --git a/src/mlpack/bindings/tests/clean_memory.hpp b/src/mlpack/bindings/tests/clean_memory.hpp new file mode 100644 index 0000000000..1035b7f5cb --- /dev/null +++ b/src/mlpack/bindings/tests/clean_memory.hpp @@ -0,0 +1,24 @@ +/** + * @file clean_memory.hpp + * @author Ryan Curtin + * + * Delete any unique pointers that are held by the CLI object. This is similar + * to the code in end_program.hpp. + */ +#ifndef MLPACK_BINDINGS_TESTS_CLEAN_MEMORY_HPP +#define MLPACK_BINDINGS_TESTS_CLEAN_MEMORY_HPP + +namespace mlpack { +namespace bindings { +namespace tests { + +/** + * Delete any unique pointers that are held by the CLI object. + */ +void CleanMemory(); + +} // namespace tests +} // namespace bindings +} // namespace mlpack + +#endif diff --git a/src/mlpack/bindings/tests/delete_allocated_memory.hpp b/src/mlpack/bindings/tests/delete_allocated_memory.hpp new file mode 100644 index 0000000000..f39deb113b --- /dev/null +++ b/src/mlpack/bindings/tests/delete_allocated_memory.hpp @@ -0,0 +1,56 @@ +/** + * @file delete_allocated_memory.hpp + * @author Ryan Curtin + * + * If any memory has been allocated by the parameter, delete it. + */ +#ifndef MLPACK_BINDINGS_CLI_DELETE_ALLOCATED_MEMORY_HPP +#define MLPACK_BINDINGS_CLI_DELETE_ALLOCATED_MEMORY_HPP + +#include + +namespace mlpack { +namespace bindings { +namespace tests { + +template +void DeleteAllocatedMemoryImpl( + const util::ParamData& /* d */, + const typename boost::disable_if>::type* = 0, + const typename boost::disable_if>::type* = 0) +{ + // Do nothing. +} + +template +void DeleteAllocatedMemoryImpl( + const util::ParamData& /* d */, + const typename boost::enable_if>::type* = 0) +{ + // Do nothing. +} + +template +void DeleteAllocatedMemoryImpl( + const util::ParamData& d, + const typename boost::disable_if>::type* = 0, + const typename boost::enable_if>::type* = 0) +{ + // Delete the allocated memory (hopefully we actually own it). + delete *boost::any_cast(&d.value); +} + +template +void DeleteAllocatedMemory( + const util::ParamData& d, + const void* /* input */, + void* /* output */) +{ + DeleteAllocatedMemoryImpl::type>(d); +} + +} // namespace cli +} // namespace bindings +} // namespace mlpack + +#endif diff --git a/src/mlpack/bindings/tests/get_allocated_memory.hpp b/src/mlpack/bindings/tests/get_allocated_memory.hpp new file mode 100644 index 0000000000..ec53740a40 --- /dev/null +++ b/src/mlpack/bindings/tests/get_allocated_memory.hpp @@ -0,0 +1,57 @@ +/** + * @file get_allocated_memory.hpp + * @author Ryan Curtin + * + * If the parameter has a type that may need to be deleted, return the address + * of that object. Otherwise return NULL. + */ +#ifndef MLPACK_BINDINGS_CLI_GET_ALLOCATED_MEMORY_HPP +#define MLPACK_BINDINGS_CLI_GET_ALLOCATED_MEMORY_HPP + +#include + +namespace mlpack { +namespace bindings { +namespace tests { + +template +void* GetAllocatedMemory( + const util::ParamData& /* d */, + const typename boost::disable_if>::type* = 0, + const typename boost::disable_if>::type* = 0) +{ + return NULL; +} + +template +void* GetAllocatedMemory( + const util::ParamData& /* d */, + const typename boost::enable_if>::type* = 0) +{ + return NULL; +} + +template +void* GetAllocatedMemory( + const util::ParamData& d, + const typename boost::disable_if>::type* = 0, + const typename boost::enable_if>::type* = 0) +{ + // Here we have a model; return its memory location. + return *boost::any_cast(&d.value); +} + +template +void GetAllocatedMemory(const util::ParamData& d, + const void* /* input */, + void* output) +{ + *((void**) output) = + GetAllocatedMemory::type>(d); +} + +} // namespace cli +} // namespace bindings +} // namespace mlpack + +#endif diff --git a/src/mlpack/bindings/tests/test_option.hpp b/src/mlpack/bindings/tests/test_option.hpp index 336d9b93db..32f2fd4222 100644 --- a/src/mlpack/bindings/tests/test_option.hpp +++ b/src/mlpack/bindings/tests/test_option.hpp @@ -18,6 +18,8 @@ #include #include "get_printable_param.hpp" #include "get_param.hpp" +#include "get_allocated_memory.hpp" +#include "delete_allocated_memory.hpp" namespace mlpack { namespace bindings { @@ -90,6 +92,10 @@ class TestOption CLI::GetSingleton().functionMap[tname]["GetPrintableParam"] = &GetPrintableParam; CLI::GetSingleton().functionMap[tname]["GetParam"] = &GetParam; + CLI::GetSingleton().functionMap[tname]["GetAllocatedMemory"] = + &GetAllocatedMemory; + CLI::GetSingleton().functionMap[tname]["DeleteAllocatedMemory"] = + &DeleteAllocatedMemory; CLI::Add(std::move(data)); From fdb379979755e7b22e4c8160883e5cd8ff2dfda3 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Mon, 29 Jan 2018 13:00:11 -0500 Subject: [PATCH 20/25] Fix subtle memory leak. --- .../methods/linear_regression/linear_regression_main.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/mlpack/methods/linear_regression/linear_regression_main.cpp b/src/mlpack/methods/linear_regression/linear_regression_main.cpp index d98a385f9c..c1e33cd069 100644 --- a/src/mlpack/methods/linear_regression/linear_regression_main.cpp +++ b/src/mlpack/methods/linear_regression/linear_regression_main.cpp @@ -170,8 +170,13 @@ static void mlpackMain() // Ensure that test file data has the right number of features. if ((lr->Parameters().n_elem - 1) != points.n_rows) { - Log::Fatal << "The model was trained on " << lr->Parameters().n_elem - 1 - << "-dimensional data, but the test points in '" + // If we built the model, nothing will free it so we have to... + const size_t dimensions = lr->Parameters().n_elem - 1; + if (computeModel) + delete lr; + + Log::Fatal << "The model was trained on " << dimensions << "-dimensional " + << "data, but the test points in '" << CLI::GetPrintableParam("test") << "' are " << points.n_rows << "-dimensional!" << endl; } From 2a5304b7b735f10f19dc4033f0ac594dd42febbd Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Mon, 29 Jan 2018 13:00:32 -0500 Subject: [PATCH 21/25] Add tests for GetAllocatedMemory() and DeleteAllocatedMemory(). --- src/mlpack/tests/cli_binding_test.cpp | 93 +++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/src/mlpack/tests/cli_binding_test.cpp b/src/mlpack/tests/cli_binding_test.cpp index 0aba4f49e4..adab141c9f 100644 --- a/src/mlpack/tests/cli_binding_test.cpp +++ b/src/mlpack/tests/cli_binding_test.cpp @@ -538,4 +538,97 @@ BOOST_AUTO_TEST_CASE(SetParamDatasetInfoMatTest) BOOST_REQUIRE_EQUAL(get<1>(t3), "new_filename.csv"); } +// Test that GetAllocatedMemory() will properly return NULL for a non-model +// type. +BOOST_AUTO_TEST_CASE(GetAllocatedMemoryNonModelTest) +{ + util::ParamData d; + + bool b = true; + d.value = boost::any(b); + d.input = true; + + void* result = (void*) 1; // Invalid pointer, should be overwritten. + + GetAllocatedMemory((const util::ParamData&) d, + (const void*) NULL, (void*) &result); + + BOOST_REQUIRE_EQUAL(result, (void*) NULL); + + // Also test with a matrix type. + arma::mat test(10, 10, arma::fill::ones); + string filename = "test.csv"; + tuple t = make_tuple(test, filename); + d.value = boost::any(t); + + result = (void*) 1; + + GetAllocatedMemory((const util::ParamData&) d, + (const void*) NULL, (void*) &result); + + BOOST_REQUIRE_EQUAL(result, (void*) NULL); +} + +// Test that GetAllocatedMemory() will properly return pointers for a +// serializable model type. +BOOST_AUTO_TEST_CASE(GetAllocatedMemoryModelTest) +{ + util::ParamData d; + + GaussianKernel g(2.0); + string filename = "hello.bin"; + tuple t = make_tuple(&g, filename); + d.value = boost::any(t); + d.input = true; + + void* result = NULL; + + GetAllocatedMemory((const util::ParamData&) d, + (const void*) NULL, (void*) &result); + + BOOST_REQUIRE_EQUAL(&g, (GaussianKernel*) result); +} + +// Test that calling DeleteAllocatedMemory() on non-model types does not delete +// pointers. +BOOST_AUTO_TEST_CASE(DeleteAllocatedMemoryNonModelTest) +{ + util::ParamData d; + + bool b = true; + d.value = boost::any(b); + d.input = true; + + DeleteAllocatedMemory((const util::ParamData&) d, + (const void*) NULL, (void*) NULL); + + arma::mat test(10, 10, arma::fill::ones); + string filename = "test.csv"; + tuple t = make_tuple(test, filename); + d.value = boost::any(t); + + DeleteAllocatedMemory((const util::ParamData&) d, + (const void*) NULL, (void*) NULL); +} + +// Test that DeleteAllocatedMemory() will properly delete pointers for a +// serializable model type. +BOOST_AUTO_TEST_CASE(DeleteAllocatedMemoryModelTest) +{ + // This test will just delete it, and we'll hope that it worked and that + // valgrind won't throw any issues (so really we can't *quite* test this in + // the context of the boost unit test framework). + util::ParamData d; + + GaussianKernel* g = new GaussianKernel(2.0); + string filename = "hello.bin"; + tuple t = make_tuple(g, filename); + + d.value = boost::any(t); + d.input = false; + + DeleteAllocatedMemory((const util::ParamData&) d, + (const void*) NULL, (void*) NULL); +} + BOOST_AUTO_TEST_SUITE_END(); From 1096b2606190dbd4deea551e7d0d12b791450047 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Mon, 29 Jan 2018 13:01:30 -0500 Subject: [PATCH 22/25] Add header guards. --- src/mlpack/tests/main_tests/test_helper.hpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/mlpack/tests/main_tests/test_helper.hpp b/src/mlpack/tests/main_tests/test_helper.hpp index 0043b6d242..12cb72c715 100644 --- a/src/mlpack/tests/main_tests/test_helper.hpp +++ b/src/mlpack/tests/main_tests/test_helper.hpp @@ -9,6 +9,10 @@ * 3-clause BSD license along with mlpack. If not, see * http://www.opensource.org/licenses/BSD-3-Clause for more information. */ +#ifndef MLPACK_TESTS_MAIN_TESTS_TEST_HELPER_HPP +#define MLPACK_TESTS_MAIN_TESTS_TEST_HELPER_HPP + +#include namespace mlpack { namespace util { @@ -31,3 +35,5 @@ void SetInputParam(const std::string& name, T&& value) } // namespace util } // namespace mlpack + +#endif From 9d59cc82e033347e7bbb56c76008c7bbc972b5ef Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Mon, 29 Jan 2018 13:01:38 -0500 Subject: [PATCH 23/25] Correctly handle memory in all main tests. --- src/mlpack/core/util/mlpack_main.hpp | 1 + .../tests/main_tests/decision_stump_test.cpp | 8 ++--- .../tests/main_tests/decision_tree_test.cpp | 1 + src/mlpack/tests/main_tests/emst_test.cpp | 1 + .../main_tests/linear_regression_test.cpp | 3 ++ src/mlpack/tests/main_tests/nbc_test.cpp | 9 ++--- src/mlpack/tests/main_tests/pca_test.cpp | 1 + .../tests/main_tests/perceptron_test.cpp | 7 ++-- .../main_tests/preprocess_binarize_test.cpp | 1 + .../main_tests/preprocess_imputer_test.cpp | 1 + .../main_tests/preprocess_split_test.cpp | 1 + .../tests/main_tests/random_forest_test.cpp | 24 +++++++------- .../main_tests/softmax_regression_test.cpp | 33 +++++++++---------- 13 files changed, 51 insertions(+), 40 deletions(-) diff --git a/src/mlpack/core/util/mlpack_main.hpp b/src/mlpack/core/util/mlpack_main.hpp index e675d3d1b0..36df283031 100644 --- a/src/mlpack/core/util/mlpack_main.hpp +++ b/src/mlpack/core/util/mlpack_main.hpp @@ -74,6 +74,7 @@ int main(int argc, char** argv) #include #include +#include // These functions will do nothing. #define PRINT_PARAM_STRING(A) std::string(" ") diff --git a/src/mlpack/tests/main_tests/decision_stump_test.cpp b/src/mlpack/tests/main_tests/decision_stump_test.cpp index 02a6212471..f5de6b89d8 100644 --- a/src/mlpack/tests/main_tests/decision_stump_test.cpp +++ b/src/mlpack/tests/main_tests/decision_stump_test.cpp @@ -35,6 +35,7 @@ struct DecisionStumpTestFixture ~DecisionStumpTestFixture() { // Clear the settings. + bindings::tests::CleanMemory(); CLI::ClearSettings(); } }; @@ -136,6 +137,9 @@ BOOST_AUTO_TEST_CASE(DecisionStumpLabelsLessDimensionTest) arma::Row predictions; predictions = std::move(CLI::GetParam>("predictions")); + // Delete the previous model. + bindings::tests::CleanMemory(); + // Now train DS with labels provided. // Delete last row of inputData. @@ -213,8 +217,6 @@ BOOST_AUTO_TEST_CASE(DecisionStumpModelReuseTest) // Check that initial predictions and final predicitons matrix // using saved model are same. CheckMatrices(predictions, CLI::GetParam>("predictions")); - - delete CLI::GetParam("output_model"); } /** @@ -256,8 +258,6 @@ BOOST_AUTO_TEST_CASE(DecisionStumpTrainingVerTest) Log::Fatal.ignoreInput = true; BOOST_REQUIRE_THROW(mlpackMain(), std::runtime_error); Log::Fatal.ignoreInput = false; - - delete CLI::GetParam("output_model"); } BOOST_AUTO_TEST_SUITE_END(); diff --git a/src/mlpack/tests/main_tests/decision_tree_test.cpp b/src/mlpack/tests/main_tests/decision_tree_test.cpp index 3511239f40..7e48d3ecbe 100644 --- a/src/mlpack/tests/main_tests/decision_tree_test.cpp +++ b/src/mlpack/tests/main_tests/decision_tree_test.cpp @@ -36,6 +36,7 @@ struct DecisionTreeTestFixture ~DecisionTreeTestFixture() { // Clear the settings. + bindings::tests::CleanMemory(); CLI::ClearSettings(); } }; diff --git a/src/mlpack/tests/main_tests/emst_test.cpp b/src/mlpack/tests/main_tests/emst_test.cpp index f49d934a88..87ca2cd3a9 100644 --- a/src/mlpack/tests/main_tests/emst_test.cpp +++ b/src/mlpack/tests/main_tests/emst_test.cpp @@ -38,6 +38,7 @@ struct EMSTTestFixture ~EMSTTestFixture() { // Clear the settings. + bindings::tests::CleanMemory(); CLI::ClearSettings(); } }; diff --git a/src/mlpack/tests/main_tests/linear_regression_test.cpp b/src/mlpack/tests/main_tests/linear_regression_test.cpp index 28a205aacd..d24e928844 100644 --- a/src/mlpack/tests/main_tests/linear_regression_test.cpp +++ b/src/mlpack/tests/main_tests/linear_regression_test.cpp @@ -32,6 +32,7 @@ struct LRTestFixture ~LRTestFixture() { // Clear the settings. + bindings::tests::CleanMemory(); CLI::ClearSettings(); } }; @@ -66,6 +67,7 @@ BOOST_AUTO_TEST_CASE(LRDifferentLambdas) mlpackMain(); const double testY1 = CLI::GetParam("output_predictions")(0); + bindings::tests::CleanMemory(); ResetSettings(); SetInputParam("training", std::move(trainX)); @@ -100,6 +102,7 @@ BOOST_AUTO_TEST_CASE(LRResponsesRepresentation) mlpackMain(); const double testY1 = CLI::GetParam("output_predictions")(0); + bindings::tests::CleanMemory(); ResetSettings(); arma::mat trainX2({1.0, 2.0, 3.0}); diff --git a/src/mlpack/tests/main_tests/nbc_test.cpp b/src/mlpack/tests/main_tests/nbc_test.cpp index 1e12cf9aa5..212e0d1d98 100644 --- a/src/mlpack/tests/main_tests/nbc_test.cpp +++ b/src/mlpack/tests/main_tests/nbc_test.cpp @@ -35,6 +35,7 @@ struct NBCTestFixture ~NBCTestFixture() { // Clear the settings. + bindings::tests::CleanMemory(); CLI::ClearSettings(); } }; @@ -142,6 +143,8 @@ BOOST_AUTO_TEST_CASE(NBCLabelsLessDimensionTest) output = std::move(CLI::GetParam>("output")); output_probs = std::move(CLI::GetParam("output_probs")); + bindings::tests::CleanMemory(); + // Now train NBC with labels provided. inputData.shed_row(inputData.n_rows - 1); @@ -226,8 +229,6 @@ BOOST_AUTO_TEST_CASE(NBCModelReuseTest) // matrix using saved model are same. CheckMatrices(output, CLI::GetParam>("output")); CheckMatrices(output_probs, CLI::GetParam("output_probs")); - - delete CLI::GetParam("output_model"); } /** @@ -251,8 +252,6 @@ BOOST_AUTO_TEST_CASE(NBCTrainingVerTest) Log::Fatal.ignoreInput = true; BOOST_REQUIRE_THROW(mlpackMain(), std::runtime_error); Log::Fatal.ignoreInput = false; - - delete CLI::GetParam("output_model"); } /** @@ -294,6 +293,8 @@ BOOST_AUTO_TEST_CASE(NBCIncrementalVarianceTest) BOOST_REQUIRE_EQUAL(CLI::GetParam>("output").n_rows, 1); BOOST_REQUIRE_EQUAL(CLI::GetParam("output_probs").n_rows, 2); + bindings::tests::CleanMemory(); + // Reset data passed. CLI::GetSingleton().Parameters()["training"].wasPassed = false; CLI::GetSingleton().Parameters()["incremental_variance"].wasPassed = false; diff --git a/src/mlpack/tests/main_tests/pca_test.cpp b/src/mlpack/tests/main_tests/pca_test.cpp index a191aef906..b2b51984ed 100644 --- a/src/mlpack/tests/main_tests/pca_test.cpp +++ b/src/mlpack/tests/main_tests/pca_test.cpp @@ -31,6 +31,7 @@ struct PCATestFixture ~PCATestFixture() { // Clear the settings. + bindings::tests::CleanMemory(); CLI::ClearSettings(); } }; diff --git a/src/mlpack/tests/main_tests/perceptron_test.cpp b/src/mlpack/tests/main_tests/perceptron_test.cpp index 653a3ccbe4..d54dd80aba 100644 --- a/src/mlpack/tests/main_tests/perceptron_test.cpp +++ b/src/mlpack/tests/main_tests/perceptron_test.cpp @@ -35,6 +35,7 @@ struct PerceptronTestFixture ~PerceptronTestFixture() { // Clear the settings. + bindings::tests::CleanMemory(); CLI::ClearSettings(); } }; @@ -137,7 +138,9 @@ BOOST_AUTO_TEST_CASE(PerceptronLabelsLessDimensionTest) arma::Row output; output = std::move(CLI::GetParam>("output")); - // Now train pereptron with labels provided. + bindings::tests::CleanMemory(); + + // Now train perceptron with labels provided. // Input training data. SetInputParam("training", std::move(inputData)); @@ -195,7 +198,7 @@ BOOST_AUTO_TEST_CASE(PerceptronModelReuseTest) // Input trained model. SetInputParam("test", std::move(testData)); SetInputParam("input_model", - std::move(CLI::GetParam("output_model"))); + CLI::GetParam("output_model")); mlpackMain(); diff --git a/src/mlpack/tests/main_tests/preprocess_binarize_test.cpp b/src/mlpack/tests/main_tests/preprocess_binarize_test.cpp index 5f47d0dad9..bf93f75464 100644 --- a/src/mlpack/tests/main_tests/preprocess_binarize_test.cpp +++ b/src/mlpack/tests/main_tests/preprocess_binarize_test.cpp @@ -35,6 +35,7 @@ struct PreprocessBinarizeTestFixture ~PreprocessBinarizeTestFixture() { // Clear the settings. + bindings::tests::CleanMemory(); CLI::ClearSettings(); } }; diff --git a/src/mlpack/tests/main_tests/preprocess_imputer_test.cpp b/src/mlpack/tests/main_tests/preprocess_imputer_test.cpp index bf30f102eb..ce1ed4f104 100644 --- a/src/mlpack/tests/main_tests/preprocess_imputer_test.cpp +++ b/src/mlpack/tests/main_tests/preprocess_imputer_test.cpp @@ -37,6 +37,7 @@ struct PreprocessImputerTestFixture ~PreprocessImputerTestFixture() { // Clear the settings. + bindings::tests::CleanMemory(); CLI::ClearSettings(); } }; diff --git a/src/mlpack/tests/main_tests/preprocess_split_test.cpp b/src/mlpack/tests/main_tests/preprocess_split_test.cpp index 5911e92b8c..509a74a3d3 100644 --- a/src/mlpack/tests/main_tests/preprocess_split_test.cpp +++ b/src/mlpack/tests/main_tests/preprocess_split_test.cpp @@ -37,6 +37,7 @@ struct PreprocessSplitTestFixture ~PreprocessSplitTestFixture() { // Clear the settings. + bindings::tests::CleanMemory(); CLI::ClearSettings(); } }; diff --git a/src/mlpack/tests/main_tests/random_forest_test.cpp b/src/mlpack/tests/main_tests/random_forest_test.cpp index 340d34f1da..ab4a8460da 100644 --- a/src/mlpack/tests/main_tests/random_forest_test.cpp +++ b/src/mlpack/tests/main_tests/random_forest_test.cpp @@ -35,6 +35,7 @@ struct RandomForestTestFixture ~RandomForestTestFixture() { // Clear the settings. + bindings::tests::CleanMemory(); CLI::ClearSettings(); } }; @@ -143,8 +144,6 @@ BOOST_AUTO_TEST_CASE(RandomForestModelReuseTest) // Check that initial predictions and predictions using saved model are same. CheckMatrices(predictions, CLI::GetParam>("predictions")); CheckMatrices(probabilities, CLI::GetParam("probabilities")); - - delete CLI::GetParam("output_model"); } /** @@ -213,8 +212,6 @@ BOOST_AUTO_TEST_CASE(RandomForestTrainingVerTest) Log::Fatal.ignoreInput = true; BOOST_REQUIRE_THROW(mlpackMain(), std::runtime_error); Log::Fatal.ignoreInput = false; - - delete CLI::GetParam("output_model"); } /** @@ -222,7 +219,7 @@ BOOST_AUTO_TEST_CASE(RandomForestTrainingVerTest) */ BOOST_AUTO_TEST_CASE(RandomForestDiffMinLeafSizeTest) { - // Train for minimium leaf size 20. + // Train for minimum leaf size 20. arma::mat inputData; if (!data::Load("vc2.csv", inputData)) BOOST_FAIL("Cannot load train dataset vc2.csv!"); @@ -246,7 +243,9 @@ BOOST_AUTO_TEST_CASE(RandomForestDiffMinLeafSizeTest) size_t correct = arma::accu(predictions == labels); double accuracy20 = (double(correct) / double(labels.n_elem) * 100); - // Train for minimium leaf size 10. + bindings::tests::CleanMemory(); + + // Train for minimum leaf size 10. // Input training data. SetInputParam("training", inputData); @@ -262,7 +261,9 @@ BOOST_AUTO_TEST_CASE(RandomForestDiffMinLeafSizeTest) correct = arma::accu(predictions == labels); double accuracy10 = (double(correct) / double(labels.n_elem) * 100); - // Train for minimium leaf size 1. + bindings::tests::CleanMemory(); + + // Train for minimum leaf size 1. // Input training data. SetInputParam("training", inputData); @@ -272,15 +273,13 @@ BOOST_AUTO_TEST_CASE(RandomForestDiffMinLeafSizeTest) mlpackMain(); // Calculate training accuracy. - CLI::GetParam("output_model").rf.Classify(inputData, + CLI::GetParam("output_model")->rf.Classify(inputData, predictions); correct = arma::accu(predictions == labels); double accuracy1 = (double(correct) / double(labels.n_elem) * 100); BOOST_REQUIRE(accuracy1 > accuracy10 && accuracy10 > accuracy20); - - delete CLI::GetParam("output_model"); } /** @@ -316,7 +315,7 @@ BOOST_AUTO_TEST_CASE(RandomForestDiffNumTreeTest) arma::Row predictions; CLI::GetParam("output_model")->rf.Classify(testData, predictions); - delete CLI::GetParam("output_model"); + bindings::tests::CleanMemory(); size_t correct = arma::accu(predictions == testLabels); double accuracy1 = (double(correct) / double(testLabels.n_elem) * 100); @@ -333,7 +332,7 @@ BOOST_AUTO_TEST_CASE(RandomForestDiffNumTreeTest) // Calculate training accuracy. CLI::GetParam("output_model")->rf.Classify(testData, predictions); - delete CLI::GetParam("output_model"); + bindings::tests::CleanMemory(); correct = arma::accu(predictions == testLabels); double accuracy5 = (double(correct) / double(testLabels.n_elem) * 100); @@ -350,7 +349,6 @@ BOOST_AUTO_TEST_CASE(RandomForestDiffNumTreeTest) // Calculate training accuracy. CLI::GetParam("output_model")->rf.Classify(testData, predictions); - delete CLI::GetParam("output_model"); correct = arma::accu(predictions == testLabels); double accuracy10 = (double(correct) / double(testLabels.n_elem) * 100); diff --git a/src/mlpack/tests/main_tests/softmax_regression_test.cpp b/src/mlpack/tests/main_tests/softmax_regression_test.cpp index 358ad1b8bd..f646b69b9c 100644 --- a/src/mlpack/tests/main_tests/softmax_regression_test.cpp +++ b/src/mlpack/tests/main_tests/softmax_regression_test.cpp @@ -35,6 +35,7 @@ struct SoftmaxRegressionTestFixture ~SoftmaxRegressionTestFixture() { // Clear the settings. + bindings::tests::CleanMemory(); CLI::ClearSettings(); } }; @@ -165,8 +166,6 @@ BOOST_AUTO_TEST_CASE(SoftmaxRegressionModelReuseTest) // Check that initial predictions and final predicitons matrix // using saved model are same. CheckMatrices(predictions, CLI::GetParam>("predictions")); - - delete CLI::GetParam("output_model"); } /** @@ -275,7 +274,7 @@ BOOST_AUTO_TEST_CASE(SoftmaxRegressionTrainingVerTest) // Input pre-trained model. SetInputParam("input_model", - std::move(CLI::GetParam("output_model"))); + CLI::GetParam("output_model")); Log::Fatal.ignoreInput = true; BOOST_REQUIRE_THROW(mlpackMain(), std::runtime_error); @@ -308,8 +307,6 @@ BOOST_AUTO_TEST_CASE(SoftmaxRegressionDiffLambdaTest) // Delete the last row containing labels from test dataset. testData.shed_row(testData.n_rows - 1); - size_t testSize = testData.n_cols; - // Input training data. SetInputParam("training", inputData); SetInputParam("labels", labels); @@ -322,7 +319,9 @@ BOOST_AUTO_TEST_CASE(SoftmaxRegressionDiffLambdaTest) // Store output parameters. arma::mat modelParam; - modelParam = CLI::GetParam("output_model").Parameters(); + modelParam = CLI::GetParam("output_model")->Parameters(); + + bindings::tests::CleanMemory(); // Reset passed parameters. CLI::GetSingleton().Parameters()["training"].wasPassed = false; @@ -344,13 +343,13 @@ BOOST_AUTO_TEST_CASE(SoftmaxRegressionDiffLambdaTest) for (size_t i = 0; i < modelParam.n_elem; ++i) { BOOST_REQUIRE_NE(modelParam[i], - CLI::GetParam("output_model").Parameters()[i]); + CLI::GetParam("output_model")->Parameters()[i]); } } /** - * Check that output object parameters are different - * for different numbers of max_iterations. + * Check that output object parameters are different for different numbers of + * max_iterations. */ BOOST_AUTO_TEST_CASE(SoftmaxRegressionDiffMaxItrTest) { @@ -374,8 +373,6 @@ BOOST_AUTO_TEST_CASE(SoftmaxRegressionDiffMaxItrTest) // Delete the last row containing labels from test dataset. testData.shed_row(testData.n_rows - 1); - size_t testSize = testData.n_cols; - // Input training data. SetInputParam("training", inputData); SetInputParam("labels", labels); @@ -388,7 +385,9 @@ BOOST_AUTO_TEST_CASE(SoftmaxRegressionDiffMaxItrTest) // Store output parameters. arma::mat modelParam; - modelParam = CLI::GetParam("output_model").Parameters(); + modelParam = CLI::GetParam("output_model")->Parameters(); + + bindings::tests::CleanMemory(); // Reset passed parameters. CLI::GetSingleton().Parameters()["training"].wasPassed = false; @@ -410,7 +409,7 @@ BOOST_AUTO_TEST_CASE(SoftmaxRegressionDiffMaxItrTest) for (size_t i = 0; i < modelParam.n_elem; ++i) { BOOST_REQUIRE_NE(modelParam[i], - CLI::GetParam("output_model").Parameters()[i]); + CLI::GetParam("output_model")->Parameters()[i]); } } @@ -440,8 +439,6 @@ BOOST_AUTO_TEST_CASE(SoftmaxRegressionDiffInterceptTest) // Delete the last row containing labels from test dataset. testData.shed_row(testData.n_rows - 1); - size_t testSize = testData.n_cols; - // Input training data. SetInputParam("training", inputData); SetInputParam("labels", labels); @@ -454,7 +451,9 @@ BOOST_AUTO_TEST_CASE(SoftmaxRegressionDiffInterceptTest) // Store output parameters. arma::mat modelParam; - modelParam = CLI::GetParam("output_model").Parameters(); + modelParam = CLI::GetParam("output_model")->Parameters(); + + bindings::tests::CleanMemory(); // Reset passed parameters. CLI::GetSingleton().Parameters()["training"].wasPassed = false; @@ -474,7 +473,7 @@ BOOST_AUTO_TEST_CASE(SoftmaxRegressionDiffInterceptTest) // Check that initial parameters has 1 more parameter than // final parameters matrix. BOOST_REQUIRE_EQUAL( - CLI::GetParam("output_model").Parameters().n_cols, + CLI::GetParam("output_model")->Parameters().n_cols, modelParam.n_cols + 1); } From a402f61455ff1a0f2fd2eab10b06d997e051e395 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Mon, 29 Jan 2018 13:05:21 -0500 Subject: [PATCH 24/25] Fix comments. --- src/mlpack/bindings/tests/delete_allocated_memory.hpp | 2 +- src/mlpack/bindings/tests/get_allocated_memory.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mlpack/bindings/tests/delete_allocated_memory.hpp b/src/mlpack/bindings/tests/delete_allocated_memory.hpp index f39deb113b..a0541a19e6 100644 --- a/src/mlpack/bindings/tests/delete_allocated_memory.hpp +++ b/src/mlpack/bindings/tests/delete_allocated_memory.hpp @@ -49,7 +49,7 @@ void DeleteAllocatedMemory( DeleteAllocatedMemoryImpl::type>(d); } -} // namespace cli +} // namespace tests } // namespace bindings } // namespace mlpack diff --git a/src/mlpack/bindings/tests/get_allocated_memory.hpp b/src/mlpack/bindings/tests/get_allocated_memory.hpp index ec53740a40..82f47e1130 100644 --- a/src/mlpack/bindings/tests/get_allocated_memory.hpp +++ b/src/mlpack/bindings/tests/get_allocated_memory.hpp @@ -50,7 +50,7 @@ void GetAllocatedMemory(const util::ParamData& d, GetAllocatedMemory::type>(d); } -} // namespace cli +} // namespace tests } // namespace bindings } // namespace mlpack From ac89b49e27e7f3df8c23cd92249ff6a44a4be7ef Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Tue, 30 Jan 2018 19:06:31 -0500 Subject: [PATCH 25/25] Minor comment fixes. --- src/mlpack/bindings/python/print_input_processing.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mlpack/bindings/python/print_input_processing.hpp b/src/mlpack/bindings/python/print_input_processing.hpp index bb688ad463..3cdf096c0e 100644 --- a/src/mlpack/bindings/python/print_input_processing.hpp +++ b/src/mlpack/bindings/python/print_input_processing.hpp @@ -31,8 +31,8 @@ void PrintInputProcessing( const typename boost::disable_if>>::type* = 0) { - // The copy_all_inputs parameter must be handled first, and so is outside the - // scope of this code. + // The copy_all_inputs parameter must be handled first, and therefore is + // outside the scope of this code. if (d.name == "copy_all_inputs") return; @@ -179,7 +179,7 @@ void PrintInputProcessing( * except TypeError as e: * if type(param_name).__name__ == "ModelType": * SetParamPtr[Model]('param_name', ( param_name).modelptr, - * CLI.HasParam('copy_all_inputs')) TODO + * CLI.HasParam('copy_all_inputs')) * else: * raise e * CLI.SetPassed( 'param_name')