Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding the fit_model function #277

Draft
wants to merge 41 commits into
base: devel
Choose a base branch
from
Draft

Adding the fit_model function #277

wants to merge 41 commits into from

Conversation

gareth-j
Copy link
Contributor

@gareth-j gareth-j commented Nov 20, 2023

  • Summary of changes (Bug fix, feature, docs update, ...)

This will add a fit_model function that in one line will create

  • a mesh
  • a formula
  • fit the model
  • return the fitted model object
  • Please check if the PR fulfills these requirements

Copy link
Contributor

@XueqingYin XueqingYin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi , I found several issues in fit_model.R and fit_model.Rmd.

for fit_model.R:

  1. The purpose of having two inlabru::like() in this model is not clear to me. Maybe it's useful to explain the aim of each like(), and the problem that this model is solving?
  2. Do you want to include the arguments (such as max.edge) that control mesh resolution when creating mesh? At present, they are missing in fm_mesh_2d_inla(), and I found max_edge_mesh is not used.
  3. The object types for space_variables, spacetime_variables, and data need clear description. If space_variables and spacetime_variables are list objects as described now at the beginning of the script, then data[space_variables] will give an error "Error in .subset2(x, i, exact = exact) : invalid subscript type 'list'". To avoid this, space_variables and spacetime_variables should be a vector of spatial and spacetime variables, respectively.
  4. The input for the argument locations should be a matrix of point locations, rather than a list of coordinates

for fit_model.Rmd:

  1. process_coords argument does not exist in the fit_model() function
  2. space_variables and spacetime_variables are missing in this rain example
  3. If I remember it correctly, inlabru::bru requires the time index to start from 1, whereas it now starts from 5 in this example

@mnky9800n
Copy link
Contributor

For the fit_model.rmd we actually stopped using that and it should be removed because we switched to using the test_fit_model.rmd

@mnky9800n
Copy link
Contributor

Also, process_coords was replaced with locations to stay consistent

@mnky9800n
Copy link
Contributor

We didn't put the arguments max.edge in the fmesher function because it is now supposed to estimate the best values for those. Since the goal was trying to produce a quick and dirty latent random field generator we figured this was inline with that idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fit_model - fitting the model fit_model - generate formula fit_model - meshing function
4 participants