Added monotonic spline-function for dx-values. #43

nmueller18 · 2019-07-07T22:02:11Z

@nevrome , @MartinHinz , @chrinne: In relation to the issues if have filed in the past (here and here), I have added a spline-function to offer the possibility to smooth the sometimes very rugged curves of the life-table for the age categories 20+. In my view this is much better than a moving window because within the range of adjacent knots the sums stay the same. Of course, this is still a severe manipulation of the data and should not applied lightly.
The function cm.spline derives from the demography-package and has the necessary advantage that it is forced to be monotonic, negative values are therefore not possible. It is applied to the original cumulative dx-values. The user can choose the wanted resolution (e. g. one knot every 20 years), and because every other column of the life table depends on dx, the interpolated dx-values bear on the life-table as a whole.
E.g.: life.table(prep_life,option_spline = 20)
In theory, the data can be split at every arbitrary age-interval (indeed, it is mainly used to get yearly census of already tabulated data), but I do not think that it makes a lot of sense to go beyond 5 year-intervals. Therefore, and because a non-standard vector of age-intervals will almost certainly collide with the regular spacing as done by cm.spline, I would only allow this method of interpolation to be called when the 'method' in prep.life.table was either set to 'Standard' or to 'Equal5'. This, however, has not been implemented yet because I have not found an easy way to carry over these options from one function to the other.
Please have a look; any comments are appreciated as well as help in the issue just mentioned, or in rewriting the doubtless very unelegant code.

codecov-io · 2019-07-07T22:21:07Z

Codecov Report

Merging #43 into master will decrease coverage by 11.3%.
The diff coverage is 15.38%.

@@             Coverage Diff             @@
##           master      #43       +/-   ##
===========================================
- Coverage   96.97%   85.67%   -11.31%     
===========================================
  Files           4        4               
  Lines         331      377       +46     
===========================================
+ Hits          321      323        +2     
- Misses         10       54       +44

