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

Combine codes? #5

Open
hrlai opened this issue Sep 4, 2020 · 5 comments
Open

Combine codes? #5

hrlai opened this issue Sep 4, 2020 · 5 comments
Labels
enhancement New feature or request

Comments

@hrlai
Copy link
Collaborator

hrlai commented Sep 4, 2020

@xp-song you separated model selection and model fitting in two steps. Also separated prediction from simulation.

I combined selection and fitting, and did not distinguish prediction and simulation...

Not sure which one is better. But you'll see what I mean when you look at my codes with the _mlm suffix.

@xp-song
Copy link
Owner

xp-song commented Sep 5, 2020

For the single-sp models, the idea was to give users flexibility to specify the allometric equation to use (which may not necessarily be the 'best-fit' one) when fitting the models, using ss_modelfit() and ss_modelfit_multi(). Alternatively, users can use the best-fit model instead, using ss_modelselect() and ss_modelselect_multi(). The challenge was the presence of multiple models (one for each species), thus the need for the wrapper functions with the suffix _multi.

For the mixed-effects model, is there a situation where the user would want to specify the equation to use? Since we're using the best-fit model, for consistent naming, let's use mix_modelselect() for the function you wrote.

As for prediction/simulations in the single-sp models, ss_predict() was used to provide predictions on any data, including custom data provided by the user. Since there are many models (one per species), there was a need to refer to some kind of reference table for the modelcode/weights/whether to use correction factor/etc. ss_simulate() was designed as a wrapper to ss_predict, allowing the user to simulate data based on the generated reference table from ss_modelselect() or ss_modelselect_multi().

For the mixed-effects models, perhaps this is less crucial, since there is only one model. The user doesn't have to refer to some kind of reference table, but simply use the regular mertools::predictInterval() (though one still has to note if there is a need to back-transform predictions if the mixed-effects model is loglog/exp transformed). So likewise, I propose we use mix_simulate() for the function you wrote.

Btw, the helper function u wrote is a great idea! I edited it allow customisation in terms of specifying the range of values for extrapolation of x (default no extrapolation)

@hrlai
Copy link
Collaborator Author

hrlai commented Sep 5, 2020

For the mixed-effects model, is there a situation where the user would want to specify the equation to use?

Yes, I think this situation will arise, so I will also write a _modelfit equivalent for mixed effects model. Let me think of the best way to do so...

for consistent naming, let's use mix_modelselect() for the function you wrote.

Feel free to change all my function names in the end. I'll try to be consistent while I code them.

one still has to note if there is a need to back-transform predictions if the mixed-effects model is loglog/exp transformed

This is implicitly implemented as a default for loglog or expo mixed models. We could consider adding an argument if we think the users should have a choice in this later on.

I propose we use mix_simulate() for the function you wrote.

Let's use this for now. I am happy to make it more like your single-species case if there is demand for it.

I edited it allow customisation in terms

Cool!

@hrlai hrlai added the enhancement New feature or request label Sep 5, 2020
@xp-song
Copy link
Owner

xp-song commented Sep 6, 2020

This is implicitly implemented as a default for loglog or expo mixed models. We could consider adding an argument if we think the users should have a choice in this later on.

I just pushed my latest changes, where I separated the functions mix_simulate() into mix_predict() and mix_simulate(), to allow user to make predictions using mixed-effects model for any other dataset. This is consistent with the naming for the single-species model functions.

I think the package is more or less ready as a first version! Other than the possibility of adding mix_modelfit() and maybe a separate vignette for the mixed-effects models, are there any outstanding issues you'd like to address?

@hrlai
Copy link
Collaborator Author

hrlai commented Sep 8, 2020

Nope! I suggest that we release a beta version (say v0.1) with the accepted manuscript. And then I will spend some time after that adding mix_modelfit() and more vignettes. We can note upcoming version changes in the website. Are you ok with this? Just that I need to now prioritise some other projects before returning to this.

@xp-song
Copy link
Owner

xp-song commented Sep 8, 2020

Thanks and no problem! I've changed the version to 0.1.0 as suggested (FYI this is a good reference on releasing a package).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants