SEEG-Oxford / movement

R package containing useful functions for the analysis of movement data in disease modelling and mapping
20 stars 16 forks source link

define more user-friendly interface #39

Closed goldingn closed 8 years ago

goldingn commented 8 years ago

currently the main model-fitting function is movement. This requires many pieces of data from the user:

movement(locations, coords, population, movement_matrix, model,  model.params = NULL, ...)

with required arguments:

And optional arguments:

A simpler interface would be:

movement(movement_matrix, location_dataframe, model = gravity(), optim_options = list())

possibly with glm-like function interface:

movement(movement_matrix ~ location_dataframe)

with required arguments:

and optional parameters

Existing functions could be modified to help users create the location_dataframe and movement_matrix objects, the *.flux functions could be unexported and their docs replaced with more model- (rather than code-) centric docs for these initialisation functions.

Finally, we could write generic print, plot, and summary methods for the objects returned by the initialisation functions (flux objects?) and the same models with optimised parameters. e.g.:

flux <- gravity(c(0.5, 0.2, 0.1, 5))
plot(flux)

and

m <- movement(obs ~ locs)
plot(m$flux)

Thoughts on this proposed interface?

goldingn commented 8 years ago

Sorry to shift the goalposts - could we please change the interface to the flux functions so that each parameter is a separate named argument?

I.e.:

flux <- gravity(theta = 0.5, alpha = 0.2, beta = 0.1, gamma = 5)

this would enable users to change only one of the arguments from its defaults, without having to write out the lot:

flux <- gravity(theta = 0.1)

This could work internally like this:

gravity  <- function(theta=0.01, alpha=0.06, beta=0.03, gamma=0.01) {
  params <- c(theta=theta, alpha=alpha, beta=beta, gamma=gamma)
  ans  <- list(params = params, flux = gravity.flux)
  class(ans)  <- 'flux'
  return(ans)
}

and if we need to pass these automatically we can do this:

params <- c(theta = 0.01, alpha = 0.06, beta = 0.03, gamma = 0.01)
fl <- do.call('gravity', params)
KathrinTessella commented 8 years ago

Just a quick summary what has been done so far: 1) combine the args 'locations, coords, population' into a single data frame object: see https://github.com/SEEG-Oxford/movement/pull/49 2) define the various flux models: see https://github.com/SEEG-Oxford/movement/pull/50 3) update the flux model interfaces: see https://github.com/SEEG-Oxford/movement/pull/56 4) adjust the code to incorporate the newly created flux models: see https://github.com/SEEG-Oxford/movement/pull/58

KathrinTessella commented 8 years ago

@goldingn : any further comments what do you want for the generic methods 'print', 'plot', and 'summary' for the flux objects (e.g. flux <- gravity(); print(flux); summary(flux) and plot(flux))?

goldingn commented 8 years ago

Looks great!

For now, could you please not define plot, define summary to give the same output as print, and define print to print the flux type and named parameters. E.g. something along the lines of:

print(flux)
  flux object for a gravity model with parameters:

      theta = 0.01
      alpha = 0.06
      beta  = 0.03
      gamma = 0.01

  see ?gravity for the model formulation and explanation of parameters
KathrinTessella commented 8 years ago

Defined the generic methods for print() and summary() on the flux objects as suggested above (see https://github.com/SEEG-Oxford/movement/pull/62)

KathrinTessella commented 8 years ago

@goldingn I believe the final task on this issue is to further adjust the interface to be called like: movement(movement_matrix ~ location_dataframe) Is that still wanted ?

goldingn commented 8 years ago

yes please!

KathrinTessella commented 8 years ago

The final tasks to update the interface using the formula 'movement_matrix ~ location_dataframe' has been addressed in pull request https://github.com/SEEG-Oxford/movement/pull/64.