Impacted Files	Coverage Δ
R/analytical_functions.R	`74.13% <15.38%> (-25.09%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab0c2b5...b288a98. Read the comment docs.

MartinHinz · 2019-07-09T08:00:24Z

NAMESPACE

@@ -17,6 +17,7 @@ export(is.mortaar_life_table_list)
 export(life.table)
 export(prep.life.table)
 importFrom(Rdpack,reprompt)
+importFrom(demography,cm.spline)


Probably not necessary to add another dependency: cm.spline is only a wrapper for stats::spline, see below

MartinHinz · 2019-07-09T08:01:05Z

R/analytical_functions.R

+  x_spline <- a_cumsum$a[a_cumsum_select]
+
+  # interpolating the values with a monotonic cubic spline
+  dem <- cm.spline(x_spline, y_spline, n = (limit/5 + 1), xmin = 0, xmax = limit)


could be replaced by:
dem <- stats::spline(x_spline, y_spline, n = (limit/5 + 1), xmin = 0, xmax = limit, method = "hyman")

MartinHinz · 2019-07-09T08:02:11Z

R/analytical_functions.R

+
+  }
+
+


the whole function might benefit from refactoring and extracting functions. Can be done after merging pull request

I agree. Maybe -- as a first step -- it's best to create an individual calculation function for each variable in life.table(). In case of dx the code in life.table() could look like this:

if (!(length(option_spline) > 0)) { necdf['dx'] <- dx_default(necdf['Dx']) } else { necdf['dx'] <- dx_spline(necdf['a'], necdf['Dx']) }

with dx_default() like this

dx_default <- function(Dx) { Dx / Dx * 100 }

and dx_spline() with the code you wrote.

MartinHinz

In general very nice, thank you for adding this feature! We might not need to include the demography package, see comments in the code! In short,

dem <- demography::cm.spline(x_spline, y_spline, n = (limit/5 + 1), xmin = 0, xmax = limit)

might be replaced by

dem <- stats::spline(x_spline, y_spline, n = (limit/5 + 1), xmin = 0, xmax = limit, method = "hyman")

in line 311

nmueller18 · 2019-07-09T20:33:25Z

dem <- demography::cm.spline(x_spline, y_spline, n = (limit/5 + 1), xmin = 0, xmax = limit)
might be replaced by
dem <- stats::spline(x_spline, y_spline, n = (limit/5 + 1), xmin = 0, xmax = limit, method = "hyman")
in line 311

Done.

nevrome

Nice!

nevrome · 2019-07-11T11:09:37Z

R/analytical_functions.R

+#' will interpolate the values for individuals of an age of 20 or older by 10- or 20-
+#' year cumulated values. To be used carefully, as diagnostic features of the life
+#' table might be smoothed and essentially removed. Only available when the methods
+#' 'Standard' or 'Equal5' in prep.life.table have been chosen.


This option should not soft-depend on an obscure (obscure for the user) option in the prep function. The prep function is not necessary to use life.table(). Instead the conditions to use this option should be explained here directly. I also suggest to add a check at the beginning of the if (length(option_spline) > 0) {...} block to test if the condition is met.

A check like this could look like this:

if (!condition) { stop("The condition is not fulfilled. Please take a look at ?life.table to determine how your input data should look like for the option_spline option.") }

nevrome · 2019-07-11T11:20:30Z

R/analytical_functions.R

+
+  }
+
+


I agree. Maybe -- as a first step -- it's best to create an individual calculation function for each variable in life.table(). In case of dx the code in life.table() could look like this:

if (!(length(option_spline) > 0)) { necdf['dx'] <- dx_default(necdf['Dx']) } else { necdf['dx'] <- dx_spline(necdf['a'], necdf['Dx']) }

with dx_default() like this

dx_default <- function(Dx) { Dx / Dx * 100 }

and dx_spline() with the code you wrote.

nmueller18 · 2019-07-12T07:47:28Z

Thank you for your input! I will try to implement these suggestions during the weekend.

…y standard-options are used for creation of life table.

nmueller18 · 2019-07-21T17:54:10Z

@nevrome , @MartinHinz: Could you please have a look at the new commit? life.tabe now throws an error with option_spline enabled. I cannot find the error because the individual functions seem to run fine.
The error code is as follows:

Fehler in Math.factor(necdf[, "dx"]) : ‘cumsum’ not meaningful for factors
Math.factor(necdf[, "dx"]) at analytical_functions.R#349

Somehow, dx is transformed to a factor, but I do not know where and why.

… work again.

some refactoring

nmueller18 · 2019-07-28T21:11:20Z

@nevrome: As usual: Much more elegant and consistent! From my point of view, the branch could be merged (@nevrome: Your approval seems to be necessary ...).

Added monotonic spline-function for dx-values.

6030f00

nmueller18 requested review from nevrome, chrinne and MartinHinz July 7, 2019 22:02

MartinHinz reviewed Jul 9, 2019

View reviewed changes

MartinHinz requested changes Jul 9, 2019

View reviewed changes

Removed dependency of package "demography" as suggested

e622b2b

MartinHinz approved these changes Jul 10, 2019

View reviewed changes

nevrome requested changes Jul 11, 2019

View reviewed changes

Spline-option now refactored and check added for making sure that onl…

854c221

…y standard-options are used for creation of life table.

nevrome and others added 4 commits July 21, 2019 21:55

some refactoring

545e6a9

Made a small change in the variable naming, and the function seems to…

e76d717

… work again.

Merge pull request #44 from ISAAKiel/dx_spline_slightly_changed

1fb767b

some refactoring

implemented option_spline test a bit more elegant

b288a98

nevrome merged commit 65710c3 into master Jul 29, 2019

nevrome deleted the dx_spline branch July 29, 2019 07:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added monotonic spline-function for dx-values. #43

Added monotonic spline-function for dx-values. #43

nmueller18 commented Jul 7, 2019 •

edited

Loading

codecov-io commented Jul 7, 2019 •

edited

Loading

MartinHinz Jul 9, 2019

MartinHinz Jul 9, 2019

MartinHinz Jul 9, 2019

nevrome Jul 11, 2019

MartinHinz left a comment •

edited

Loading

nmueller18 commented Jul 9, 2019

nevrome left a comment

nevrome Jul 11, 2019

nevrome Jul 11, 2019

nevrome Jul 11, 2019

nmueller18 commented Jul 12, 2019

nmueller18 commented Jul 21, 2019

nmueller18 commented Jul 28, 2019

Added monotonic spline-function for dx-values. #43

Added monotonic spline-function for dx-values. #43

Conversation

nmueller18 commented Jul 7, 2019 • edited Loading

codecov-io commented Jul 7, 2019 • edited Loading

Codecov Report

MartinHinz Jul 9, 2019

Choose a reason for hiding this comment

MartinHinz Jul 9, 2019

Choose a reason for hiding this comment

MartinHinz Jul 9, 2019

Choose a reason for hiding this comment

nevrome Jul 11, 2019

Choose a reason for hiding this comment

MartinHinz left a comment • edited Loading

Choose a reason for hiding this comment

nmueller18 commented Jul 9, 2019

nevrome left a comment

Choose a reason for hiding this comment

nevrome Jul 11, 2019

Choose a reason for hiding this comment

nevrome Jul 11, 2019

Choose a reason for hiding this comment

nevrome Jul 11, 2019

Choose a reason for hiding this comment

nmueller18 commented Jul 12, 2019

nmueller18 commented Jul 21, 2019

nmueller18 commented Jul 28, 2019

nmueller18 commented Jul 7, 2019 •

edited

Loading

codecov-io commented Jul 7, 2019 •

edited

Loading

MartinHinz left a comment •

edited

Loading