Poisson family and line-search procedure #27

wzzlcss · 2019-08-14T23:35:28Z

This PR implements poisson regression using adaptive step size. Here is a comparison between glmnet using caddisfly dataset. This is the output from a version that previously under development and test: poisson_check.pdf.

data("caddisfly")
sfit0 <- sgdnet(caddisfly$x,caddisfly$y,family="poisson",alpha=0,thresh=1e-9)
gfit0 <- glmnet(caddisfly$x,caddisfly$y,family="poisson",alpha=0,thresh=1e-9)

sfit1 <- sgdnet(caddisfly$x,caddisfly$y,family="poisson",alpha=1,thresh=1e-9)
gfit1 <- glmnet(caddisfly$x,caddisfly$y,family="poisson",alpha=1,thresh=1e-9)

…or adaptive step size [skip ci]

michaelweylandt · 2019-08-21T03:02:08Z

Hi @wzzlcss

Do you have any references for the step-size optimization / line-search procedure scheme being used? I haven't seen any and would be interested in learning more about the approach being used here.

wzzlcss · 2019-08-21T03:05:52Z

Hi mentor, I am using the method from lightning: https://github.com/scikit-learn-contrib/lightning/blob/master/lightning/impl/sag_fast.pyx, which use theory from the section 4.6 of Schmidt, M., Roux, N., & Bach, F. (2013). "Minimizing finite sums with the stochastic average gradient". arXiv Preprint arXiv:1309.2388(https://arxiv.org/abs/1309.2388)

michaelweylandt

I didn't check all the step size stuff yet (haven't had a chance to read the paper you referenced) but the tests look solid so I'm hopeful.

michaelweylandt · 2019-08-21T22:16:11Z

R/score.R

+  type.measure <- match.arg(type.measure)
+  y <- as.vector(y)
+
+  y_hat <- stats::predict(fit, x, s = s)


Maybe import the generic from stats instead? I imagine we're using it pretty frequently already.

Hi mentor, may I ask how to do this?

Just add roxygen2 comment @importFrom stats predict and update the NAMESPACE file. It should "just work" after that.

michaelweylandt · 2019-08-21T22:18:29Z

R/sgdnet.R

+      n_classes <- 1L
+
+      if (length(unique(y)) == 1)
+        stop("only one class in response.")


"Class" isn't the right name here - maybe "response is constant." would be a better phrasing.

michaelweylandt · 2019-08-21T22:20:10Z

src/families.h

+      auto y_std = StandardDeviation(y, y_bar);
+
+      for (decltype(n) i = 0; i < n; ++i)
+        y_map(i) = (y(i) - y_bar(0))/y_std(0);


braces around loop body.

That's probably my doing. I have a slight preference for braceless one-line loops (sticking loosely to the google c++ code style guide), but either is fine by me.

michaelweylandt · 2019-08-21T22:21:25Z

tests/testthat/test-families.R

@@ -35,6 +35,6 @@ test_that("all combinations run without errors", {
    sfit <- do.call(sgdnet, pars)
    gfit <- do.call(glmnet, pars)

-    compare_predictions(sfit, gfit, x, tol = 1e-3)
+    compare_predictions(sfit, gfit, x, tol = 1e-2)


Is this necessary? If only needed for the Poisson, maybe special case it? I don't like loosening tolerances unless we really have to.

michaelweylandt · 2019-08-21T22:23:19Z

Dumb question: how well does the Adaptive stuff work for the Gaussian and Binomial cases? I know we have general upper bounds, but I wonder if these might be too conservative in practice.

wzzlcss · 2019-08-21T23:02:52Z

Hi mentor, thanks for the comments! I will try this with Gaussian and Binomial. And lightning sets the check frequency for lipschitz constant to 10 and it sometimes goes wrong for small size dataset, so I set to 1 to make sure to have a solution. Should I make the check frequency an user input option? I think poisson is ok with small tolerance since I have manually run these tests, maybe just because of some seed I will check all tests.

michaelweylandt · 2019-08-21T23:13:45Z

Can't hurt to make it an option, but I think if we are also suggesting decently large batches, the Lipschitz check frequency of 1 shouldn't be too high a cost to pay.

jolars · 2019-08-22T13:51:34Z

Dumb question: how well does the Adaptive stuff work for the Gaussian and Binomial cases? I know we have general upper bounds, but I wonder if these might be too conservative in practice.

I think I tried this out last summer without noticing any real benefit, but a more rigorous benchmark is probably warranted.

jolars

Nice work, this is looking good. However, would you please like to

Add examples to sgdnet() for poisson regression
Add a data set (to use in examples) for poisson. Or you could repurpose the abalone data set for this and find another data set for gaussian.

plus attend to the other comments in the review.

jolars · 2019-08-22T13:56:42Z

R/sgdnet.R

+      n_classes <- 1L
+
+      if (length(unique(y)) == 1)
+        stop("response is constant.")


Suggested change

stop("response is constant.")

stop("response is constant")

Hi mentor, I see other stop() has a ., should I remove them?

jolars · 2019-08-22T13:58:44Z

src/families.h

+
+  template <typename T>
+  double
+    LambdaMax(const T&               x,


This entire code block should not be indented.

jolars · 2019-08-22T14:03:17Z

tests/testthat/test-poisson.R

@@ -0,0 +1,26 @@
+context("poisson regression")


The authors of testthat now recommend not using context() at all. Perhaps remove it everywhere?

jolars · 2019-08-22T14:04:28Z

tests/testthat/test-poisson.R

+test_that("solutions along regularization path are the same as glmnet", {
+  set.seed(1)
+
+  N=2000; p=10


Try to keep good coding style here as well. Avoid ;, use <- instead of =, add spacing between commas and so on.

split test cases

wzzlcss added 6 commits August 14, 2019 15:52

poisson class in family.h [skip ci]

4ba1631

add poisson to input option [skip ci]

6df7e11

set up poisson in sgdnet.cpp [skip ci]

028f1c5

store column norm in ColNormsMax and Lipschitz constant in StepSize f…

7443b5b

…or adaptive step size [skip ci]

change step size during fitting for poisson dense [skip ci]

d788282

change saga-sparse.h for poisson [skip ci]

8e23685

wzzlcss added the enhancement New feature or request label Aug 15, 2019

wzzlcss added 7 commits August 20, 2019 11:19

match glmnet's loss [skip ci]

6540569

add test cases and legal check for poisson

d2275b7

prediction and cv method for poisson

3d7023e

unwant changes in NAMESPACE

d54abaf

set a smaller search frequency [skip ci]

9401ce8

add tests for poisson [skip ci]

d51bf6b

change expression [skip ci]

08164b5

add corresponding tests for poisson

55caae8

michaelweylandt reviewed Aug 21, 2019

View reviewed changes

change warning message and add braces

24a6914

jolars requested changes Aug 22, 2019

View reviewed changes

wzzlcss added 5 commits August 22, 2019 09:48

delete context() in test file and unwant indent in poisson LambdaMax

b445c11

add space between commas [skip ci]

c745e33

add poisson dataset and documents

9105635

use @importFrom stats predict

ca247a0

split test cases

ba208b2

split test cases

wzzlcss force-pushed the poisson branch from 85db5dc to ba208b2 Compare August 24, 2019 00:57

avoid generating 1 sample in a class

3a9d3bb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Poisson family and line-search procedure #27

Poisson family and line-search procedure #27

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Poisson family and line-search procedure #27

Are you sure you want to change the base?

Poisson family and line-search procedure #27

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!