Changed extract_estimates argument order so the output from run() can…

[Tess-LaCoil]

… be piped into the first arg as a fit.

Also updated testing for the function to reflect this structure.

While it's not strictly optimal to do because this workflow doesn't save the fit for diagnostics it is useful.

[fontikar]

While it's not strictly optimal to do because this workflow doesn't save the fit for diagnostics it is useful.

Maybe worth thinking about whether we want to make so easy for users to extract estimates without looking at the fit first... carefully...we don't want to encourage sub-par user behaviour right?!

You can still pipe run to extract_estimates() using the placement of the . operator

See the solution here for explainer of the . https://stackoverflow.com/questions/54815607/r-combinations-with-dot-and-pipe-operator

[Tess-LaCoil]

The direct pipe of fit-to-extract is something I've wanted for when I'm doing a lot of simulation stuff.
If I'm understanding the forum thread correctly, the following would be the relevant workflow with the old order of args?

# Constant function chosen as Step 1
trout_constant_ests <- hmde_model("constant_multi_ind") |> #Step 2
  hmde_assign_data(data = Trout_Size_Data)  |> #Step 3
  hmde_run(chains = 4, cores = 4, iter = 2000) |> #Step 4
  hmde_extract_estimates(model = "constant_multi_ind",
  fit = . ,
  input_measurement_data = Trout_Size_Data)

[fontikar]

The direct pipe of fit-to-extract is something I've wanted for when I'm doing a lot of simulation stuff. If I'm understanding the forum thread correctly, the following would be the relevant workflow with the old order of args?

# Constant function chosen as Step 1
trout_constant_ests <- hmde_model("constant_multi_ind") |> #Step 2
  hmde_assign_data(data = Trout_Size_Data)  |> #Step 3
  hmde_run(chains = 4, cores = 4, iter = 2000) |> #Step 4
  hmde_extract_estimates(model = "constant_multi_ind",
  fit = . ,
  input_measurement_data = Trout_Size_Data)

Gahhh, it doesn't seem like the '.' passing method working for some reason

If you use the version of hdme on master

library(hmde)

# Constant function chosen as Step 1
trout_constant_ests <- hmde_model("constant_multi_ind") |> #Step 2
  hmde_assign_data(data = Trout_Size_Data)  |> #Step 3
  hmde_run(chains = 4, cores = 4, iter = 2000) |> #Step 4
  hmde_extract_estimates(fit = .,
                         model = "constant_multi_ind",
                         input_measurement_data = Trout_Size_Data)

Returns an error...

Error in hmde_extract_estimates(hmde_run(hmde_assign_data(hmde_model("constant_multi_ind"),  : 
 unused argument (hmde_run(hmde_assign_data(hmde_model("constant_multi_ind"), data = Trout_Size_Data), chains = 4, cores = 4, iter = 2000))

@dfalster do you have any ideas. Is it probably pedantic of me to be hung up on this... in theory the dot would be the same as the changes implemented in this PR...

[dfalster]

A small comment, . doesn't work with native pipe |>, only with maggrittr pipe %>%. Looks like _ is the native pipe equivalent, provided argument is named,

https://www.tidyverse.org/blog/2023/04/base-vs-magrittr-pipe/

[fontikar]

A small comment, . doesn't work with native pipe |>, only with maggrittr pipe %>%. Looks like _ is the native pipe equivalent, provided argument is named,

https://www.tidyverse.org/blog/2023/04/base-vs-magrittr-pipe/

Aha! 💡 works splendidly.

install_github("traitecoevo/hmde")

library(hmde)

trout_constant_ests <- hmde_model("constant_multi_ind") |> #Step 2
  hmde_assign_data(data = Trout_Size_Data)  |> #Step 3
  hmde_run(chains = 4, cores = 4, iter = 2000) |> #Step 4
  hmde_extract_estimates(model = "constant_multi_ind",
                         fit = _ ,
                         input_measurement_data = Trout_Size_Data)

With the interest of consistency and ordering of arguments for users and promoting good diagnostic practices. My feeling is leave the code base as it and amend your simulation code as above using _ to pass the fit to the next function.

If the package was for personal use and strictly for Tess's thesis, I would have approved the PR in a heartbeat, but also would be interested to hear @dfalster opinion :)

This reminds me a little of austraits::load_function() where we had a default argument to load the latest version of the database, but after thinking about reproducible practices, we decided not to make it too easy for users and built in error messages to for users to specific a version number.

[Tess-LaCoil]

Dan and I just had a chat about this. His suggestion was to change the parameter order so it's an easy pipe when I want it but not talk about it in the paper and instead have the break in workflow to save the fit.

We also discussed using an attribute imposed on the Stan fit object in hmde_run in order to avoid having to write the parameter name in as an arg for extract_estimates at all. I've never tried to do this, but it does seem like a neater way to go?

[fontikar]

Love the idea of a class attribute! Safer too if your user knows how to fiddle with code! Let me know if you want a primer on this :)

We also discussed using an attribute imposed on the Stan fit object in hmde_run in order to avoid having to write the parameter name in as an arg for extract_estimates at all. I've never tried to do this, but it does seem like a neater way to go?

I don't see the paramater name as an arg for extract_estimates maybe I am missing something!

[Tess-LaCoil]

Oh I'm a dingus, model name not parameter name. And yeah, can you talk me through the attributes stuff? Because the run function returns a stan fit object it works directly with the various things that have been built to handle those. Dan said that an attribute is probably the best way to retain all of the stan fit structure and uses while also storing the meta-data about the model.