search

gasparrini / dlnm

R package dlnm
68 stars 13 forks source link

Add "cs" basis as a possibility for smooth.construct.cb.smooth.spec #7

Closed Aariq closed 3 years ago

Aariq commented 3 years ago

All I did in this PR is edit an if statement that allows "cs" to be a basis for a crossbasis smooth created by mgcv::s(). I think this "just works" because all that smooth.construct.cs.smooth.spec() does is add an attribute to object, and then pass it to smooth.construct.cr.smooth.spec().

To see that this is working:

library(dlnm)
#> This is dlnm 2.4.2. For details: help(dlnm) and vignette('dlnmOverview').
library(mgcv)
#> Loading required package: nlme
#> This is mgcv 1.8-33. For overview type 'help("mgcv-package")'.

Q <- as.matrix(nested[, 5:14]) 
L <- matrix(1:ncol(Q), nrow = nrow(Q), ncol = ncol(Q), byrow = TRUE)

m_cr <- gam(case ~ age + s(Q, L, bs = "cb", xt = list(bs = "cr")), family = binomial, data = nested)
m_cs <- gam(case ~ age + s(Q, L, bs = "cb", xt = list(bs = "cs")), family = binomial, data = nested)
#increased gamma exaggerates the difference between "cr" and "cs"

cr_pred <- crosspred("Q", m_cr)
#> centering value unspecified. Automatically set to 600
cs_pred <- crosspred("Q", m_cs)
#> centering value unspecified. Automatically set to 600

#cs spline is pulled toward 0 in both lag and response dimensions

plot(cr_pred, ptype = "slices", lag = 10, ylim = c(-50, 50))

plot(cs_pred, ptype = "slices", lag = 10, ylim = c(-50, 50))


plot(cr_pred, ptype = "slices", var = 1000, ylim = c(-20, 40))

plot(cs_pred, ptype = "slices", var = 1000, ylim = c(-20, 40))

Created on 2020-10-15 by the reprex package (v0.3.0)

I also added an example to the help file for smooth.construct.cb.smooth.spec() and updated NEWS.md.

I can't get the vignettes to build locally, so wasn't able to run R CMD check

Aariq commented 3 years ago

Forgot to mention: this closes #5

gasparrini commented 3 years ago

Thanks, Eric.

I would like to see some more checks, for example used in some of the case-studies and/or simulations in https://github.com/gasparrini/2017_gasparrini_Biomet_Rcodedata.

In particular, I would check if the definition of the penalty matrices (smooth.construct.cb.smooth.spec) and predictions (see Predict.matrix.cb.smooth) work as expected.

Antonio Gasparrini, BSc Mbiol MSc PhD

Professor of Biostatistics and Epidemiology London School of Hygiene & Tropical Medicine 15-17 Tavistock Place, London WC1H 9SH, UK Office: 0044 (0)20 79272406 Mobile: 0044 (0)79 64925523 Skype: a.gasparrini Twitter: @AGasparrini75 http://www.ag-myresearch.com

