console-rs / indicatif

A command line progress reporting library for Rust
MIT License
4.36k stars 241 forks source link

Slower refresh than 1hz? #452

Open mpetri opened 2 years ago

mpetri commented 2 years ago

Previously I could refresh progress once every 5 seconds for example. Now the lowest refresh rate seems to be once per second with https://docs.rs/indicatif/latest/indicatif/struct.ProgressDrawTarget.html#method.stdout_with_hz and stdout_with_hz(1).

Is there a way to achieve this currently?

djc commented 2 years ago

You can't. Why would you want to?

mpetri commented 2 years ago

For progress bars with many items that run over many hours I find it esthetically more pleasing to not update so frequently. I used to just use the draw_delta to achieve the desired update rate relative to the number of items being processed.

mpetri commented 2 years ago

Another reason for this would be that it provides smoother estimates around ETC when iterating over elems where processing times fluctuate a bit. Currently I do once per sec updates and ETA jumps from 11hours to 4 hours all the time. With a 5 or 10 sec update rate this would be smoothed out to provide a more accurate estimate.

djc commented 2 years ago

Are the estimates worse than before if you change the refresh rate in the old version?

Walther commented 2 years ago

Wasn't entirely sure whether to open a new issue, decided to comment here on an existing issue.

After upgrading to 0.17, in my use case the ETA became effectively useless. Two issues:

  1. The numbers jump around way too fast to be readable
  2. As mpetri mentioned above, their value range jumps around way too much to give useful information. On a task that takes approx 3 minutes, the eta_precise keeps jumping around values between less than one minute and over 10 minutes, never converging to a useful mostly-decreasing 3min value.

https://user-images.githubusercontent.com/2943750/184963629-0a944bdf-17ce-4a4d-a708-b54b96e26179.mov

Here's a screen recording of 0.16 with a bar.set_draw_delta():

https://user-images.githubusercontent.com/2943750/184965122-f5b037b7-6d3d-4c3a-8edb-47a9a538c51c.mov

Compared to 0.16,

  1. The numbers don't twitch around as much, making it much more readable
  2. The ETA stays mostly convergent and mostly monotonically decreasing

Even if the accuracy could be better here (that's one long 30 seconds, heh), 0.16 had better UX on those two aspects.

_(Pre-empting the possible suggestion of using eta instead of eta_precise: sadly due to the range issue (2), it too became pretty unreadable (1).)_


EDIT:

The 0.17 video above was recorded with no limiting. I also made a test run with bar.set_draw_target(ProgressDrawTarget::stdout_with_hz(1)); and while it solves some of the readability issues (1), it leaves the jumpy values problem (2) - at every tick, the value can jump pretty wildly compared to the 0.16 behavior. Something is different regarding the convergence between the versions 🤔

https://user-images.githubusercontent.com/2943750/184972230-d54e0aaa-0d36-4fbe-b8e7-c3fe036b70c9.mov

djc commented 2 years ago

@Walther thanks for the report. I think this is a separate issue, as previously discussed in https://github.com/console-rs/indicatif/issues/394. Please reopen that issue or open a new one. If you can do a bisection of the regression you're seeing, that would be great.

mpetri commented 2 years ago

Given the amazing videos @Walther posted I tried to capture my problem as well in this video:

https://user-images.githubusercontent.com/291755/185441298-2b13b697-7863-46e1-b713-82aaa4c3c46c.mp4

which was created with:

  let pb = m.add(ProgressBar::with_draw_target(
      Some(num_nodes as u64),
      indicatif::ProgressDrawTarget::stderr_with_hz(1),
  ));
  pb.set_style(
      ProgressStyle::with_template(
          "{msg} {spinner:.green} [{elapsed_precise}] [{bar:40.cyan/blue}] ({pos}/{len}, ETA {eta}, SPEED {per_sec})",
      )
      .unwrap(),
  );
  pb.set_message(format!("[CREATE CLUSTER {}]", seed));
djc commented 2 years ago

If you had a setup on 0.16 that you did find pleasing to look at, do you have a video of that, too?

hut8 commented 1 year ago

What do you think about this patch? It shouldn't be backwards incompatible due to trait bounds. I am a beginner at rust but changing this to f64 (or really, Into<f64>) seems like an okay solution. Then I can change the rate to 0.1 to update only once every 10 seconds, for example.

I think this is quite necessary due to the jumpy nature of the values otherwise. Within the span of one second, my progress bar estimates that the ETA is 20 minutes all the way to 1 hour and is impossible to read.

https://github.com/console-rs/indicatif/compare/main...hut8:indicatif:float-rate-limit

djc commented 1 year ago

Again, if you want to run a lower refresh rate because the estimates jump around too much, you should investigate the estimation code and fix it there. See discussion in #394.

remi-dupre commented 7 months ago

Hi!

I just came across this issue and it seems that it is idle because no good reason was given why a lower refresh rate would be required.

A while ago at my previous job I built a tool that would run for a very log time with the full dataset as input (at least 24h). With a refresh rate of 1Hz a full day would then output almost 100k lines of logs, which was very inconvenient to work with (truncated logs or slow searches).

At the time it even motivated to build my own lighter progress library (which allowed me to update ~ every 1min) :confused: https://github.com/remi-dupre/prog-rs

djc commented 7 months ago

Well, if someone is sufficiently motivated I might take a PR for this.