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

Progress bar #150

Closed
mattwigway opened this issue Feb 24, 2021 · 12 comments
Closed

Progress bar #150

mattwigway opened this issue Feb 24, 2021 · 12 comments
Assignees
Labels

Comments

@mattwigway
Copy link
Contributor

A progress bar for the travel_time_matrix function would be very handy for larger runs, and not too difficult to implement (it would have to be on the Java side). There's some tricks with safe multithreading, but not too bad - I'm happy to take this on myself.

@rafapereirabr
Copy link
Member

That would be great, Matt! Do you think this something that can be added to our Jar file https://github.com/ipeaGIT/r5r/blob/master/r-package/inst/jar/r5r_0_3_0.jar ?

@mvpsaraiva
Copy link
Collaborator

Thanks Matt. I had planned to add some simple progress info like printing "processing origin x out of y" in the console, but a progress bar would be much better. I actually don't know how to do this in Java without interfering with R5's outputs in the console.
Feel free to ask me anything if you need help navigating our Java code.

@mattwigway
Copy link
Contributor Author

Great. I probably won't work on this for a month or so, but putting some implementation notes here so I remember them then:

  1. The \r character (carriage return) is what is needed for implementing progress bars
  2. To get the log messages to work, define a special java logging handler for the messages that prints enough spaces to clear out the progress bar, \r, log message, redraw progress bar. That way the progress bar will always be at the bottom.

@rafapereirabr
Copy link
Member

Hi @mattwigway! Have you made any progress on this issue?

mvpsaraiva added a commit that referenced this issue May 27, 2021
…verbose = FALSE` (Issue #150).

Small tweaks in the documentation.
@mvpsaraiva
Copy link
Collaborator

I've added a quite simple implementation of this feature in the dev branch, here. It's only activated when verbose = FALSE, so it doesn't interfere with R5's log messages.

@rafapereirabr
Copy link
Member

Thanks, @mvpsaraiva . I'm wondering if this could this be slowing r5r down, though. I had the impression it's slower after quick test

@mvpsaraiva
Copy link
Collaborator

Technically, it has to slow things down a bit, because it needs to synchronise the counter between all threads.

I've ran it 3 times, because R5 gets faster over time. Here are the results:

Run With Progress Without Progress
1 14.841 12.138
2 12.551 9.026
3 12.471 8.672

@rafapereirabr
Copy link
Member

Hm.... so showing the counter can make computation process 20% to 40% slower. This seems a lot. @mattwigway , any suggestions here?

@mattwigway
Copy link
Contributor Author

mattwigway commented May 28, 2021 via email

@mvpsaraiva
Copy link
Collaborator

I've suspected the 20% to 40% slow down was too much, and probably due to the small test area (r5r's Porto Alegre sample dataset), then I've run some more tests on an entire city (Belo Horizonte). I've also looked at LambdaCounter as @mattwigway suggested, and implemented versions that show progress at intervals and as percents.

Run No Progress Progress Count Progress Interval Progress %
1 28.236 28.571 32.419 31.697
2 25.971 27.722 26.299 27.140
3 25.300 26.306 24.464 26.985

There are a few differences between the runs, but they're probably more due to other things than the progress method.I think it's probably fine to use any of them.

I think the large overhead of the previous test was because, in a small study area, it's more likely that more threads finish computing travel times at once and try to update the counter simultaneously, causing a slow down. In larger study areas, threads are more likely to complete tasks asynchronously and access the counter at different times.

@rafapereirabr
Copy link
Member

Setting this issue as priority, to be incorporated in the next package release.

@rafapereirabr
Copy link
Member

rafapereirabr commented Jul 2, 2021

Hi both. I've updated the documentation of the verbose to include information about the progress counter. This is the proposed documentation.

#' @param verbose logical. `TRUE` to show detailed output messages (the default).
#'                If verbose is set to `FALSE`, r5r prints a progress counter and
#'                eventual `ERROR` messages. Setting `verbose` to  `FALSE` imposes
#'                a small penalty for computation efficiency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants