New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resource monitoring: before or after seize/release #28

Closed
Enchufa2 opened this Issue Dec 3, 2015 · 9 comments

Comments

Projects
None yet
2 participants
@Enchufa2
Member

Enchufa2 commented Dec 3, 2015

This is just a reminder for me to think about it, but also a discussion forum for throwing pros/cons and, eventually, deciding. Wrapping up:

  • Before: The time diffs are aligned when calculating statistics, but we lose the last observation.
  • After: The time diffs are misaligned and some reengineering is needed to get the right statistics, but we lose the first observation.

@Enchufa2 Enchufa2 added the question label Dec 3, 2015

@Bart6114

This comment has been minimized.

Member

Bart6114 commented Dec 4, 2015

my 2¢

the end user should have access to all activity stats (this includes the last observation). imo this puts us in the after scenario (where some reengineering has to realign the vectors).

@Enchufa2

This comment has been minimized.

Member

Enchufa2 commented Dec 4, 2015

On the other hand, one could argue that the after scenario deprives the user of the first observation... 😉

@Enchufa2

This comment has been minimized.

Member

Enchufa2 commented Dec 5, 2015

Let's bring some code to the discussion. This is what we are doing now:

mutate(mean = cumsum(value * diff(c(0,time))) / time)

And I recovered what we were doing when monitoring was performed after:

mutate(mean = c(0, cumsum(head(value,-1) * diff(time)) / tail(time,-1)))

So the reengineering consisted of removing the last observation! 😄

@Bart6114

This comment has been minimized.

Member

Bart6114 commented Dec 8, 2015

indeed 😬

something related:

library(simmer)

t0<-create_trajectory() %>%
  seize("server", 2) %>%
  timeout(5) %>%
  release("server")

env<-
simmer() %>%
  add_resource("server", 1) %>%
  add_generator("test", t0, at(0)) %>%
  run()

get_mon_arrivals(env)

What would be the expected/desired output in this case? Currently it returns:

[1] name          start_time    end_time      activity_time finished     
<0 rows> (or 0-length row.names)

Intuitively I would expect one row were finished is FALSE.

@Enchufa2

This comment has been minimized.

Member

Enchufa2 commented Dec 8, 2015

Arrival reporting is executed when an arrival leaves the trajectory: because 1) it has reached the end or 2) it was rejected. In this case, the arrival is requesting 2 units when the resource has only 1, so the arrival will be enqueued forever. But it has not finished its lifetime, so nothing is reported.

This is a consequence of the very flexibility that the trajectory-based seize/release system offers: the programmer is responsible for creating well-formed trajectories, and simply this is not.

@Bart6114

This comment has been minimized.

Member

Bart6114 commented Dec 8, 2015

I understand the reasoning behind it, just adding to the discussion (trying to speak from the side of the end-user 😃 ).

@Enchufa2

This comment has been minimized.

Member

Enchufa2 commented Dec 8, 2015

This is interesting as a separate question (#36 opened), but I think it is not important for this discussion (the resource will never be seized).

@Enchufa2 Enchufa2 modified the milestone: v3.1.0 Dec 8, 2015

@Enchufa2 Enchufa2 added this to the v3.2.0 milestone Feb 20, 2016

@Enchufa2

This comment has been minimized.

Member

Enchufa2 commented Feb 20, 2016

After giving much thought, I think that probably you are right: the user would expect an observation to be the snapshot of that instant, not of the last interval. The definitive argument that I've found in favour of your point is the behaviour of stepfun().

So, I'll perform the monitoring after, starting from the next release. This requires a minor version change and a careful explanation.

@Enchufa2 Enchufa2 closed this in 9db5c66 Feb 20, 2016

@Bart6114

This comment has been minimized.

Member

Bart6114 commented Feb 22, 2016

great 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment