Repository: libKriging/libKriging (language: python)

Issue:
remove "normalize" arg from update()
should use normalize according initial setup


Discussion / hints:

Fix patch:
diff --git a/bindings/Octave/Kriging_binding.cpp b/bindings/Octave/Kriging_binding.cpp
index d7167716..aa18cbf3 100644
--- a/bindings/Octave/Kriging_binding.cpp
+++ b/bindings/Octave/Kriging_binding.cpp
@@ -123,10 +123,10 @@ void update(int nlhs, mxArray** plhs, int nrhs, const mxArray** prhs) {
   MxMapper input{"Input",
                  nrhs,
                  const_cast<mxArray**>(prhs),  // NOLINT(cppcoreguidelines-pro-type-const-cast)
-                 RequiresArg::Exactly{4}};
+                 RequiresArg::Exactly{3}};
   MxMapper output{"Output", nlhs, plhs, RequiresArg::Exactly{0}};
   auto* km = input.getObject<0, Kriging>("Kriging reference");
-  km->update(input.get<1, arma::vec>("new y"), input.get<2, arma::mat>("new X"), input.get<3, bool>("normalize"));
+  km->update(input.get<1, arma::vec>("new y"), input.get<2, arma::mat>("new X"));
 }
 
 void summary(int nlhs, mxArray** plhs, int nrhs, const mxArray** prhs) {
diff --git a/bindings/Octave/NuggetKriging_binding.cpp b/bindings/Octave/NuggetKriging_binding.cpp
index 714ed95e..ff0afc30 100644
--- a/bindings/Octave/NuggetKriging_binding.cpp
+++ b/bindings/Octave/NuggetKriging_binding.cpp
@@ -125,10 +125,10 @@ void update(int nlhs, mxArray** plhs, int nrhs, const mxArray** prhs) {
   MxMapper input{"Input",
                  nrhs,
                  const_cast<mxArray**>(prhs),  // NOLINT(cppcoreguidelines-pro-type-const-cast)
-                 RequiresArg::Exactly{4}};
+                 RequiresArg::Exactly{3}};
   MxMapper output{"Output", nlhs, plhs, RequiresArg::Exactly{0}};
   auto* km = input.getObject<0, NuggetKriging>("NuggetKriging reference");
-  km->update(input.get<1, arma::vec>("new y"), input.get<2, arma::mat>("new X"), input.get<3, bool>("normalize"));
+  km->update(input.get<1, arma::vec>("new y"), input.get<2, arma::mat>("new X"));
 }
 
 void summary(int nlhs, mxArray** plhs, int nrhs, const mxArray** prhs) {
diff --git a/bindings/Python/src/_pylibkriging/Kriging_binding.cpp b/bindings/Python/src/_pylibkriging/Kriging_binding.cpp
index b9ae2cd1..24720117 100644
--- a/bindings/Python/src/_pylibkriging/Kriging_binding.cpp
+++ b/bindings/Python/src/_pylibkriging/Kriging_binding.cpp
@@ -52,10 +52,10 @@ py::array_t<double> PyKriging::simulate(const int nsim, const int seed, const py
   return carma::mat_to_arr(result, true);
 }
 
-void PyKriging::update(const py::array_t<double>& newy, const py::array_t<double>& newX, bool normalize) {
+void PyKriging::update(const py::array_t<double>& newy, const py::array_t<double>& newX) {
   arma::mat mat_y = carma::arr_to_col<double>(newy);
   arma::mat mat_X = carma::arr_to_mat<double>(newX);
-  m_internal->update(mat_y, mat_X, normalize);
+  m_internal->update(mat_y, mat_X);
 }
 
 std::string PyKriging::summary() const {
diff --git a/bindings/Python/src/_pylibkriging/Kriging_binding.hpp b/bindings/Python/src/_pylibkriging/Kriging_binding.hpp
index 863cf377..65bce42f 100644
--- a/bindings/Python/src/_pylibkriging/Kriging_binding.hpp
+++ b/bindings/Python/src/_pylibkriging/Kriging_binding.hpp
@@ -43,7 +43,7 @@ class PyKriging {
 
   py::array_t<double> simulate(const int nsim, const int seed, const py::array_t<double>& Xp);
 
-  void update(const py::array_t<double>& newy, const py::array_t<double>& newX, bool normalize);
+  void update(const py::array_t<double>& newy, const py::array_t<double>& newX);
 
   std::string summary() const;
 
diff --git a/bindings/Python/src/_pylibkriging/NuggetKriging_binding.cpp b/bindings/Python/src/_pylibkriging/NuggetKriging_binding.cpp
index ab3288b6..6aecdfa6 100644
--- a/bindings/Python/src/_pylibkriging/NuggetKriging_binding.cpp
+++ b/bindings/Python/src/_pylibkriging/NuggetKriging_binding.cpp
@@ -53,10 +53,10 @@ py::array_t<double> PyNuggetKriging::simulate(const int nsim, const int seed, co
   return carma::mat_to_arr(result, true);
 }
 
-void PyNuggetKriging::update(const py::array_t<double>& newy, const py::array_t<double>& newX, bool normalize) {
+void PyNuggetKriging::update(const py::array_t<double>& newy, const py::array_t<double>& newX) {
   arma::mat mat_y = carma::arr_to_col<double>(newy);
   arma::mat mat_X = carma::arr_to_mat<double>(newX);
-  m_internal->update(mat_y, mat_X, normalize);
+  m_internal->update(mat_y, mat_X);
 }
 
 std::string PyNuggetKriging::summary() const {
diff --git a/bindings/Python/src/_pylibkriging/NuggetKriging_binding.hpp b/bindings/Python/src/_pylibkriging/NuggetKriging_binding.hpp
index 82654997..08850bc0 100644
--- a/bindings/Python/src/_pylibkriging/NuggetKriging_binding.hpp
+++ b/bindings/Python/src/_pylibkriging/NuggetKriging_binding.hpp
@@ -43,7 +43,7 @@ class PyNuggetKriging {
 
   py::array_t<double> simulate(const int nsim, const int seed, const py::array_t<double>& Xp);
 
-  void update(const py::array_t<double>& newy, const py::array_t<double>& newX, bool normalize);
+  void update(const py::array_t<double>& newy, const py::array_t<double>& newX);
 
   std::string summary() const;
 
diff --git a/bindings/R/rlibkriging/R/KrigingClass.R b/bindings/R/rlibkriging/R/KrigingClass.R
index 4dff1f8f..97b38bb2 100644
--- a/bindings/R/rlibkriging/R/KrigingClass.R
+++ b/bindings/R/rlibkriging/R/KrigingClass.R
@@ -416,9 +416,6 @@ simulate.Kriging <- function(object, nsim = 1, seed = 123, x,  ...) {
 #'
 #' @param newX Numeric matrix of new input points.
 #' 
-#' @param normalize Logical. If \code{TRUE}, the input \code{X} and
-#'     the output \code{y} are normalized to lie in \eqn{[0, 1]}.
-#'
 #' @param ... Ignored.
 #' 
 #' @section Caution: The method \emph{does not return the updated
@@ -457,7 +454,7 @@ simulate.Kriging <- function(object, nsim = 1, seed = 123, x,  ...) {
 #' lines(x, p2_x$mean - 2 * p2_x$stdev, col = "red")
 #' lines(x, p2_x$mean + 2 * p2_x$stdev, col = "red")
 #' 
-update.Kriging <- function(object, newy, newX, normalize = FALSE, ...) {
+update.Kriging <- function(object, newy, newX, ...) {
     
     if (length(L <- list(...)) > 0) warnOnDots(L)
     k <- kriging_model(object) 
@@ -470,7 +467,7 @@ update.Kriging <- function(object, newy, newX, normalize = FALSE, ...) {
         stop("Objects 'newX' and 'newy' must have the same number of rows.")
 
     ## Modify 'object' in the parent environment
-    kriging_update(object, newy, newX, normalize = normalize)
+    kriging_update(object, newy, newX)
     
     invisible(NULL)
     
diff --git a/bindings/R/rlibkriging/R/NuggetKrigingClass.R b/bindings/R/rlibkriging/R/NuggetKrigingClass.R
index 9a79e58a..caec329a 100644
--- a/bindings/R/rlibkriging/R/NuggetKrigingClass.R
+++ b/bindings/R/rlibkriging/R/NuggetKrigingClass.R
@@ -416,9 +416,6 @@ simulate.NuggetKriging <- function(object, nsim = 1, seed = 123, x,  ...) {
 #'
 #' @param newX Numeric matrix of new input points.
 #' 
-#' @param normalize Logical. If \code{TRUE}, the input \code{X} and
-#'     the output \code{y} are normalized to lie in \eqn{[0, 1]}.
-#'
 #' @param ... Ignored.
 #' 
 #' @section Caution: The method \emph{does not return the updated
@@ -457,7 +454,7 @@ simulate.NuggetKriging <- function(object, nsim = 1, seed = 123, x,  ...) {
 #' lines(x, p2_x$mean - 2 * p2_x$stdev, col = "red")
 #' lines(x, p2_x$mean + 2 * p2_x$stdev, col = "red")
 #' 
-update.NuggetKriging <- function(object, newy, newX, normalize = FALSE, ...) {
+update.NuggetKriging <- function(object, newy, newX, ...) {
     
     if (length(L <- list(...)) > 0) warnOnDots(L)
     k <- nuggetkriging_model(object) 
@@ -470,7 +467,7 @@ update.NuggetKriging <- function(object, newy, newX, normalize = FALSE, ...) {
         stop("Objects 'newX' and 'newy' must have the same number of rows.")
 
     ## Modify 'object' in the parent environment
-    nuggetkriging_update(object, newy, newX, normalize = normalize)
+    nuggetkriging_update(object, newy, newX)
     
     invisible(NULL)
     
diff --git a/bindings/R/rlibkriging/src/kriging_binding.cpp b/bindings/R/rlibkriging/src/kriging_binding.cpp
index 01b6c32a..a9841147 100644
--- a/bindings/R/rlibkriging/src/kriging_binding.cpp
+++ b/bindings/R/rlibkriging/src/kriging_binding.cpp
@@ -182,14 +182,14 @@ arma::mat kriging_simulate(Rcpp::List k, int nsim, int seed, arma::mat X) {
 }
 
 // [[Rcpp::export]]
-void kriging_update(Rcpp::List k, arma::vec y, arma::mat X, bool normalize = false) {
+void kriging_update(Rcpp::List k, arma::vec y, arma::mat X) {
   if (!k.inherits("Kriging"))
     Rcpp::stop("Input must be a Kriging object.");
   SEXP impl = k.attr("object");
 
   Rcpp::XPtr<Kriging> impl_ptr(impl);
 
-  impl_ptr->update(y, X, normalize);
+  impl_ptr->update(y, X);
 
   // Rcpp::List obj;
   // obj.attr("object") = impl_ptr;
diff --git a/bindings/R/rlibkriging/src/nuggetkriging_binding.cpp b/bindings/R/rlibkriging/src/nuggetkriging_binding.cpp
index a5cd5623..fd67ccad 100644
--- a/bindings/R/rlibkriging/src/nuggetkriging_binding.cpp
+++ b/bindings/R/rlibkriging/src/nuggetkriging_binding.cpp
@@ -204,14 +204,14 @@ arma::mat nuggetkriging_simulate(Rcpp::List k, int nsim, int seed, arma::mat X)
 }
 
 // [[Rcpp::export]]
-void nuggetkriging_update(Rcpp::List k, arma::vec y, arma::mat X, bool normalize = false) {
+void nuggetkriging_update(Rcpp::List k, arma::vec y, arma::mat X) {
   if (!k.inherits("NuggetKriging"))
     Rcpp::stop("Input must be a NuggetKriging object.");
   SEXP impl = k.attr("object");
 
   Rcpp::XPtr<NuggetKriging> impl_ptr(impl);
 
-  impl_ptr->update(y, X, normalize);
+  impl_ptr->update(y, X);
 
   // Rcpp::List obj;
   // obj.attr("object") = impl_ptr;
diff --git a/src/lib/Kriging.cpp b/src/lib/Kriging.cpp
index 43d39a9a..823f0a16 100644
--- a/src/lib/Kriging.cpp
+++ b/src/lib/Kriging.cpp
@@ -1296,7 +1296,7 @@ LIBKRIGING_EXPORT arma::mat Kriging::simulate(const int nsim, const int seed, co
  * @param optim_method is an optimizer name from OptimLib, or 'none' to keep previously estimated parameters unchanged
  * @param optim_objective is 'loo' or 'loglik'. Ignored if optim_method=='none'.
  */
-LIBKRIGING_EXPORT void Kriging::update(const arma::vec& newy, const arma::mat& newX, bool normalize = false) {
+LIBKRIGING_EXPORT void Kriging::update(const arma::vec& newy, const arma::mat& newX) {
   // rebuild starting parameters
   Parameters parameters{this->m_sigma2,
                         true,
@@ -1309,6 +1309,7 @@ LIBKRIGING_EXPORT void Kriging::update(const arma::vec& newy, const arma::mat& n
                         this->m_est_beta};
   // re-fit
   // TODO refit() method which will use Shurr forms to fast update matrix (R, ...)
+  bool normalize = (m_centerY == 0.0 && m_scaleY == 1) && (m_centerX.is_zero() && m_scaleX.is_zero());
   this->fit(
       arma::join_cols(m_y, newy), arma::join_cols(m_X, newX), m_regmodel, normalize, m_optim, m_objective, parameters);
 }
diff --git a/src/lib/NuggetKriging.cpp b/src/lib/NuggetKriging.cpp
index 64b6ab7b..fa0fc663 100644
--- a/src/lib/NuggetKriging.cpp
+++ b/src/lib/NuggetKriging.cpp
@@ -959,7 +959,7 @@ LIBKRIGING_EXPORT arma::mat NuggetKriging::simulate(const int nsim, const int se
  * @param optim_method is an optimizer name from OptimLib, or 'none' to keep previously estimated parameters unchanged
  * @param optim_objective is 'loo' or 'loglik'. Ignored if optim_method=='none'.
  */
-LIBKRIGING_EXPORT void NuggetKriging::update(const arma::vec& newy, const arma::mat& newX, bool normalize = false) {
+LIBKRIGING_EXPORT void NuggetKriging::update(const arma::vec& newy, const arma::mat& newX) {
   // rebuild starting parameters
   Parameters parameters{arma::vec(1, arma::fill::value(this->m_nugget)),
                         true,
@@ -975,6 +975,7 @@ LIBKRIGING_EXPORT void NuggetKriging::update(const arma::vec& newy, const arma::
                         this->m_est_beta};
   // re-fit
   // TODO refit() method which will use Shurr forms to fast update matrix (R, ...)
+  bool normalize = (m_centerY == 0.0 && m_scaleY == 1) && (m_centerX.is_zero() && m_scaleX.is_zero());
   this->fit(
       arma::join_cols(m_y, newy), arma::join_cols(m_X, newX), m_regmodel, normalize, m_optim, m_objective, parameters);
 }
diff --git a/src/lib/include/libKriging/Kriging.hpp b/src/lib/include/libKriging/Kriging.hpp
index 9421d297..dc38084f 100644
--- a/src/lib/include/libKriging/Kriging.hpp
+++ b/src/lib/include/libKriging/Kriging.hpp
@@ -177,10 +177,8 @@ class Kriging {
   /** Add new conditional data points to previous (X,y)
    * @param newy is m length column vector of new output
    * @param newX is m*d matrix of new input
-   * @param optim_method is an optimizer name from OptimLib, or 'none' to keep previously estimated parameters unchanged
-   * @param optim_objective is 'loo' or 'loglik'. Ignored if optim_method=='none'.
    */
-  LIBKRIGING_EXPORT void update(const arma::vec& newy, const arma::mat& newX, bool normalize);
+  LIBKRIGING_EXPORT void update(const arma::vec& newy, const arma::mat& newX);
 
   LIBKRIGING_EXPORT std::string summary() const;
 };
diff --git a/src/lib/include/libKriging/NuggetKriging.hpp b/src/lib/include/libKriging/NuggetKriging.hpp
index 786726d9..1d0c640f 100644
--- a/src/lib/include/libKriging/NuggetKriging.hpp
+++ b/src/lib/include/libKriging/NuggetKriging.hpp
@@ -194,10 +194,8 @@ class NuggetKriging {
   /** Add new conditional data points to previous (X,y)
    * @param newy is m length column vector of new output
    * @param newX is m*d matrix of new input
-   * @param optim_method is an optimizer name from OptimLib, or 'none' to keep previously estimated parameters unchanged
-   * @param optim_objective is 'loo' or 'loglik'. Ignored if optim_method=='none'.
    */
-  LIBKRIGING_EXPORT void update(const arma::vec& newy, const arma::mat& newX, bool normalize);
+  LIBKRIGING_EXPORT void update(const arma::vec& newy, const arma::mat& newX);
 
   LIBKRIGING_EXPORT std::string summary() const;
 };


Test patch:
diff --git a/bindings/Octave/tests/mLibKriging_demo.m b/bindings/Octave/tests/mLibKriging_demo.m
index b8a2adee..dc3f21c2 100644
--- a/bindings/Octave/tests/mLibKriging_demo.m
+++ b/bindings/Octave/tests/mLibKriging_demo.m
@@ -60,5 +60,5 @@
 Xn = [0.3;0.4];
 yn = f(Xn);
 disp(k_m.summary());
-k_m.update(yn, Xn, false)
+k_m.update(yn, Xn)
 disp(k_m.summary());
diff --git a/bindings/Python/tests/pylibkriging_demo.py b/bindings/Python/tests/pylibkriging_demo.py
index 9a32bce6..5fd5c1f9 100644
--- a/bindings/Python/tests/pylibkriging_demo.py
+++ b/bindings/Python/tests/pylibkriging_demo.py
@@ -46,7 +46,7 @@ def test_kriging():
     Xn = np.array([0.3, 0.4])
     yn = [f(xi) for xi in Xn]
     print(k_py.summary())
-    k_py.update(yn, Xn, False)
+    k_py.update(yn, Xn)
     print(k_py.summary())
 
 
diff --git a/bindings/R/rlibkriging/tests/testthat/test-rlibkriging-demo.R b/bindings/R/rlibkriging/tests/testthat/test-rlibkriging-demo.R
index a7136e36..a5a0a37b 100644
--- a/bindings/R/rlibkriging/tests/testthat/test-rlibkriging-demo.R
+++ b/bindings/R/rlibkriging/tests/testthat/test-rlibkriging-demo.R
@@ -28,5 +28,5 @@ matplot(x,s,col=rgb(0,0,1,0.2),type='l',lty=1,add=T)
 Xn <- as.matrix(c(0.3,0.4))
 yn <- f(Xn)
 print(k_R)
-update(k_R, yn, Xn, FALSE)
+update(k_R, yn, Xn)
 print(k_R)
\ No newline at end of file