Merge pull request #35 from fabian-sp/f-rescaling-default

Rescaling by default
fabian-sp · Mar 17, 2022 · 1667d58 · 1667d58
2 parents bd79363 + 63d0f31
commit 1667d58
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 16 deletions.
diff --git a/gglasso/__init__.py b/gglasso/__init__.py
@@ -1,2 +1,2 @@
-__version__ = "0.1.8"
+__version__ = "0.1.9"
 
diff --git a/gglasso/problem.py b/gglasso/problem.py
@@ -251,7 +251,7 @@ def _scale_input_to_correlation(self):
         """
 
         warnings.warn("NOTE: Input data S is rescaled to correlations, this has impact on the scale of the regularization parameters!")
-        warnings.warn("The output/solution is not rescaled automatically, you can rescale by self._rescale_to_covariances(X, self._scale).")
+        warnings.warn("The output/solution is rescaled to covariances. All model selection output, in particular the optimal regularization parameters in self.reg_params are corresponding to the correlations.")
 
         if not self.multiple:
             self._scale = np.diag(self.S)
@@ -424,7 +424,6 @@ def solve(self, Omega_0 = None, solver_params = dict(), tol = 1e-8, rtol = 1e-7,
         self.solver_params.update(solver_params)
         self.solver_params["verbose"] = verbose
 
-        #print(self.solver_params.keys())
         #print(f"\n Solve problem with {solver.upper()} solver... \n ")
 
         if not self.multiple:
@@ -450,13 +449,13 @@ def solve(self, Omega_0 = None, solver_params = dict(), tol = 1e-8, rtol = 1e-7,
                                          latent = self.latent, mu1 = self.reg_params['mu1'], **self.solver_params)
 
 
-        # rescale (DEACTIVATED)
-        # if self.do_scaling:
-        #     sol['Theta'] = self._rescale_to_covariances(sol['Theta'], self._scale)
-        #     if self.latent:
-        #         sol['L'] = self._rescale_to_covariances(sol['L'], self._scale)
+        # rescale
+        if self.do_scaling:
+            sol['Theta'] = self._rescale_to_covariances(sol['Theta'], self._scale)
+            if self.latent:
+                sol['L'] = self._rescale_to_covariances(sol['L'], self._scale)
 
-
+                
         # set the computed solution
         if self.latent:
             self.solution._set_solution(Theta = sol['Theta'], L = sol['L'])   
@@ -622,11 +621,11 @@ def model_selection(self, modelselect_params = None, method = 'eBIC', gamma = 0.
         # SET SOLUTION AND STORE INFOS
         ###############################
 
-        # rescale (DEACTIVATED)
-        # if self.do_scaling:
-        #     sol['Theta'] = self._rescale_to_covariances(sol['Theta'], self._scale)
-        #     if self.latent:
-        #         sol['L'] = self._rescale_to_covariances(sol['L'], self._scale)
+        # rescale
+        if self.do_scaling:
+            sol['Theta'] = self._rescale_to_covariances(sol['Theta'], self._scale)
+            if self.latent:
+                sol['L'] = self._rescale_to_covariances(sol['L'], self._scale)
 
         # set the computed solution
         if self.latent:

diff --git a/gglasso/tests/test_problem.py b/gglasso/tests/test_problem.py
@@ -172,11 +172,13 @@ def test_scaling_SGL():
     P2.set_reg_params(reg_params)
     P2.solve(tol = 1e-15, rtol = 1e-15, solver_params = solver_params)
 
-    # scaled one has factor 1/sc after rescaling
+    # precision is rescaled with 1/sc
     Theta = P.solution.precision_
     Theta2 = P2.solution.precision_
 
-    assert np.linalg.norm(Theta - Theta2)/np.linalg.norm(Theta) <= 1e-10
+    Theta2 = scale_array_by_diagonal(Theta2, 1/sc) # from covariances to correlations on inverse
+
+    assert_array_almost_equal(Theta, Theta2, decimal=3)
     assert_array_almost_equal(P.solution.adjacency_, P2.solution.adjacency_)
 
     return