From: Eric R Scott notifications@github.com Sent: 16 October 2020 02:08 To: gasparrini/dlnm dlnm@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: [gasparrini/dlnm] Add "cs" basis as a possibility for smooth.construct.cb.smooth.spec (#7)

All I did in this PR is edit an if statement that allows "cs" to be a basis for a crossbasis smooth created by mgcv::s(). I think this "just works" because all that smooth.construct.cs.smooth.spec() does is add an attribute to object, and then pass it to smooth.construct.cr.smooth.spec().

To see that this is working:

library(dlnm)

> This is dlnm 2.4.2. For details: help(dlnm) and vignette('dlnmOverview').

library(mgcv)

> Loading required package: nlme

> This is mgcv 1.8-33. For overview type 'help("mgcv-package")'.

Q <- as.matrix(nested[, 5:14])

L <- matrix(1:ncol(Q), nrow = nrow(Q), ncol = ncol(Q), byrow = TRUE)

m_cr <- gam(case ~ age + s(Q, L, bs = "cb", xt = list(bs = "cr")), family = binomial, data = nested)

m_cs <- gam(case ~ age + s(Q, L, bs = "cb", xt = list(bs = "cs")), family = binomial, data = nested)

increased gamma exaggerates the difference between "cr" and "cs"

cr_pred <- crosspred("Q", m_cr)

> centering value unspecified. Automatically set to 600

cs_pred <- crosspred("Q", m_cs)

> centering value unspecified. Automatically set to 600

cs spline is pulled toward 0 in both lag and response dimensions

plot(cr_pred, ptype = "slices", lag = 10, ylim = c(-50, 50))

[https://camo.githubusercontent.com/6397baccd8df5b2352eb613cf759a8f48ecd74cc/68747470733a2f2f692e696d6775722e636f6d2f58334a73644e562e706e67]https://camo.githubusercontent.com/6397baccd8df5b2352eb613cf759a8f48ecd74cc/68747470733a2f2f692e696d6775722e636f6d2f58334a73644e562e706e67

plot(cs_pred, ptype = "slices", lag = 10, ylim = c(-50, 50))

[https://camo.githubusercontent.com/0b7b9e839a3fa4584dc86314cd37922ee08b4cc2/68747470733a2f2f692e696d6775722e636f6d2f534e436d7638422e706e67]https://camo.githubusercontent.com/0b7b9e839a3fa4584dc86314cd37922ee08b4cc2/68747470733a2f2f692e696d6775722e636f6d2f534e436d7638422e706e67

plot(cr_pred, ptype = "slices", var = 1000, ylim = c(-20, 40))

[https://camo.githubusercontent.com/02c0808e09537a43346c159f45a0146e6355ea62/68747470733a2f2f692e696d6775722e636f6d2f4679596f6669702e706e67]https://camo.githubusercontent.com/02c0808e09537a43346c159f45a0146e6355ea62/68747470733a2f2f692e696d6775722e636f6d2f4679596f6669702e706e67

plot(cs_pred, ptype = "slices", var = 1000, ylim = c(-20, 40))

[https://camo.githubusercontent.com/a278fcb9e06b96c537dcc9cf3e1799fe57e1b7bb/68747470733a2f2f692e696d6775722e636f6d2f57546d373654422e706e67]https://camo.githubusercontent.com/a278fcb9e06b96c537dcc9cf3e1799fe57e1b7bb/68747470733a2f2f692e696d6775722e636f6d2f57546d373654422e706e67

Created on 2020-10-15 by the reprex packagehttps://reprex.tidyverse.org (v0.3.0)

I also added an example to the help file for smooth.construct.cb.smooth.spec() and updated NEWS.md.

I can't get the vignettes to build locally, so wasn't able to run R CMD check

You can view, comment on, or merge this pull request online at:

https://github.com/gasparrini/dlnm/pull/7https://github.com/gasparrini/dlnm/pull/7

Commit Summary

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/gasparrini/dlnm/pull/7, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADHVOGT2DEYZBGZEOKAVGHDSK6FF5ANCNFSM4SSUVX6A.

Aariq commented 3 years ago

That makes sense. I haven't checked how this change interacts with predict(). I'll add some tests and let you know how it goes.

Aariq commented 3 years ago

I have a question: what is the intended difference (if any) between the following two ways of specifying marginal basis?

1)

gam(y ~ s(Q, L, bs = "cb", xt = list(bs = c("ps", "cr"))))

2)

gam(y ~ s(Q, L, bs = "cb", xt = list(argvar = list(fun = "ps"), arglag = list(fun = "cr")))))

Method 1 is what is described in the help file for smooth.construct.cb.smooth.spec, but I see that method 2 is what is used in the 2017 paper. They seem to be handled differently by smooth.construct.cb.smooth.spec.

Aariq commented 3 years ago

I'm not totally convinced that this is working right as the difference between "cr" and "cs" in a cross basis seems way more drastic than in a normal 1D spline where I see only minor differences. I'm also no longer convinced that I ever want to use the "cs" basis personally, so I'm going to close this pull request.

gasparrini commented 3 years ago

I’ve always had doubts that this could work so easily. In addition, shrinking to null this bidimensional shape in one dimension causes the other dimension to shrink as well, which seems problematic.

In any case, thanks for the interest and for doing some work on the topic. Maybe it will turn out useful in the future.

Best

Antonio Gasparrini, BSc Mbiol MSc PhD

Professor of Biostatistics and Epidemiology London School of Hygiene & Tropical Medicine 15-17 Tavistock Place, London WC1H 9SH, UK Office: 0044 (0)20 79272406 Mobile: 0044 (0)79 64925523 Skype: a.gasparrini Twitter: @AGasparrini75 http://www.ag-myresearch.com

From: Eric R Scott notifications@github.com Sent: 23 October 2020 23:57 To: gasparrini/dlnm dlnm@noreply.github.com Cc: Antonio Gasparrini Antonio.Gasparrini@lshtm.ac.uk; Comment comment@noreply.github.com Subject: Re: [gasparrini/dlnm] Add "cs" basis as a possibility for smooth.construct.cb.smooth.spec (#7)

I'm not totally convinced that this is working right as the difference between "cr" and "cs" in a cross basis seems way more drastic than in a normal 1D spline where I see only minor differences. I'm also no longer convinced that I ever want to use the "cs" basis personally, so I'm going to close this pull request.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/gasparrini/dlnm/pull/7#issuecomment-715608005, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADHVOGR6EPTHJTFZSVOBGODSMH33HANCNFSM4SSUVX6A.