aemon-j / LakeEnsemblR

An R package that facilitates multi-model ensembles for lake thermodynamics. Also includes tools for calibration, sensitivity analysis and data visualization.
GNU General Public License v2.0
32 stars 19 forks source link

Perhaps put model package in suggests instead of imports? #183

Open jordansread opened 4 years ago

jordansread commented 4 years ago

I think it is excellent to support mac/nix/win right now, but worry about a couple scenarios where it may be limiting to put all of the model packages (the current ones and future models) in imports. Doing so requires that users are able to successfully install all models, but what if they only want to use LER for the helpful tools for one or two?

Likewise, our supercomputing at USGS is still not able to install GLM3r because of some odd library issues, so I presently can't use LER there for the other models w/o creating my own fork and changing around the installations.

I realize this is potentially an odd question, but I think it might be worth a little bit of discussion, especially since all of the installs in imports are quite hefty. I'm also assuming that LER might be gaining models in the future, and perhaps some of those models would be impossible to support for all OS's.

jorritmesman commented 4 years ago

This is a very good suggestion! We need to think as well how to facilitate additions of new model versions, and how we catch errors if people try to run a model that they haven't installed the (right) package for (we have a check_models function, so maybe there). We're working on it

jordansread commented 4 years ago

I’d be happy to help work on the strategy and code to support this kind of thing if the group is interested.

tadhg-moore commented 4 years ago

I agree, this makes sense and will make it easier for installation for users who just want to use the features from one or two models. We could try to trim out some of the packages in Imports that are not so neccessary e.g. lintr, reshape2/plyr/dplyr

Would the best strategy be to just place them in Suggests or will we need to re-design the code to check for the packages in specific functions?

JFeldbauer commented 4 years ago

I started to work on this issue and created a branch on my github https://github.com/JFeldbauer/LakeEnsemblR/tree/no_model_dependency which can now be installed without having to install the model libraries. But we still have some dependencies on the glmtools and gotmtools packages in LER and they both require the corresponding model, so not all tools of LER will work in this version.

jorritmesman commented 4 years ago

With the latest version of gotmtools (5 minutes ago), gotmtools should be independent. However, glmtools still relies on GLMr. We'll have to see if this is a problem (i.e. if glmtools is only used for GLM-things inside LER, or also in more general ways).

jorritmesman commented 4 years ago

We have made the following changes (still on Johannes' and my personal Github repo's):

The only remaining issue is regarding glmtools. glmtools depends on GLMr (not GLM3r, which we use to run GLM). In LER, glmtools is used for the functions get_ice, get_nml_value, get_var, read_nml, set_nml, and write_nml, in relatively many places and not only for GLM-related code (e.g. FLake also uses .nml files, and we use glmtools functions for it).

We had a look at the glmtools code. As far as we can see, GLMr is only used in the examples and in read_nml (in case the user inserts "template", the template in GLMr is used).

So this leaves us two options: 1) either the dependency of glmtools on GLMr needs to be removed, or 2) we need to copy the nml functions from glmtools into LER. @jread-usgs , since you are the main contributor of the glmtools package, what would you prefer? Do you think that moving GLMr to "Suggests" in glmtools is desirable?

JFeldbauer commented 4 years ago

I would just like to add that the ggplot-overhaul branch of glmtools uses GLM3r, but as far as I can see uses it at the same places and for the same reasons as GLMr is used in the master branch.

jordansread commented 4 years ago

I think for the same reasons I suggested it here that GLMr or GLM3r should also be "suggests" for glmtools. You can still use the package to interact with outputs created from running the model in a different way.

tadhg-moore commented 3 years ago

I just checked both branches of glmtools and the GLMr and GLM3r are still in Imports instead of Suggests, @jread-usgs would you be able to make these chances in glmtools and then we can close this issue?

jordansread commented 3 years ago

I think so @tadhg-moore I just need to take a closer look to make sure there aren't additional implications to those packages. Probably a little bit more effort than just changing the DESCRIPTION file since I probably need to adjust the imports for the relevant packages.