simChef icon indicating copy to clipboard operation
simChef copied to clipboard

`create_sim`: functionality and naming consistency

Open jpdunc23 opened this issue 2 years ago • 2 comments

  • Naming: We have many create_* functions that create R6 objects like the Experiment, DGP, and Method classes. On the other hand, we used init_docs() for setting up the Rmd documentation directory structure. I think renaming create_sim() to init_sim() would improve our API consistency. Another point of confusion is the hpc argument, which could mislead users into thinking that with hpc = TRUE their simulation will automatically work in a distributed computing environment (e.g., a Slurm cluster). I think we should rename this argument somehow to make it a bit more clear. Maybe scripts_dir (and also rename tests to tests_dir / add a logs_dir arg)? Another option (my preference) is to replace the hpc and tests args with an init_dirs argument that accepts string vectors, default c("logs", "scripts", "tests").
  • Functionality: Can we have create_sim() optionally call init_docs() ? E.g., we could add a boolean argument init_docs.

Maybe create_sim() and init_docs() should be combined, somehow?

jpdunc23 avatar Jan 15 '24 20:01 jpdunc23

@PhilBoileau @tiffanymtang thoughts?

jpdunc23 avatar Jan 15 '24 20:01 jpdunc23

  • Naming: We have many create_* functions that create R6 objects like the Experiment, DGP, and Method classes. On the other hand, we used init_docs() for setting up the Rmd documentation directory structure. I think renaming create_sim() to init_sim() would improve our API consistency.

I like the idea of renaming create_sim() to init_sim().

Another point of confusion is the hpc argument, which could mislead users into thinking that with hpc = TRUE their simulation will automatically work in a distributed computing environment (e.g., a Slurm cluster). I think we should rename this argument somehow to make it a bit more clear. Maybe scripts_dir (and also rename tests to tests_dir / add a logs_dir arg)? Another option (my preference) is to replace the hpc and tests args with an init_dirs argument that accepts string vectors, default c("logs", "scripts", "tests").

Good points. Adding the described init_dirs sounds good to me.

  • Functionality: Can we have create_sim() optionally call init_docs() ? E.g., we could add a boolean argument init_docs.

Maybe create_sim() and init_docs() should be combined, somehow?

I'm not sure if this is very natural. Currently, init_docs initializes the documentation for a simulation experiment created with create_experiment(). I think create_sim() initializes the directory structure for the overarching "project" or "study", which may consist of multiple simulation experiments. In some sense, maybe we should consider renaming create_sim() to init_sim_study() or init_study() or something of the like.

tiffanymtang avatar Feb 01 '24 18:02 tiffanymtang