Skip to content
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

Add attributes to arrivals #16

Closed
Enchufa2 opened this issue Nov 20, 2015 · 18 comments
Closed

Add attributes to arrivals #16

Enchufa2 opened this issue Nov 20, 2015 · 18 comments
Milestone

Comments

@Enchufa2
Copy link
Member

Probably, it is more efficient (and more easy to use) to integrate this attributes in the C++ core. Use case provided by @bart6114:

t0 <- create_trajectory("trauma patient") %>%
  set_attribute(health_status=function() runif(1, .05, .3)) %>%
  seize("doctor", 1) %>%
  timeout(function() rnorm(1,15)) %>%
  release("doctor", 1) %>%
  set_attribute(health_status=
                       get_attribute("health_status") + 
                       runif(1, .05, .3))

Possibilities:

  • A list attached to the activities
  • Attributes for each arrival
  • ...?
@bart6114
Copy link
Member

I'm a big proponent of this. Ideally the get_attribute function should be acccessible everywhere in the trajectory definition. E.g. one could set an attribute on which you base a resource seize; e.g.

​t0 <- create_trajectory("trauma patient") %>%
  set_attribute("number_nurses_required", function() sample(1:3, 1)) %>%
  seize("nurse", get_attribute("number_nurses_required")) %>%
  timeout(function() rnorm(1,15)) %>%
  release("nurse", get_attribute("number_nurses_required"))

ps. the set_attribute("health_status", function() runif(1, .05, .3)) syntax is probably more in line with the rest of the code (as opposed to the original use-case)

@bart6114
Copy link
Member

To kick this off, a very preliminary implementation can be found in e3581ba (dev-bart branch). Check the bottom of the __working_file.R for a test run. Haven't check the impact performance wise (which I fear for a bit)

@Enchufa2
Copy link
Member Author

I set aside some time tonight for closing #22, because it required little effort, but I probably won't have time to spend in this issue until not this weekend but the next.

@bart6114
Copy link
Member

Worked on it a bit more (42d8e37), this is now possible:

t0 <- create_trajectory() %>%
  set_attribute("health", function() sample(40:80,1)) %>%
  set_attribute("nurses_to_seize", 
                function(attrs){
                  if(attrs[["health"]]<50) 2
                  else 1
                }, provide_attrs = TRUE) %>%
  seize("nurse", 
        function(attrs){
          attrs[["nurses_to_seize"]]
        }, provide_attrs = T) %>%
  timeout(function(attrs){(100 - attrs[["health"]])},
          provide_attrs = T) %>%
  set_attribute("health", 
                function(attrs){
                  min(attrs[["health"]] + sample(attrs[["health"]]:100, 1), 100)},
                provide_attrs=TRUE) %>%
   release("nurse", 
        function(attrs){
          attrs[["nurses_to_seize"]]
        }, provide_attrs = T) %>%

  ## some other functionality
  ## simply print the attrs using a 0 timeout
  timeout(function(attrs){print(attrs); 0}, provide_attrs=T) %>%
  timeout(function() 0) %>% #  if provide_attrs=F no attrs are passed and minimal overhead is incurred
  timeout(0) # doesn't have to be a function (will be converted to one)

env<-simmer() %>%
  add_generator("test", t0, at(seq(0,1000,200))) %>%
  add_resource("nurse", 2) %>%
  run()

Passing the attributes incurs minimal overhead (because its optional). However, I've altered the release and seize methods to allow access to the attributes. If not using the attributes in a seize/release (and simply passing an integer) you will incur a minimal overhead due to the fact that the integer is coerced into a function that returns said integer...

Have a look at it when you have some time (no rush :))

@Enchufa2
Copy link
Member Author

Looks great! One question after a quick glance: why is the member Arrival::attributes a pointer? If this is not needed, it is much faster and easier to manage a static allocation.

@Enchufa2
Copy link
Member Author

And it would be great to benchmark the overhead of providing the attributes to decide whether the provide_attrs argument is worthwhile. Because, there isn't a way of automatically checking how many arguments an R function needs, isn't it? Maybe using deparse or something like that?

@bart6114
Copy link
Member

No specific reason why it is a pointer; I quickly started out that way and didn't look back.

So, what I can do is test if a supplied function has a parameter and only if it has one set the provide_attrs to TRUE automatically. This way the end user doesn't have to bother with it and we only incur the overhead when attributes are requested.

