Allow zero-action trajectories to be valid.

[mtbakerguy]

I have a contrived example below to illustrate the issue. If you override log_ to make debug output tunable, you can end up with a situation where you get a surprising error--"Error: add_generator: 'trajectory' is not a valid trajectory." After troubleshooting for too long, it became clear what was happening--a trajectory must have a single action and, as a result, adding an artificial timeout(0) creates a valid trajectory. How did I run into this in practice? I had a branch in my trajectory where one of the outcomes was a single log_ call showing the condition that had occurred.

Thoughts (unordered) on what to do:

• Update an appropriate vignette that shows this override mechanism and how to workaround the 1 action with debugging converts to 0 actions without issue.
• Add a call(.trj,,...) mechanism with a callback mechanism that would allow someone to have a valid action. Beyond allowing someone to override log, it would also be a natural place for the case where someone calls timeout() with a callback that updates some internal state and always returns 0.
• Similar to the syslog interface, add an optional modifier to log_ to allow for levels of debugging.
• Add a hack to log_ where a function returning an empty string transparently prints nothing. I dislike this one enough that I almost didn't write it down but it is a defensible option.

There might also be an option with inheritance but I've never used R's OO capabilities so it's not clear to me if that's a potential method (pun intended) as well.

library(simmer)

t1 <- function(log=log_,work=TRUE) {
    t <- trajectory() %>% log(paste("T1","starting"))
    if(work)
        t %>% timeout(0)
    t
}

t2 <- function(log=log_,work=TRUE) {
    t <- trajectory() %>% log(paste("T2","starting"))
    if(work)
        t %>% timeout(0)
    t
}

runmerge <- function(log=log_,work=TRUE) {
    env <- simmer() %>% add_generator("T1",t1(log,work),at(0)) %>%
                        add_generator("T2",t2(log,work),at(0)) %>%
                        run() %>% invisible
}

runmerge_quiet_works <- function () runmerge(function(.trj,message) .trj)
runmerge_quiet <- function () runmerge(function(.trj,message) .trj,FALSE)

print('Runmerge with output')
runmerge()
print('Runmerge without output and a timeout of 0 to please validation')
runmerge_quiet_works()
print('Runmerge without output and no timeout')
runmerge_quiet()

[Enchufa2]

Can you please edit your comment and add backticks and a proper indentation to create legible code blocks? See this article.

[Enchufa2]

Comments on your thoughts:

• Update an appropriate vignette that shows this override mechanism and how to workaround the 1 action with debugging converts to 0 actions without issue.

I don't think that vignettes should be a place for workarounds.

• Add a call(.trj,,...) mechanism with a callback mechanism that would allow someone to have a valid action. Beyond allowing someone to override log, it would also be a natural place for the case where someone calls timeout() with a callback that updates some internal state and always returns 0.

The timeout() is the natural place for updating things, either with some delay or instantaneously. Actually, the timeout() argument is called task to highlight this fact.

• Similar to the syslog interface, add an optional modifier to log_ to allow for levels of debugging.

I prefer not to complicate the log_() method, especially since it's possible to implement this functionality on top of it easily.

• Add a hack to log_ where a function returning an empty string transparently prints nothing. I dislike this one enough that I almost didn't write it down but it is a defensible option.

Too hackish. I don't like it either.

One thing I always wanted to be able to do is the following to write very quick examples or tests:

simmer() %>%
  add_generator("dummy", trajectory(), at(0)) %>%
  run()

But I found no other advantage, so I never changed it. What you are describing is a good use case though. So this is the excuse I was looking for to invest the required effort to allow empty trajectories. And if I'm right, this would solve your issue.

The empty-trajectory branch in this repository is a first try on this. Can you test it, please?

[mtbakerguy]

First off, thanks for putting together an improvement that fits with the principal of least astonishment as allowing an empty trajectory would elegantly solve this problem. Likewise, I appreciate the pointer to the article on code formatting/highlighting as I hadn't done that before.

I looked into testing the code changes but it's unclear to me how I would get started with compilation, local installation, and reverting to the released version when complete. In any case, I would just use the above code anyway to do the following:

Before Output

Rscript log_override.r 
[1] "Runmerge with output"
0: T10: T1 starting
0: T20: T2 starting
[1] "Runmerge without output and an artificial timeout of 0 to please validation"
[1] "Runmerge without output and no artificial timeout"
Error: add_generator: 'trajectory' is not a valid trajectory
Execution halted

After Output

Rscript s.r 
[1] "Runmerge with output"
0: T10: T1 starting
0: T20: T2 starting
[1] "Runmerge without output and an artificial timeout of 0 to please validation"
[1] "Runmerge without output and no artificial timeout"

Finally, I did take a look at your two commits and the changes to the unit-tests as well as utils.R look correct to me.

[Enchufa2]

Are you a Windows, Linux or MacOS user?

[mtbakerguy]

OSX.

[Enchufa2]

Then, FYI, to install packages from source, you need the development tools (see R for Mac OS X). Then, you can install the empty-trajectory branch as follows (uncomment the first line if you don't have the remotes package installed):

# install.packages("remotes")
remotes::install_github("r-simmer/simmer", ref="empty-trajectory")

Otherwise, I built the binary package for you using the rhub builder. You can download it from my Keybase public folder here and install it.

[Enchufa2]

@mtbakerguy FYI, binary package updated, including #151 and #152, just in case you want to start using these new features before the next release, which will be rolled out in one month.

[mtbakerguy]

Thanks for the fixups. I appreciate your responsiveness.