Repository: QB3/sparse-ho

Issue:
SURE naming / API / generalization
Consider renaming `SURE` to something more in adequation with the
implementation. The current implementation follows [(Figure 3, Deledalle et al. 2020)](https://samuelvaiter.com/publications/deledalle2014sugar.pdf) by
implementing a smoothed version using Finite Difference Monte Carlo (FDMC)
method. A user testing the class might be confused to see that in low dimension,
a SURE implemented manually does not provide the same value as our `SURE` class.

I believe there is three possible options:
1. _Keep the current name._ This is probably the best way to not confuse people
   not familiar with the SURE. But in this case the documentation should reflect
   precisely why a user could observe a difference with a handcraft SURE.
2. _Change the name to one relevant to the litterature._ The more exact would be
   to use `FdmcSure` but this is a bit obscure for people and
   `FiniteDifferenceMonteCarloSure` maybe too explicit. Another option is to use
   SUGAR as it is easy to search. The issue is that SUGAR relates to the
   gradient of the criterion, which could be also confusing.
3. _Change the name to something clearer._ Maybe a `Smooth[ed]Sure` could be a
   candidate, but this is not a terminology which appears in papers.

### Parameters
- I don't understand why `SURE` takes `X_test` and `X_test` as parameters.
- Regarding `C` and `gamma_sure`. I believe it is not useful to expose them as
  parameters. Either the user does not want to play with it and two optional
  parameters are too much, or he really wants to control the finite difference
  step and it would be easier to directly access the member `epsilon`. The power
  law is advocated in the SUGAR paper, but different strategies could be used.

### Generalised SURE
It does not seem to be a priority, but this class could implements
generalization of the SURE to approximate the prediction risk and/or estimation
risk depending on the properties of `X`, which could be of interest for some use
(imaging for instance).

WDYT?

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/QB3/sparse-ho/pull/67?src=pr&el=h1) Report
> Merging [#67](https://codecov.io/gh/QB3/sparse-ho/pull/67?src=pr&el=desc) (7f41db0) into [master](https://codecov.io/gh/QB3/sparse-ho/commit/59197a06f2ba62b4fd67b9d8950dc62674eed2a1?el=desc) (59197a0) will **increase** coverage by `0.09%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/QB3/sparse-ho/pull/67/graphs/tree.svg?width=650&height=150&src=pr&token=2BACA3WUXN)](https://codecov.io/gh/QB3/sparse-ho/pull/67?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #67      +/-   ##
==========================================
+ Coverage   67.89%   67.99%   +0.09%     
==========================================
  Files          47       47              
  Lines        3118     3118              
  Branches      279      279              
==========================================
+ Hits         2117     2120       +3     
+ Misses        923      921       -2     
+ Partials       78       77       -1     
```


| [Impacted Files](https://codecov.io/gh/QB3/sparse-ho/pull/67?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sparse\_ho/criterion/\_\_init\_\_.py](https://codecov.io/gh/QB3/sparse-ho/pull/67/diff?src=pr&el=tree#diff-c3BhcnNlX2hvL2NyaXRlcmlvbi9fX2luaXRfXy5weQ==) | `100.00% <100.00%> (ø)` | |
| [sparse\_ho/criterion/sure.py](https://codecov.io/gh/QB3/sparse-ho/pull/67/diff?src=pr&el=tree#diff-c3BhcnNlX2hvL2NyaXRlcmlvbi9zdXJlLnB5) | `87.50% <100.00%> (ø)` | |
| [sparse\_ho/tests/test\_grad\_search.py](https://codecov.io/gh/QB3/sparse-ho/pull/67/diff?src=pr&el=tree#diff-c3BhcnNlX2hvL3Rlc3RzL3Rlc3RfZ3JhZF9zZWFyY2gucHk=) | `100.00% <100.00%> (ø)` | |
| [sparse\_ho/tests/test\_grid\_search.py](https://codecov.io/gh/QB3/sparse-ho/pull/67/diff?src=pr&el=tree#diff-c3BhcnNlX2hvL3Rlc3RzL3Rlc3RfZ3JpZF9zZWFyY2gucHk=) | `100.00% <100.00%> (ø)` | |
| [sparse\_ho/tests/test\_lasso.py](https://codecov.io/gh/QB3/sparse-ho/pull/67/diff?src=pr&el=tree#diff-c3BhcnNlX2hvL3Rlc3RzL3Rlc3RfbGFzc28ucHk=) | `100.00% <100.00%> (ø)` | |
| [sparse\_ho/optimizers/line\_search.py](https://codecov.io/gh/QB3/sparse-ho/pull/67/diff?src=pr&el=tree#diff-c3BhcnNlX2hvL29wdGltaXplcnMvbGluZV9zZWFyY2gucHk=) | `69.86% <0.00%> (+4.10%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/QB3/sparse-ho/pull/67?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/QB3/sparse-ho/pull/67?src=pr&el=footer). Last update [59197a0...7f41db0](https://codecov.io/gh/QB3/sparse-ho/pull/67?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/doc/api.rst b/doc/api.rst
index 6cde70a..55810a7 100644
--- a/doc/api.rst
+++ b/doc/api.rst
@@ -44,7 +44,7 @@ Criterion
    :toctree: generated/
 
    HeldOutMSE
-   SmoothedSURE
+   FiniteDiffMonteCarloSure
    HeldOutLogistic
 
 
diff --git a/sparse_ho/criterion/__init__.py b/sparse_ho/criterion/__init__.py
index 1402e4e..8e96d7e 100644
--- a/sparse_ho/criterion/__init__.py
+++ b/sparse_ho/criterion/__init__.py
@@ -1,11 +1,11 @@
 from sparse_ho.criterion.held_out import HeldOutMSE, HeldOutLogistic
 from sparse_ho.criterion.cross_val import CrossVal
-from sparse_ho.criterion.sure import SmoothedSURE
+from sparse_ho.criterion.sure import FiniteDiffMonteCarloSure
 from sparse_ho.criterion.held_out import HeldOutSmoothedHinge
 from sparse_ho.criterion.multiclass_logreg import LogisticMulticlass
 
 __all__ = ['CrossVal',
-           'SmoothedSURE',
+           'FiniteDiffMonteCarloSure',
            'HeldOutMSE',
            'HeldOutLogistic',
            'HeldOutSmoothedHinge',
diff --git a/sparse_ho/criterion/sure.py b/sparse_ho/criterion/sure.py
index 3719b75..980ad11 100644
--- a/sparse_ho/criterion/sure.py
+++ b/sparse_ho/criterion/sure.py
@@ -5,7 +5,7 @@ from sparse_ho.algo.forward import get_beta_jac_iterdiff
 from sparse_ho.criterion.base import BaseCriterion
 
 
-class SmoothedSURE(BaseCriterion):
+class FiniteDiffMonteCarloSure(BaseCriterion):
     """Smoothed version of the Stein Unbiased Risk Estimator (SURE).
 
     Implements the iterative Finite-Difference Monte-Carlo approximation of the


Test patch:
diff --git a/sparse_ho/tests/test_grad_search.py b/sparse_ho/tests/test_grad_search.py
index 492c04b..a8537f7 100644
--- a/sparse_ho/tests/test_grad_search.py
+++ b/sparse_ho/tests/test_grad_search.py
@@ -10,7 +10,7 @@ from sparse_ho.models import Lasso
 from sparse_ho import Forward
 from sparse_ho import ImplicitForward
 from sparse_ho import Implicit
-from sparse_ho.criterion import HeldOutMSE, SmoothedSURE
+from sparse_ho.criterion import HeldOutMSE, FiniteDiffMonteCarloSure
 from sparse_ho.ho import grad_search
 from sparse_ho.optimizers import LineSearch
 
@@ -64,7 +64,7 @@ def test_grad_search(model, crit):
         criterion = HeldOutMSE(idx_train, idx_val)
     else:
         n_outer = 2
-        criterion = SmoothedSURE(sigma_star)
+        criterion = FiniteDiffMonteCarloSure(sigma_star)
     # TODO MM@QBE if else scheme surprising
 
     criterion = HeldOutMSE(idx_train, idx_val)
diff --git a/sparse_ho/tests/test_grid_search.py b/sparse_ho/tests/test_grid_search.py
index 2e1d6c6..4df0ff7 100644
--- a/sparse_ho/tests/test_grid_search.py
+++ b/sparse_ho/tests/test_grid_search.py
@@ -6,7 +6,7 @@ from sparse_ho.utils import Monitor
 from sparse_ho.datasets import get_synt_data
 from sparse_ho.models import Lasso
 from sparse_ho import Forward
-from sparse_ho.criterion import HeldOutMSE, SmoothedSURE
+from sparse_ho.criterion import HeldOutMSE, FiniteDiffMonteCarloSure
 from sparse_ho.grid_search import grid_search
 
 
@@ -69,7 +69,7 @@ def test_grid_search():
     monitor_grid = Monitor()
     model = Lasso(estimator=estimator)
 
-    criterion = SmoothedSURE(sigma=sigma_star)
+    criterion = FiniteDiffMonteCarloSure(sigma=sigma_star)
     algo = Forward()
     log_alpha_opt_grid, _ = grid_search(
         algo, criterion, model, X, y, log_alpha_min, log_alpha_max,
@@ -77,7 +77,7 @@ def test_grid_search():
         tol=1e-5, samp="grid")
 
     monitor_random = Monitor()
-    criterion = SmoothedSURE(sigma=sigma_star)
+    criterion = FiniteDiffMonteCarloSure(sigma=sigma_star)
     algo = Forward()
     log_alpha_opt_random, _ = grid_search(
         algo, criterion, model, X, y, log_alpha_min, log_alpha_max,
diff --git a/sparse_ho/tests/test_lasso.py b/sparse_ho/tests/test_lasso.py
index 8852e2d..8553435 100644
--- a/sparse_ho/tests/test_lasso.py
+++ b/sparse_ho/tests/test_lasso.py
@@ -13,7 +13,7 @@ from sparse_ho import Forward
 from sparse_ho import ImplicitForward
 from sparse_ho import Implicit
 from sparse_ho import Backward
-from sparse_ho.criterion import HeldOutMSE, SmoothedSURE
+from sparse_ho.criterion import HeldOutMSE, FiniteDiffMonteCarloSure
 
 n_samples = 100
 n_features = 100
@@ -160,22 +160,22 @@ def test_val_grad():
     for key in models.keys():
         log_alpha = dict_log_alpha[key]
         model = models[key]
-        criterion = SmoothedSURE(sigma_star)
+        criterion = FiniteDiffMonteCarloSure(sigma_star)
         algo = Forward()
         val_fwd, grad_fwd = criterion.get_val_grad(
             model, X, y, log_alpha, algo.get_beta_jac_v, tol=tol)
 
-        criterion = SmoothedSURE(sigma_star)
+        criterion = FiniteDiffMonteCarloSure(sigma_star)
         algo = ImplicitForward(tol_jac=1e-8, n_iter_jac=5000)
         val_imp_fwd, grad_imp_fwd = criterion.get_val_grad(
             model, X, y, log_alpha, algo.get_beta_jac_v, tol=tol)
 
-        criterion = SmoothedSURE(sigma_star)
+        criterion = FiniteDiffMonteCarloSure(sigma_star)
         algo = Implicit(criterion)
         val_imp, grad_imp = criterion.get_val_grad(
             model, X, y, log_alpha, algo.get_beta_jac_v, tol=tol)
 
-        criterion = SmoothedSURE(sigma_star)
+        criterion = FiniteDiffMonteCarloSure(sigma_star)
         algo = Backward()
         val_bwd, grad_bwd = criterion.get_val_grad(
             model, X, y, log_alpha, algo.get_beta_jac_v, tol=tol)