create R wrapper for bgcExecute() to pass default args via R

PredictiveEcology / BiomeBGCR

Package for running Biome-BGC models in R

2 stars 0 forks source link

create R wrapper for bgcExecute() to pass default args via R #3

Closed achubaty closed 2 years ago

achubaty commented 2 years ago

rename bgcExecute() ==> .bgcExecute() and make internal
create bgcExecute() as a wrapper for .bgcExecute(), passing default argv = "-a" (others?)
@cboisvenue @jflavoie33 does it make sense to add a param spinup (logical) to this wrapper, or possibly create a similar wrapper to explicitly run the spinup (e.g., bgcSpinup()) ?

jflavoie33 commented 2 years ago

I'm not sure about the spinup param / method call because at the bgclib level, the only thing that makes the go and spinup phases different is the provided ini file. Maybe @cboisvenue can add some more insight

cboisvenue commented 2 years ago

The only differences (as far as I know/can tell) between a go INI file and a spinup INI file is the RESTART block, the OUTPUT_CONTROL block, and the DAIL_OUTPUT block. So, in my view, have a "toggle" or a logical parameter that can control the type of run would be a very useful tool. The INI file of the spinup has 0 in the two *_OUTPUT blocks and has c(0,1,1) instead of c(0,1,1). Let me know if there are other concerns about making the control of the type of run (go or spinup) a logical parameter.

jflavoie33 commented 2 years ago

Implemented in commit 1ad6f6ce9f59f1cc0e99232cb8b6a2b261ed21d3. However, I did not find how to make the bgcExecuteInternal function "internal" as desired and it is still exported by the package. @achubaty do you know a simple way to do this ? Not closing because not complete yet.

achubaty commented 2 years ago

to keep it internal, don't export it.

I removed [[Rcpp::export]] added @keyword internal and re-documented the package in 9d6279c75d46348965f8ce4c8376a0fce0e2aa2b.

achubaty commented 2 years ago

Ahh apparently need to keep the Rcpp::export so R can use that function. Sorry about that.

It's not exported by R, so does this achieve what you were hoping?

jflavoie33 commented 2 years ago

Yes this is what was needed. Thanks. Closing as everything seems done concerning this.