From 4a7b8df8f73e2dafbe57e478dc67a826f4204eec Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 15 Sep 2022 19:00:25 -0400 Subject: [PATCH 01/11] Don't set `strict` for aliases in bindings. --- src/mlpack/bindings/go/mlpack/capi/arma_util.cpp | 14 +++++++------- src/mlpack/bindings/go/mlpack/capi/io_util.cpp | 1 - src/mlpack/bindings/julia/julia_util.cpp | 14 +++++++------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/mlpack/bindings/go/mlpack/capi/arma_util.cpp b/src/mlpack/bindings/go/mlpack/capi/arma_util.cpp index 5610273071..c8deee547e 100644 --- a/src/mlpack/bindings/go/mlpack/capi/arma_util.cpp +++ b/src/mlpack/bindings/go/mlpack/capi/arma_util.cpp @@ -33,7 +33,7 @@ void mlpackToArmaMat(void* params, util::Params& p = *((util::Params*) params); // Advanced constructor. - arma::mat m(mat, row, col, false, true); + arma::mat m(mat, row, col, false, false); // Set input parameter with corresponding matrix in IO. SetParam(p, identifier, m); @@ -51,7 +51,7 @@ void mlpackToArmaUmat(void* params, util::Params& p = *((util::Params*) params); // Advanced constructor. - arma::mat m(mat, row, col, false, true); + arma::mat m(mat, row, col, false, false); arma::Mat matr = arma::conv_to>::from(m); @@ -70,7 +70,7 @@ void mlpackToArmaRow(void* params, util::Params& p = *((util::Params*) params); // Advanced constructor. - arma::rowvec m(rowvec, elem, false, true); + arma::rowvec m(rowvec, elem, false, false); // Set input parameter with corresponding row in IO. SetParam(p, identifier, m); @@ -87,7 +87,7 @@ void mlpackToArmaUrow(void* params, util::Params& p = *((util::Params*) params); // Advanced constructor. - arma::rowvec m(rowvec, elem, false, true); + arma::rowvec m(rowvec, elem, false, false); arma::Row matr = arma::conv_to>::from(m); @@ -106,7 +106,7 @@ void mlpackToArmaCol(void* params, util::Params& p = *((util::Params*) params); // Advanced constructor. - arma::colvec m(colvec, elem, false, true); + arma::colvec m(colvec, elem, false, false); // Set input parameter with corresponding column in IO. SetParam(p, identifier, m); @@ -123,7 +123,7 @@ void mlpackToArmaUcol(void* params, util::Params& p = *((util::Params*) params); // Advanced constructor. - arma::colvec m(colvec, elem, false, true); + arma::colvec m(colvec, elem, false, false); arma::Col matr = arma::conv_to>::from(m); @@ -337,7 +337,7 @@ void mlpackToArmaMatWithInfo(void* params, hasCategoricals = true; } - arma::mat m(memptr, rows, cols, false, true); + arma::mat m(memptr, rows, cols, false, false); // Do we need to find how many categories we have? if (hasCategoricals) diff --git a/src/mlpack/bindings/go/mlpack/capi/io_util.cpp b/src/mlpack/bindings/go/mlpack/capi/io_util.cpp index 86dfed3e8c..f25b8b9454 100644 --- a/src/mlpack/bindings/go/mlpack/capi/io_util.cpp +++ b/src/mlpack/bindings/go/mlpack/capi/io_util.cpp @@ -24,7 +24,6 @@ extern "C" { void* mlpackGetParams(const char* bindingName) { util::Params* p = new util::Params(IO::Parameters(bindingName)); - std::cout << "created params p " << p << "\n"; return (void*) p; } diff --git a/src/mlpack/bindings/julia/julia_util.cpp b/src/mlpack/bindings/julia/julia_util.cpp index 6c61cf0d6d..1e7781951f 100644 --- a/src/mlpack/bindings/julia/julia_util.cpp +++ b/src/mlpack/bindings/julia/julia_util.cpp @@ -157,7 +157,7 @@ void SetParamMat(void* params, util::Params* p = (util::Params*) params; // Create the matrix as an alias. - arma::mat m(memptr, arma::uword(rows), arma::uword(cols), false, true); + arma::mat m(memptr, arma::uword(rows), arma::uword(cols), false, false); p->Get(paramName) = pointsAsRows ? m.t() : std::move(m); p->SetPassed(paramName); } @@ -176,7 +176,7 @@ void SetParamUMat(void* params, // Create the matrix as an alias. arma::Mat m(memptr, arma::uword(rows), arma::uword(cols), false, - true); + false); p->Get>(paramName) = pointsAsRows ? m.t() : std::move(m); p->SetPassed(paramName); @@ -191,7 +191,7 @@ void SetParamRow(void* params, const size_t cols) { util::Params* p = (util::Params*) params; - arma::rowvec m(memptr, arma::uword(cols), false, true); + arma::rowvec m(memptr, arma::uword(cols), false, false); p->Get(paramName) = std::move(m); p->SetPassed(paramName); } @@ -205,7 +205,7 @@ void SetParamURow(void* params, const size_t cols) { util::Params* p = (util::Params*) params; - arma::Row m(memptr, arma::uword(cols), false, true); + arma::Row m(memptr, arma::uword(cols), false, false); p->Get>(paramName) = std::move(m); p->SetPassed(paramName); } @@ -219,7 +219,7 @@ void SetParamCol(void* params, const size_t rows) { util::Params* p = (util::Params*) params; - arma::vec m(memptr, arma::uword(rows), false, true); + arma::vec m(memptr, arma::uword(rows), false, false); p->Get(paramName) = std::move(m); p->SetPassed(paramName); } @@ -233,7 +233,7 @@ void SetParamUCol(void* params, const size_t rows) { util::Params* p = (util::Params*) params; - arma::Col m(memptr, arma::uword(rows), false, true); + arma::Col m(memptr, arma::uword(rows), false, false); p->Get>(paramName) = std::move(m); p->SetPassed(paramName); } @@ -260,7 +260,7 @@ void SetParamMatWithInfo(void* params, hasCategoricals = true; } - arma::mat m(memptr, arma::uword(rows), arma::uword(cols), false, true); + arma::mat m(memptr, arma::uword(rows), arma::uword(cols), false, false); // Do we need to find how many categories we have? if (hasCategoricals) From b8cb6b128348cb5ffaabcd15bdf2f089edd8a9c3 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Wed, 21 Sep 2022 13:40:35 -0400 Subject: [PATCH 02/11] Fix some comparisons. --- src/mlpack/bindings/julia/tests/runtests.jl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/mlpack/bindings/julia/tests/runtests.jl b/src/mlpack/bindings/julia/tests/runtests.jl index 26fbc56283..38ffef8ef5 100644 --- a/src/mlpack/bindings/julia/tests/runtests.jl +++ b/src/mlpack/bindings/julia/tests/runtests.jl @@ -181,6 +181,7 @@ end # Test a column vector input parameter. @testset "TestCol" begin x = rand(100) + oldX = copy(x) colOut, _, _, _, _, _, _, _, _, _, _, _, _, _ = test_julia_binding(4.0, 12, "hello", @@ -190,13 +191,14 @@ end @test typeof(colOut) == Array{Float64, 1} for i in 1:100 - @test colOut[i] == 2 * x[i] + @test colOut[i] == 2 * oldX[i] end end # Test an unsigned column vector input parameter. @testset "TestUCol" begin x = convert(Array{Int, 1}, rand(1:500, 100)) + oldX = copy(x) _, _, _, _, _, _, _, _, _, _, ucolOut, _, _, _ = test_julia_binding(4.0, 12, "hello", @@ -207,7 +209,7 @@ end for i in 1:100 # Since we subtract one when we convert to C++, and then add one when we # convert back, we get a slightly different result here. - @test ucolOut[i] == 2 * x[i] - 1 + @test ucolOut[i] == 2 * oldX[i] - 1 end end From f919505650004bc248ef8e2853c7fb204fb94df9 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Wed, 21 Sep 2022 13:44:18 -0400 Subject: [PATCH 03/11] Preserve old Go objects for comparisons. --- .../bindings/go/tests/go_binding_test.go | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/mlpack/bindings/go/tests/go_binding_test.go b/src/mlpack/bindings/go/tests/go_binding_test.go index 7b9a66ad98..0e53516c52 100644 --- a/src/mlpack/bindings/go/tests/go_binding_test.go +++ b/src/mlpack/bindings/go/tests/go_binding_test.go @@ -313,6 +313,8 @@ func TestGonumRow(t *testing.T) { x := mat.NewDense(9, 1, []float64{ 1, 2, 3, 4, 5, 6, 7, 8, 9, }) + var oldX mat.Dense + oldX.copy(x) param := mlpack.TestGoBindingOptions() param.RowIn = x @@ -327,9 +329,9 @@ func TestGonumRow(t *testing.T) { t.Errorf("Error. Wrong shape.") } for i := 0; i < rows; i++ { - if RowOut.At(i, 0) != x.At(i, 0)*2 { + if RowOut.At(i, 0) != oldX.At(i, 0)*2 { val := RowOut.At(i, 0) - expected := x.At(i, 0) * 2 + expected := oldX.At(i, 0) * 2 t.Errorf("Error. Value at [i] : %v. Expected value : %v", val, expected) } @@ -369,6 +371,8 @@ func TestGonumCol(t *testing.T) { x := mat.NewDense(1, 9, []float64{ 1, 2, 3, 4, 5, 6, 7, 8, 9, }) + var oldX mat.Dense + oldX.copy(x) param := mlpack.TestGoBindingOptions() param.ColIn = x @@ -383,9 +387,9 @@ func TestGonumCol(t *testing.T) { t.Errorf("Error. Wrong shape.") } for i := 0; i < cols; i++ { - if ColOut.At(0, i) != x.At(0, i)*2 { + if ColOut.At(0, i) != oldX.At(0, i)*2 { val := ColOut.At(0, i) - expected := x.At(0, i) * 2 + expected := oldX.At(0, i) * 2 t.Errorf("Error. Value at [i] : %v. Expected value : %v", val, expected) } @@ -549,6 +553,9 @@ func TestGonumMatrixWithInfo(t *testing.T) { 11, 12, 13, 14, 15, }) + var oldX mat.Dense + oldX.copy(x) + param := mlpack.TestGoBindingOptions() param.MatrixAndInfoIn = x d := 4.0 @@ -564,9 +571,9 @@ func TestGonumMatrixWithInfo(t *testing.T) { } for i := 0; i < rows; i++ { for j := 0; j < cols; j++ { - if x.Data.At(i, j)*2 != MatrixAndInfoOut.At(i, j) { + if oldX.Data.At(i, j)*2 != MatrixAndInfoOut.At(i, j) { val := MatrixAndInfoOut.At(i, j) - expected := x.Data.At(i, j)*2 + expected := oldX.Data.At(i, j)*2 t.Errorf("Error. Value at [%v,%v] : %v. Expected value : %v", i, j, val, expected) } @@ -592,6 +599,9 @@ func TestGonumMatrixWithInfoCategorical(t *testing.T) { 0.3, 0.0, 1, 1, 0.6, }) + var oldX mat.Dense + oldX.copy(x) + param := mlpack.TestGoBindingOptions() param.MatrixAndInfoIn = x d := 4.0 @@ -608,16 +618,16 @@ func TestGonumMatrixWithInfoCategorical(t *testing.T) { for i := 0; i < rows; i++ { for j := 0; j < cols; j++ { if j == 0 || j == 1 || j == 4 { - if x.Data.At(i, j) * 2 != MatrixAndInfoOut.At(i, j) { + if oldX.Data.At(i, j) * 2 != MatrixAndInfoOut.At(i, j) { val := MatrixAndInfoOut.At(i, j) - expected := x.Data.At(i, j)*2 + expected := oldX.Data.At(i, j)*2 t.Errorf("Error. Value at [%v,%v] : %v. Expected value : %v", i, j, val, expected) } } else { - if x.Data.At(i, j) != MatrixAndInfoOut.At(i, j) { + if oldX.Data.At(i, j) != MatrixAndInfoOut.At(i, j) { val := MatrixAndInfoOut.At(i, j) - expected := x.Data.At(i, j) + expected := oldX.Data.At(i, j) t.Errorf("Error. Value at [%v,%v] : %v. Expected value: %v", i, j, val, expected) } From bcff315728d9c788e9648d9bf2ae43f772de13ca Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Wed, 21 Sep 2022 15:02:21 -0400 Subject: [PATCH 04/11] Oops, it's Copy() not copy(). --- .../bindings/go/tests/go_binding_test.go | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/mlpack/bindings/go/tests/go_binding_test.go b/src/mlpack/bindings/go/tests/go_binding_test.go index 0e53516c52..d164f017c1 100644 --- a/src/mlpack/bindings/go/tests/go_binding_test.go +++ b/src/mlpack/bindings/go/tests/go_binding_test.go @@ -1,15 +1,15 @@ package main import ( - "mlpack.org/v1/mlpack" - "testing" - "os" + "mlpack.org/v1/mlpack" + "testing" + "os" - "gonum.org/v1/gonum/mat" + "gonum.org/v1/gonum/mat" ) func TestRunBindingNoFlag(t *testing.T) { - t.Log("Test that if we forget the mandatory flag, we should get wrong", + t.Log("Test that if we forget the mandatory flag, we should get wrong", "results.") param := mlpack.TestGoBindingOptions() d := 4.0 @@ -30,7 +30,7 @@ func TestRunBindingNoFlag(t *testing.T) { } func TestRunBindingCorrectly(t *testing.T) { - t.Log("Test that when we run the binding correctly (with correct", + t.Log("Test that when we run the binding correctly (with correct", " input parameters), we get the expected output.") param := mlpack.TestGoBindingOptions() param.Flag1 = true @@ -314,7 +314,7 @@ func TestGonumRow(t *testing.T) { 1, 2, 3, 4, 5, 6, 7, 8, 9, }) var oldX mat.Dense - oldX.copy(x) + oldX.Copy(x) param := mlpack.TestGoBindingOptions() param.RowIn = x @@ -372,7 +372,7 @@ func TestGonumCol(t *testing.T) { 1, 2, 3, 4, 5, 6, 7, 8, 9, }) var oldX mat.Dense - oldX.copy(x) + oldX.Copy(x) param := mlpack.TestGoBindingOptions() param.ColIn = x @@ -554,7 +554,7 @@ func TestGonumMatrixWithInfo(t *testing.T) { }) var oldX mat.Dense - oldX.copy(x) + oldX.Copy(x) param := mlpack.TestGoBindingOptions() param.MatrixAndInfoIn = x @@ -600,7 +600,7 @@ func TestGonumMatrixWithInfoCategorical(t *testing.T) { }) var oldX mat.Dense - oldX.copy(x) + oldX.Copy(x) param := mlpack.TestGoBindingOptions() param.MatrixAndInfoIn = x From bf42076c72b3b3c0823136622477c73b0050679f Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Wed, 21 Sep 2022 17:41:33 -0400 Subject: [PATCH 05/11] Oops, correct copy of categorical data. --- src/mlpack/bindings/go/tests/go_binding_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/mlpack/bindings/go/tests/go_binding_test.go b/src/mlpack/bindings/go/tests/go_binding_test.go index d164f017c1..2e93f89988 100644 --- a/src/mlpack/bindings/go/tests/go_binding_test.go +++ b/src/mlpack/bindings/go/tests/go_binding_test.go @@ -554,7 +554,7 @@ func TestGonumMatrixWithInfo(t *testing.T) { }) var oldX mat.Dense - oldX.Copy(x) + oldX.Copy(x.Data) param := mlpack.TestGoBindingOptions() param.MatrixAndInfoIn = x @@ -571,9 +571,9 @@ func TestGonumMatrixWithInfo(t *testing.T) { } for i := 0; i < rows; i++ { for j := 0; j < cols; j++ { - if oldX.Data.At(i, j)*2 != MatrixAndInfoOut.At(i, j) { + if oldX.At(i, j)*2 != MatrixAndInfoOut.At(i, j) { val := MatrixAndInfoOut.At(i, j) - expected := oldX.Data.At(i, j)*2 + expected := oldX.At(i, j)*2 t.Errorf("Error. Value at [%v,%v] : %v. Expected value : %v", i, j, val, expected) } @@ -600,7 +600,7 @@ func TestGonumMatrixWithInfoCategorical(t *testing.T) { }) var oldX mat.Dense - oldX.Copy(x) + oldX.Copy(x.Data) param := mlpack.TestGoBindingOptions() param.MatrixAndInfoIn = x @@ -618,16 +618,16 @@ func TestGonumMatrixWithInfoCategorical(t *testing.T) { for i := 0; i < rows; i++ { for j := 0; j < cols; j++ { if j == 0 || j == 1 || j == 4 { - if oldX.Data.At(i, j) * 2 != MatrixAndInfoOut.At(i, j) { + if oldX.At(i, j) * 2 != MatrixAndInfoOut.At(i, j) { val := MatrixAndInfoOut.At(i, j) - expected := oldX.Data.At(i, j)*2 + expected := oldX.At(i, j)*2 t.Errorf("Error. Value at [%v,%v] : %v. Expected value : %v", i, j, val, expected) } } else { - if oldX.Data.At(i, j) != MatrixAndInfoOut.At(i, j) { + if oldX.At(i, j) != MatrixAndInfoOut.At(i, j) { val := MatrixAndInfoOut.At(i, j) - expected := oldX.Data.At(i, j) + expected := oldX.At(i, j) t.Errorf("Error. Value at [%v,%v] : %v. Expected value: %v", i, j, val, expected) } From 853bd0d6ebfc09e667828a5469f9dd9ea84dfae1 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 22 Sep 2022 10:54:56 -0400 Subject: [PATCH 06/11] Try to get a little information on what is going on. --- src/mlpack/bindings/go/tests/go_binding_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/mlpack/bindings/go/tests/go_binding_test.go b/src/mlpack/bindings/go/tests/go_binding_test.go index 2e93f89988..4ffd8201cf 100644 --- a/src/mlpack/bindings/go/tests/go_binding_test.go +++ b/src/mlpack/bindings/go/tests/go_binding_test.go @@ -328,6 +328,10 @@ func TestGonumRow(t *testing.T) { if rows != 9 { t.Errorf("Error. Wrong shape.") } + oldRows, oldCols := oldX.Dims() + if oldRows != 9 { + t.Errorf("Error, oldX has shape %v, %v", oldRows, oldCols) + } for i := 0; i < rows; i++ { if RowOut.At(i, 0) != oldX.At(i, 0)*2 { val := RowOut.At(i, 0) From 59fdd68fc4b958b2f4eea82855ac48acb31905f0 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 22 Sep 2022 13:21:47 -0400 Subject: [PATCH 07/11] Do I have to allocate the size correctly first? --- src/mlpack/bindings/go/tests/go_binding_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mlpack/bindings/go/tests/go_binding_test.go b/src/mlpack/bindings/go/tests/go_binding_test.go index 4ffd8201cf..cd4f3ff0e1 100644 --- a/src/mlpack/bindings/go/tests/go_binding_test.go +++ b/src/mlpack/bindings/go/tests/go_binding_test.go @@ -313,7 +313,7 @@ func TestGonumRow(t *testing.T) { x := mat.NewDense(9, 1, []float64{ 1, 2, 3, 4, 5, 6, 7, 8, 9, }) - var oldX mat.Dense + oldX := mat.NewDense(9, 1, nil) oldX.Copy(x) param := mlpack.TestGoBindingOptions() From 480506977eadf008936587c36cf44ba6e9ab5e34 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 22 Sep 2022 15:47:36 -0400 Subject: [PATCH 08/11] Always allocate to the correct size. --- src/mlpack/bindings/go/tests/go_binding_test.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/mlpack/bindings/go/tests/go_binding_test.go b/src/mlpack/bindings/go/tests/go_binding_test.go index cd4f3ff0e1..421690adfd 100644 --- a/src/mlpack/bindings/go/tests/go_binding_test.go +++ b/src/mlpack/bindings/go/tests/go_binding_test.go @@ -328,10 +328,6 @@ func TestGonumRow(t *testing.T) { if rows != 9 { t.Errorf("Error. Wrong shape.") } - oldRows, oldCols := oldX.Dims() - if oldRows != 9 { - t.Errorf("Error, oldX has shape %v, %v", oldRows, oldCols) - } for i := 0; i < rows; i++ { if RowOut.At(i, 0) != oldX.At(i, 0)*2 { val := RowOut.At(i, 0) @@ -375,7 +371,7 @@ func TestGonumCol(t *testing.T) { x := mat.NewDense(1, 9, []float64{ 1, 2, 3, 4, 5, 6, 7, 8, 9, }) - var oldX mat.Dense + oldX := mat.NewDense(9, 1, nil) oldX.Copy(x) param := mlpack.TestGoBindingOptions() @@ -557,7 +553,7 @@ func TestGonumMatrixWithInfo(t *testing.T) { 11, 12, 13, 14, 15, }) - var oldX mat.Dense + oldX := mat.NewDense(3, 5, nil) oldX.Copy(x.Data) param := mlpack.TestGoBindingOptions() @@ -603,7 +599,7 @@ func TestGonumMatrixWithInfoCategorical(t *testing.T) { 0.3, 0.0, 1, 1, 0.6, }) - var oldX mat.Dense + oldX := mat.NewDense(6, 5, nil) oldX.Copy(x.Data) param := mlpack.TestGoBindingOptions() From 3d435995d8ce9c989f7de0b253cf935d7d87afb7 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Thu, 22 Sep 2022 17:39:58 -0400 Subject: [PATCH 09/11] Oops, correct the size. --- src/mlpack/bindings/go/tests/go_binding_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mlpack/bindings/go/tests/go_binding_test.go b/src/mlpack/bindings/go/tests/go_binding_test.go index 421690adfd..65e7f0d248 100644 --- a/src/mlpack/bindings/go/tests/go_binding_test.go +++ b/src/mlpack/bindings/go/tests/go_binding_test.go @@ -371,7 +371,7 @@ func TestGonumCol(t *testing.T) { x := mat.NewDense(1, 9, []float64{ 1, 2, 3, 4, 5, 6, 7, 8, 9, }) - oldX := mat.NewDense(9, 1, nil) + oldX := mat.NewDense(1, 9, nil) oldX.Copy(x) param := mlpack.TestGoBindingOptions() From f60bf591e431df3752a0d0e2a04ea5485514c416 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Wed, 28 Sep 2022 18:38:34 -0400 Subject: [PATCH 10/11] Clean up memory handling: ensure Julia GC doesn't double-free. --- src/mlpack/bindings/julia/julia_util.cpp | 41 ++-- src/mlpack/bindings/julia/julia_util.h | 19 +- src/mlpack/bindings/julia/mlpack/params.jl.in | 182 ++++++++++++++---- .../julia/print_input_processing_impl.hpp | 11 +- src/mlpack/bindings/julia/print_jl.cpp | 6 + .../julia/print_output_processing_impl.hpp | 4 +- src/mlpack/bindings/julia/tests/runtests.jl | 23 +-- 7 files changed, 206 insertions(+), 80 deletions(-) diff --git a/src/mlpack/bindings/julia/julia_util.cpp b/src/mlpack/bindings/julia/julia_util.cpp index 1e7781951f..0f1819496a 100644 --- a/src/mlpack/bindings/julia/julia_util.cpp +++ b/src/mlpack/bindings/julia/julia_util.cpp @@ -128,7 +128,7 @@ void SetParamVectorStrStr(void* params, */ void SetParamVectorInt(void* params, const char* paramName, - int* ints, + long long* ints, const size_t length) { util::Params* p = (util::Params*) params; @@ -138,7 +138,7 @@ void SetParamVectorInt(void* params, std::vector vec; vec.resize(length); for (size_t i = 0; i < length; ++i) - vec[i] = ints[i]; + vec[i] = int(ints[i]); p->Get>(paramName) = std::move(vec); p->SetPassed(paramName); @@ -164,10 +164,13 @@ void SetParamMat(void* params, /** * Call params.SetParam>(). + * + * Note that we will have to allocate memory, since we must convert to a size_t + * matrix, and we also subtract by one (since Julia uses 1-indexed labels). */ void SetParamUMat(void* params, const char* paramName, - size_t* memptr, + long long* memptr, const size_t rows, const size_t cols, const bool pointsAsRows) @@ -175,10 +178,11 @@ void SetParamUMat(void* params, util::Params* p = (util::Params*) params; // Create the matrix as an alias. - arma::Mat m(memptr, arma::uword(rows), arma::uword(cols), false, - false); - p->Get>(paramName) = pointsAsRows ? m.t() : - std::move(m); + arma::Mat m(memptr, arma::uword(rows), arma::uword(cols), true, + true); + arma::Mat convM = arma::conv_to>::from(m - 1); + p->Get>(paramName) = pointsAsRows ? convM.t() : + std::move(convM); p->SetPassed(paramName); } @@ -201,12 +205,13 @@ void SetParamRow(void* params, */ void SetParamURow(void* params, const char* paramName, - size_t* memptr, + long long* memptr, const size_t cols) { util::Params* p = (util::Params*) params; - arma::Row m(memptr, arma::uword(cols), false, false); - p->Get>(paramName) = std::move(m); + arma::Row m(memptr, arma::uword(cols), false, true); + arma::Row convM = arma::conv_to>::from(m - 1); + p->Get>(paramName) = std::move(convM); p->SetPassed(paramName); } @@ -229,12 +234,13 @@ void SetParamCol(void* params, */ void SetParamUCol(void* params, const char* paramName, - size_t* memptr, + long long* memptr, const size_t rows) { util::Params* p = (util::Params*) params; - arma::Col m(memptr, arma::uword(rows), false, false); - p->Get>(paramName) = std::move(m); + arma::Col m(memptr, arma::uword(rows), false, true); + arma::Col convM = arma::conv_to>::from(m - 1); + p->Get>(paramName) = std::move(convM); p->SetPassed(paramName); } @@ -365,14 +371,17 @@ size_t GetParamVectorIntLen(void* params, const char* paramName) * The vector will be created in-place and it is expected that the calling * function will take ownership. */ -int* GetParamVectorIntPtr(void* params, const char* paramName) +long long* GetParamVectorIntPtr(void* params, const char* paramName) { util::Params* p = (util::Params*) params; const size_t size = p->Get>(paramName).size(); - int* ints = new int[size]; + if (size == 0) + return NULL; + + long long* ints = new long long[size]; for (size_t i = 0; i < size; ++i) - ints[i] = p->Get>(paramName)[i]; + ints[i] = (long long)(p->Get>(paramName)[i]); return ints; } diff --git a/src/mlpack/bindings/julia/julia_util.h b/src/mlpack/bindings/julia/julia_util.h index b990678d7f..b3e01620b1 100644 --- a/src/mlpack/bindings/julia/julia_util.h +++ b/src/mlpack/bindings/julia/julia_util.h @@ -84,7 +84,7 @@ void SetParamVectorStrStr(void* params, */ void SetParamVectorInt(void* params, const char* paramName, - int* ints, + long long* ints, const size_t length); /** @@ -99,10 +99,13 @@ void SetParamMat(void* params, /** * Call params.SetParam>(). + * + * Note that we will have to allocate memory, since we must convert to a size_t + * matrix, and we also subtract by one (since Julia uses 1-indexed labels). */ void SetParamUMat(void* params, const char* paramName, - size_t* memptr, + long long* memptr, const size_t rows, const size_t cols, const bool pointsAsRows); @@ -117,10 +120,13 @@ void SetParamRow(void* params, /** * Call params.SetParam>(). + * + * Note that we will have to allocate memory, since we must convert to a size_t + * vector, and we also subtract by one (since Julia uses 1-indexed labels). */ void SetParamURow(void* params, const char* paramName, - size_t* memptr, + long long* memptr, const size_t cols); /** @@ -133,10 +139,13 @@ void SetParamCol(void* params, /** * Call params.SetParam>(). + * + * Note that we will have to allocate memory, since we must convert to a size_t + * vector, and we also subtract by one (since Julia uses 1-indexed labels). */ void SetParamUCol(void* params, const char* paramName, - size_t* memptr, + long long* memptr, const size_t rows); /** @@ -193,7 +202,7 @@ size_t GetParamVectorIntLen(void* params, const char* paramName); * The vector will be created in-place and it is expected that the calling * function will take ownership. */ -int* GetParamVectorIntPtr(void* params, const char* paramName); +long long* GetParamVectorIntPtr(void* params, const char* paramName); /** * Get the number of rows in a matrix parameter. diff --git a/src/mlpack/bindings/julia/mlpack/params.jl.in b/src/mlpack/bindings/julia/mlpack/params.jl.in index bf3309c1a8..53c490d5ba 100644 --- a/src/mlpack/bindings/julia/mlpack/params.jl.in +++ b/src/mlpack/bindings/julia/mlpack/params.jl.in @@ -103,7 +103,9 @@ end function SetParamMat(params::Ptr{Nothing}, paramName::String, paramValue, - pointsAsRows::Bool) + pointsAsRows::Bool, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(paramValue))) paramMat = to_matrix(paramValue, Float64) ccall((:SetParamMat, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Float64}, Csize_t, Csize_t, Bool), params, paramName, Base.pointer(paramMat), @@ -113,7 +115,9 @@ end function SetParamUMat(params::Ptr{Nothing}, paramName::String, paramValue, - pointsAsRows::Bool) + pointsAsRows::Bool, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(paramValue))) paramMat = to_matrix(paramValue, Int) # Sanity check. @@ -122,10 +126,11 @@ function SetParamUMat(params::Ptr{Nothing}, "Must be 1 or greater.")) end - m = convert(Array{Csize_t, 2}, paramMat .- 1) - ccall((:SetParamUMat, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Csize_t}, - Csize_t, Csize_t, Bool), params, paramName, Base.pointer(m), - size(paramValue, 1), size(paramValue, 2), pointsAsRows) + # Conversion (and subtracting 1 from the labels) happens in :SetParamUMat. + ccall((:SetParamUMat, library), Nothing, (Ptr{Nothing}, Cstring, + Ptr{Clonglong}, Csize_t, Csize_t, Bool), params, paramName, + Base.pointer(paramMat), size(paramValue, 1), size(paramValue, 2), + pointsAsRows) end function SetParam(params::Ptr{Nothing}, @@ -146,16 +151,18 @@ end function SetParam(params::Ptr{Nothing}, paramName::String, vector::Vector{Int}) - cint_vec = convert(Vector{Cint}, vector) + cintVec = convert(Vector{Clonglong}, vector) ccall((:SetParamVectorInt, library), Nothing, (Ptr{Nothing}, Cstring, - Ptr{Cint}, Csize_t), params, paramName, Base.pointer(cint_vec), - size(cint_vec, 1)) + Ptr{Clonglong}, Csize_t), params, paramName, Base.pointer(cintVec), + size(cintVec, 1)) end function SetParam(params::Ptr{Nothing}, paramName::String, matWithInfo::Tuple{Array{Bool, 1}, Array{Float64, 2}}, - pointsAsRows::Bool) + pointsAsRows::Bool, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(matWithInfo[2]))) ccall((:SetParamMatWithInfo, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Bool}, Ptr{Float64}, Int, Int, Bool), params, paramName, Base.pointer(matWithInfo[1]), Base.pointer(matWithInfo[2]), @@ -164,7 +171,9 @@ end function SetParamRow(params::Ptr{Nothing}, paramName::String, - paramValue) + paramValue, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(paramValue))) paramVec = to_vector(paramValue, Float64) ccall((:SetParamRow, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Float64}, Csize_t), params, paramName, Base.pointer(paramVec), size(paramVec, 1)) @@ -172,7 +181,9 @@ end function SetParamCol(params::Ptr{Nothing}, paramName::String, - paramValue) + paramValue, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(paramValue))) paramVec = to_vector(paramValue, Float64) ccall((:SetParamCol, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Float64}, Csize_t), params, paramName, Base.pointer(paramVec), size(paramVec, 1)) @@ -180,7 +191,9 @@ end function SetParamURow(params::Ptr{Nothing}, paramName::String, - paramValue) + paramValue, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(paramValue))) paramVec = to_vector(paramValue, Int) # Sanity check. @@ -188,15 +201,18 @@ function SetParamURow(params::Ptr{Nothing}, throw(DomainError("Input $(paramName) cannot have 0 or negative values! " * "Must be 1 or greater.")) end - m = convert(Array{Csize_t, 1}, paramVec .- 1) - ccall((:SetParamURow, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Csize_t}, - Csize_t), params, paramName, Base.pointer(m), size(paramValue, 1)) + # Conversion (and subtracting 1 from the labels) happens in :SetParamURow. + ccall((:SetParamURow, library), Nothing, (Ptr{Nothing}, Cstring, + Ptr{Clonglong}, Csize_t), params, paramName, Base.pointer(paramVec), + size(paramValue, 1)) end function SetParamUCol(params::Ptr{Nothing}, paramName::String, - paramValue) + paramValue, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(paramValue))) paramVec = to_vector(paramValue, Int) # Sanity check. @@ -204,10 +220,11 @@ function SetParamUCol(params::Ptr{Nothing}, throw(DomainError("Input $(paramName) cannot have 0 or negative values! " * "Must be 1 or greater.")) end - m = convert(Array{Csize_t, 1}, paramValue .- 1) - ccall((:SetParamUCol, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Csize_t}, - Csize_t), params, paramName, Base.pointer(m), size(paramValue, 1)) + # Conversion (and subtracting 1 from the labels) happens in :SetParamUCol. + ccall((:SetParamUCol, library), Nothing, (Ptr{Nothing}, Cstring, + Ptr{Clonglong}, Csize_t), params, paramName, Base.pointer(paramVec), + size(paramValue, 1)) end function GetParamBool(params::Ptr{Nothing}, paramName::String) @@ -238,6 +255,10 @@ function GetParamVectorStr(params::Ptr{Nothing}, paramName::String) size = ccall((:GetParamVectorStrLen, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) out = Array{String, 1}() + if size == 0 + return out + end + for i = 1:size s = ccall((:GetParamVectorStrStr, library), Cstring, (Ptr{Nothing}, Cstring, Csize_t), params, paramName, i .- 1) @@ -249,22 +270,28 @@ end function GetParamVectorInt(params::Ptr{Nothing}, paramName::String) local size::Csize_t - local ptr::Ptr{Cint} + local ptr::Ptr{Clonglong} # Get the size of the vector, then the pointer to it. We will own the # pointer. size = ccall((:GetParamVectorIntLen, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) - ptr = ccall((:GetParamVectorIntPtr, library), Ptr{Cint}, (Ptr{Nothing}, + # Shortcut: if the vector is empty, no need to allocate. + if size == 0 + return Int[] + end + + ptr = ccall((:GetParamVectorIntPtr, library), Ptr{Clonglong}, (Ptr{Nothing}, Cstring,), params, paramName) - return convert(Array{Int, 1}, Base.unsafe_wrap(Array{Cint, 1}, ptr, (size), - own=true)) + return convert(Array{Int, 1}, Base.unsafe_wrap(Array{Clonglong, 1}, ptr, + (size), own=true)) end function GetParamMat(params::Ptr{Nothing}, paramName::String, - pointsAsRows::Bool) + pointsAsRows::Bool, + juliaOwnedMemory::Set{Ptr{Nothing}}) # Can we return different return types? For now let's restrict to a matrix to # make it easy... local ptr::Ptr{Float64} @@ -275,22 +302,32 @@ function GetParamMat(params::Ptr{Nothing}, params, paramName) cols = ccall((:GetParamMatCols, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the matrix is empty, return an empty matrix. + if rows == 0 && cols == 0 + return Array{Float64, 2}(undef, 0, 0) + end + ptr = ccall((:GetParamMat, library), Ptr{Float64}, (Ptr{Nothing}, Cstring,), params, paramName) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptr) in juliaOwnedMemory) + if pointsAsRows # In this case we have to transpose, unfortunately. - m = Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), own=true) + m = Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), own=own) return m' else # Here no transpose is necessary. - return Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), own=true) + return Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), own=own) end end function GetParamUMat(params::Ptr{Nothing}, paramName::String, - pointsAsRows::Bool) + pointsAsRows::Bool, + juliaOwnedMemory::Set{Ptr{Nothing}}) # Can we return different return types? For now let's restrict to a matrix to # make it easy... local ptr::Ptr{Csize_t} @@ -301,73 +338,127 @@ function GetParamUMat(params::Ptr{Nothing}, params, paramName) cols = ccall((:GetParamUMatCols, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the matrix is empty, return an empty matrix. + if rows == 0 && cols == 0 + return Array{Int, 2}(undef, 0, 0) + end + ptr = ccall((:GetParamUMat, library), Ptr{Csize_t}, (Ptr{Nothing}, Cstring,), params, paramName) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptr) in juliaOwnedMemory) + if pointsAsRows # In this case we have to transpose, unfortunately. - m = Base.unsafe_wrap(Array{Csize_t, 2}, ptr, (rows, cols), own=true) + m = Base.unsafe_wrap(Array{Csize_t, 2}, ptr, (rows, cols), own=own) return convert(Array{Int, 2}, m' .+ 1) # Add 1 because these are indexes. else # Here no transpose is necessary. - m = Base.unsafe_wrap(Array{Csize_t, 2}, ptr, (rows, cols), own=true) + m = Base.unsafe_wrap(Array{Csize_t, 2}, ptr, (rows, cols), own=own) return convert(Array{Int, 2}, m .+ 1) end end -function GetParamCol(params::Ptr{Nothing}, paramName::String) +function GetParamCol(params::Ptr{Nothing}, + paramName::String, + juliaOwnedMemory::Set{Ptr{Nothing}}) local ptr::Ptr{Float64} local rows::Csize_t rows = ccall((:GetParamColRows, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the vector is empty, return an empty matrix. + if rows == 0 + return Array{Float64, 1}() + end + ptr = ccall((:GetParamCol, library), Ptr{Float64}, (Ptr{Nothing}, Cstring,), params, paramName) - return Base.unsafe_wrap(Array{Float64, 1}, ptr, rows, own=true) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptr) in juliaOwnedMemory) + + return Base.unsafe_wrap(Array{Float64, 1}, ptr, rows, own=own) end -function GetParamRow(params::Ptr{Nothing}, paramName::String) +function GetParamRow(params::Ptr{Nothing}, + paramName::String, + juliaOwnedMemory::Set{Ptr{Nothing}}) local ptr::Ptr{Float64} local cols::Csize_t cols = ccall((:GetParamRowCols, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the vector is empty, return an empty matrix. + if cols == 0 + return Array{Float64, 1}(undef, 0) + end + ptr = ccall((:GetParamRow, library), Ptr{Float64}, (Ptr{Nothing}, Cstring,), params, paramName) - return Base.unsafe_wrap(Array{Float64, 1}, ptr, cols, own=true) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptr) in juliaOwnedMemory) + + return Base.unsafe_wrap(Array{Float64, 1}, ptr, cols, own=own) end -function GetParamUCol(params::Ptr{Nothing}, paramName::String) +function GetParamUCol(params::Ptr{Nothing}, + paramName::String, + juliaOwnedMemory::Set{Ptr{Nothing}}) local ptr::Ptr{Csize_t} local rows::Csize_t rows = ccall((:GetParamUColRows, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the vector is empty, return an empty matrix. + if rows == 0 + return Array{Int, 1}() + end + ptr = ccall((:GetParamUCol, library), Ptr{Csize_t}, (Ptr{Nothing}, Cstring,), params, paramName) - m = Base.unsafe_wrap(Array{Csize_t, 1}, ptr, rows, own=true) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptr) in juliaOwnedMemory) + + m = Base.unsafe_wrap(Array{Csize_t, 1}, ptr, rows, own=own) return convert(Array{Int, 1}, m .+ 1) end -function GetParamURow(params::Ptr{Nothing}, paramName::String) +function GetParamURow(params::Ptr{Nothing}, + paramName::String, + juliaOwnedMemory::Set{Ptr{Nothing}}) local ptr::Ptr{Csize_t} local cols::Csize_t cols = ccall((:GetParamURowCols, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the vector is empty, return an empty matrix. + if cols == 0 + return Array{Int, 1}(undef, 0) + end + ptr = ccall((:GetParamURow, library), Ptr{Csize_t}, (Ptr{Nothing}, Cstring,), params, paramName) - m = Base.unsafe_wrap(Array{Csize_t, 1}, ptr, cols, own=true) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptr) in juliaOwnedMemory) + + m = Base.unsafe_wrap(Array{Csize_t, 1}, ptr, cols, own=own) return convert(Array{Int, 1}, m .+ 1) end function GetParamMatWithInfo(params::Ptr{Nothing}, paramName::String, - pointsAsRows::Bool) + pointsAsRows::Bool, + juliaOwnedMemory::Set{Ptr{Nothing}}) local ptrBool::Ptr{Bool} local ptrData::Ptr{Float64} local rows::Csize_t @@ -377,20 +468,29 @@ function GetParamMatWithInfo(params::Ptr{Nothing}, Cstring,), params, paramName) cols = ccall((:GetParamMatWithInfoCols, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the vector is empty, return an empty matrix. + if rows == 0 && cols == 0 + return (Array{Bool, 1}(undef, 0), Array{Float64, 2}(undef, 0, 0)) + end + ptrBool = ccall((:GetParamMatWithInfoBoolPtr, library), Ptr{Bool}, (Ptr{Nothing}, Cstring,), params, paramName) ptrMem = ccall((:GetParamMatWithInfoPtr, library), Ptr{Float64}, (Ptr{Nothing}, Cstring,), params, paramName) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptrMem) in juliaOwnedMemory) + types = Base.unsafe_wrap(Array{Bool, 1}, ptrBool, (rows), own=true) if pointsAsRows # In this case we have to transpose, unfortunately. - m = Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), own=true) + m = Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), own=own) return (types, m') else # Here no transpose is necessary. return (types, Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), - own=true)) + own=own)) end end diff --git a/src/mlpack/bindings/julia/print_input_processing_impl.hpp b/src/mlpack/bindings/julia/print_input_processing_impl.hpp index 06969d0847..c605d7c727 100644 --- a/src/mlpack/bindings/julia/print_input_processing_impl.hpp +++ b/src/mlpack/bindings/julia/print_input_processing_impl.hpp @@ -104,7 +104,8 @@ void PrintInputProcessing( // Now print the SetParam call. std::cout << indent << "SetParam" << uChar << matTypeModifier << "(p, \"" - << d.name << "\", " << juliaName << extra << ")" << std::endl; + << d.name << "\", " << juliaName << extra << ", juliaOwnedMemory)" + << std::endl; if (!d.required) { @@ -180,8 +181,8 @@ void PrintInputProcessing( // // SetParam(p, "", convert(, )) std::cout << " SetParam(p, \"" << d.name << "\", convert(" - << GetJuliaType(d) << ", " << juliaName << "), points_are_rows)" - << std::endl; + << GetJuliaType(d) << ", " << juliaName << "), points_are_rows, " + << "juliaOwnedMemory)" << std::endl; } else { @@ -192,8 +193,8 @@ void PrintInputProcessing( // end std::cout << " if !ismissing(" << juliaName << ")" << std::endl; std::cout << " SetParam(p, \"" << d.name << "\", convert(" - << GetJuliaType(d) << ", " << juliaName << "), points_are_rows)" - << std::endl; + << GetJuliaType(d) << ", " << juliaName << "), points_are_rows, " + << "juliaOwnedMemory)" << std::endl; std::cout << " end" << std::endl; } } diff --git a/src/mlpack/bindings/julia/print_jl.cpp b/src/mlpack/bindings/julia/print_jl.cpp index 276c74090f..292b0850a5 100644 --- a/src/mlpack/bindings/julia/print_jl.cpp +++ b/src/mlpack/bindings/julia/print_jl.cpp @@ -257,6 +257,12 @@ void PrintJL(const string& bindingName, cout << " t = Timers()" << endl; cout << endl; + // Create a set where we will store the pointers associated with all memory + // that Julia owns. This is to prevent situations where we end up wrapping a + // result object and telling Julia to own that too---in that case, the GC will + // free the object twice! + cout << " juliaOwnedMemory = Set{Ptr{Nothing}}()" << endl; + // Handle each input argument's processing before calling mlpackMain(). cout << " # Process each input argument before calling mlpackMain()." << endl; diff --git a/src/mlpack/bindings/julia/print_output_processing_impl.hpp b/src/mlpack/bindings/julia/print_output_processing_impl.hpp index 1a3b7c0b9d..b3de0f688a 100644 --- a/src/mlpack/bindings/julia/print_output_processing_impl.hpp +++ b/src/mlpack/bindings/julia/print_output_processing_impl.hpp @@ -90,7 +90,7 @@ void PrintOutputProcessing( } std::cout << "GetParam" << uChar << matTypeSuffix << "(p, \"" << d.name - << "\"" << extra << ")"; + << "\"" << extra << ", juliaOwnedMemory)"; } /** @@ -120,7 +120,7 @@ void PrintOutputProcessing( const typename std::enable_if>::value>::type*) { - std::cout << "GetParamMatWithInfo(p, \"" << d.name << "\")"; + std::cout << "GetParamMatWithInfo(p, \"" << d.name << "\", juliaOwnedMemory)"; } } // namespace julia diff --git a/src/mlpack/bindings/julia/tests/runtests.jl b/src/mlpack/bindings/julia/tests/runtests.jl index 38ffef8ef5..fa95bfc7a5 100644 --- a/src/mlpack/bindings/julia/tests/runtests.jl +++ b/src/mlpack/bindings/julia/tests/runtests.jl @@ -216,6 +216,7 @@ end # Test a row vector input parameter. @testset "TestRow" begin x = rand(100) + oldX = copy(x) _, _, _, _, _, _, _, rowOut, _, _, _, _, _, _ = test_julia_binding(4.0, 12, "hello", @@ -224,7 +225,7 @@ end @test size(rowOut, 1) == 100 @test typeof(rowOut) == Array{Float64, 1} for i in 1:100 - @test rowOut[i] == 2 * x[i] + @test rowOut[i] == 2 * oldX[i] end end @@ -250,7 +251,7 @@ end x = rand(Float64, (10, 100)) # Dimension information. dims = [false, false, false, false, false, false, false, false, false, false] - z = x + z = copy(x) _, _, _, matrix_and_info_out, _, _, _, _, _, _, _, _, _, _ = test_julia_binding(4.0, 12, "hello", @@ -262,7 +263,7 @@ end for i in 1:100 for j in 1:10 - @test matrix_and_info_out[j, i] == 2.0 * z[j, i] + @test matrix_and_info_out[j, i] == 2.0 * x[j, i] end end end @@ -272,7 +273,7 @@ end x = rand(Float64, (100, 10)) # Dimension information. dims = [false, false, false, false, false, false, false, false, false, false] - z = x + z = copy(x) _, _, _, matrix_and_info_out, _, _, _, _, _, _, _, _, _, _ = test_julia_binding(4.0, 12, "hello", @@ -284,7 +285,7 @@ end for i in 1:100 for j in 1:10 - @test matrix_and_info_out[i, j] == 2.0 * z[i, j] + @test matrix_and_info_out[i, j] == 2.0 * x[i, j] end end end @@ -298,7 +299,7 @@ end rand(1:6, 100), rand(100))') dims = [false, true, false, true, true, false] - z = x + z = copy(x) _, _, _, matrix_and_info_out, _, _, _, _, _, _, _, _, _, _ = test_julia_binding(4.0, 12, "hello", @@ -310,10 +311,10 @@ end for i in 1:100 for j in [1, 3, 6] - @test matrix_and_info_out[j, i] == 2.0 * z[j, i] + @test matrix_and_info_out[j, i] == 2.0 * x[j, i] end for j in [2, 4, 5] - @test matrix_and_info_out[j, i] == z[j, i] + @test matrix_and_info_out[j, i] == x[j, i] end end end @@ -327,7 +328,7 @@ end rand(1:6, 100), rand(100)) dims = [false, true, false, true, true, false] - z = x + z = copy(x) _, _, _, matrix_and_info_out, _, _, _, _, _, _, _, _, _, _ = test_julia_binding(4.0, 12, "hello", @@ -339,10 +340,10 @@ end for i in 1:100 for j in [1, 3, 6] - @test matrix_and_info_out[i, j] == 2.0 * z[i, j] + @test matrix_and_info_out[i, j] == 2.0 * x[i, j] end for j in [2, 4, 5] - @test matrix_and_info_out[i, j] == z[i, j] + @test matrix_and_info_out[i, j] == x[i, j] end end end From c62429fb9c9c2cd9b9fd019c48ddc21e68d54c39 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Wed, 28 Sep 2022 18:38:47 -0400 Subject: [PATCH 11/11] Apply @kpamnany's suggestions for GC.@preserve. --- src/mlpack/bindings/julia/print_param_defn.hpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/mlpack/bindings/julia/print_param_defn.hpp b/src/mlpack/bindings/julia/print_param_defn.hpp index af5145b2f6..df862c2aac 100644 --- a/src/mlpack/bindings/julia/print_param_defn.hpp +++ b/src/mlpack/bindings/julia/print_param_defn.hpp @@ -82,7 +82,7 @@ void PrintParamDefn( // buf_len = UInt[0] // buffer = ccall((:SerializePtr, Library), // Vector{UInt8}, (Ptr{Nothing}, Ptr{UInt8}), model.ptr, - // Base.pointer(buf_len)) + // pointer(buf_len)) // buf = Base.unsafe_wrap(buf_ptr, buf_len[1]; own=true) // write(stream, buf_len[1]) // write(stream, buf) @@ -91,8 +91,9 @@ void PrintParamDefn( // function deserialize(stream::IO):: // buf_len = read(stream, UInt) // buffer = read(stream, buf_len) - // (ccall((:DeserializePtr, Library), - // Ptr{Nothing}, (Vector{UInt8}, UInt), buffer, length(buffer))) + // GC.@preserve buffer (ccall((:DeserializePtr, + // Library), Ptr{Nothing}, (Vector{UInt8}, UInt), buffer, + // length(buffer))) // end std::string type = util::StripType(d.cppType); @@ -142,7 +143,7 @@ void PrintParamDefn( std::cout << " buf_len = UInt[0]" << std::endl; std::cout << " buf_ptr = ccall((:Serialize" << type << "Ptr, " << programName << "Library), Ptr{UInt8}, (Ptr{Nothing}, Ptr{UInt}), model.ptr, " - << "Base.pointer(buf_len))" << std::endl; + << "pointer(buf_len))" << std::endl; std::cout << " buf = Base.unsafe_wrap(Vector{UInt8}, buf_ptr, buf_len[1]; " << "own=true)" << std::endl; std::cout << " write(stream, buf_len[1])" << std::endl; @@ -155,9 +156,9 @@ void PrintParamDefn( << std::endl; std::cout << " buf_len = read(stream, UInt)" << std::endl; std::cout << " buffer = read(stream, buf_len)" << std::endl; - std::cout << " " << type << "(ccall((:Deserialize" << type << "Ptr, " - << programName << "Library), Ptr{Nothing}, (Ptr{UInt8}, UInt), " - << "Base.pointer(buffer), length(buffer)))" << std::endl; + std::cout << " GC.@preserve buffer " << type << "(ccall((:Deserialize" + << type << "Ptr, " << programName << "Library), Ptr{Nothing}, " + << "(Ptr{UInt8}, UInt), pointer(buffer), length(buffer)))" << std::endl; std::cout << "end" << std::endl; }