Will work some more on it later

@bart6114
Copy link
Member

Arrival::attributes is now staticly allocated and provide_attrs doesn't have to be supplied anymore. If a function in the form of function(param){} is supplied provide_attrs is inferred as TRUE, else if a function without parameter is passed, it is inferred as FALSE.

@bart6114
Copy link
Member

So the above example is now simply:

t0 <- create_trajectory() %>%
  set_attribute("health", function() sample(40:80,1)) %>%
  set_attribute("nurses_to_seize", 
                function(attrs){
                  if(attrs[["health"]]<50) 2
                  else 1
                }) %>%
  seize("nurse", 
        function(attrs){attrs[["nurses_to_seize"]]}) %>%
  timeout(function(attrs){(100 - attrs[["health"]])}) %>%
  set_attribute("health", 
                function(attrs){
                  min(attrs[["health"]] + sample(attrs[["health"]]:100, 1), 100)}) %>%
   release("nurse", 
        function(attrs){attrs[["nurses_to_seize"]]})

env<-simmer() %>%
  add_generator("test", t0, at(seq(0,1000,200))) %>%
  add_resource("nurse", 2) %>%
  run()

@Enchufa2
Copy link
Member Author

I've just discovered the inline comments inside commits. 😄 I've dropped some comments there. And, apart from that, have you thought about how to monitor the attributes (where and how to retrieve the time series of values)?

@bart6114
Copy link
Member

Thanks for the comments, I've pushed another commit in response to them.

With regards to monitoring the attributes, do they have to be monitored? Or would it be enough to simply be able to extract the values as they are at the moment the simulation stops. (At the moment) this would be enough for my use-cases, and can be implemented quite easily. I.e. if you want to store something along the way, you can simply store it in an (extra) attribute that you don't change after that. What do you think?

@Enchufa2
Copy link
Member Author

I asked because I haven't thought about it. Since the programmer can monitor these attributes, we may put this feature in standby for now. But if we come up with use cases that make a heavy use of attribute statistics, we should implement it inside the C++ core in order to do it efficiently.

@Enchufa2
Copy link
Member Author

I've made some changes (check b23471b, please) in order to allow numeric input values into seize, release, timeout and set_attribute without wrap them into a function (to be more efficient in simple cases).

@bart6114
Copy link
Member

Nice one!!

Too bad for the "gc" initialization, but I don't see an easy way around this (without making the code less readable)

@Enchufa2
Copy link
Member Author

Yeap, it's a dirty trick. It doesn't compile if you don't initialise those functions. We need to figure out if there is an alternative, a NULL function or something.

@Enchufa2
Copy link
Member Author

Comprehensive attribute statistics gathering added (check ba07e23, please). Let's suppose a simple trajectory:

t0 <- create_trajectory() %>% set_attribute("dummy", 1)

These statistics are optional, so if you do the following:

env <- simmer() %>%
  add_generator("test", t0, at(seq(0,1000,200))) %>%
  run()

attributes <- env %>% get_mon_attributes()
head(attributes)
#> [1] time  name  key   value
#> <0 rows> (or 0-length row.names)

then mon=1 by default and attributes are empty. But if you increment the monitoring level to mon=2 (or more):

env <- simmer() %>%
  add_generator("test", t0, at(seq(0,1000,200)), mon=2) %>%
  run()

attributes <- env %>% get_mon_attributes()
head(attributes)
#>   time  name   key value
#> 1    0 test0 dummy     1
#> 2  200 test1 dummy     1
#> 3  400 test2 dummy     1
#> 4  600 test3 dummy     1
#> 5  800 test4 dummy     1

More unit tests are needed to cover the new functionalities and probably a plot_attributes function or something like that as a helper. I have run out of time for now.

@bart6114
Copy link
Member

nice addition 👍
I've added a preliminary plot_attributes function. I'll try to work a bit on documentation tomorrow and send you a mail later.

Enchufa2 added a commit that referenced this issue Nov 30, 2015
Merge latest developments (originating from #16) into master
@Enchufa2
Copy link
Member Author

Enchufa2 commented Dec 1, 2015

Note: the dirty "gc" initialization is gone with ba2b5fa. 👍

@Enchufa2 Enchufa2 added this to the v3.1.0 milestone Dec 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants