vscode-dvc icon indicating copy to clipboard operation
vscode-dvc copied to clipboard

Plots should be more resilient to errors in specific revisions

Open shcheklein opened this issue 2 years ago • 18 comments

https://github.com/iterative/vscode-dvc/assets/3659196/9b346856-bf72-4cc0-a467-eea94c6ccaeb

shcheklein avatar Jul 23 '23 17:07 shcheklein

I would argue that this is a CLI issue. The extension is only reflecting what is provided by DVC:

image

mattseddon avatar Jul 23 '23 21:07 mattseddon

It might be. It's still an issue that we have in the product. Users don't care about the implementation details. We can discuss and create a plan with the DVC team to address this.

shcheklein avatar Jul 23 '23 21:07 shcheklein

Did motor-abac exist at the time?

Edit: okay, I see there's more context in #4332. Should we disable/drop plots for queued and/or failed experiments? The only fix I can see in the CLI would be to still show the other revisions, but I still don't like that UX since I would be wondering where the motor-abac plots are.

dberenbaum avatar Jul 24 '23 12:07 dberenbaum

@dberenbaum good question:

Screenshot 2023-07-24 at 7 28 46 AM

shcheklein avatar Jul 24 '23 14:07 shcheklein

Should we disable/drop plots for queued and/or failed experiments?

We have a way to deal with - we show an error in the ribbon

Should we disable/drop plots for queued and/or failed experiments?

We can - it's just also not a good UX - let's say it fails in the middle - plots disappear and there is no atm even a way to tell what went wrong.

shcheklein avatar Jul 24 '23 14:07 shcheklein

~I think we tried to tackle this before in https://github.com/iterative/dvc/issues/9025~

That's not right, I'll open a new issue tomorrow.

mattseddon avatar Jul 27 '23 09:07 mattseddon

Raised https://github.com/iterative/dvc/issues/9776

mattseddon avatar Jul 28 '23 06:07 mattseddon

What is the end goal of the UX here? It still seems to me that it would be better to disable plots for rows where they don't exist, and this matches the UX in Studio. An error in the ribbon makes sense to me if there is an issue with one plot within a revision, but I don't see the point in showing the revision at all if it has no plots or isn't even a valid git revision.

dberenbaum avatar Jul 31 '23 14:07 dberenbaum

The goal here is to have a "stable" UX for plots. As a user I should be able to pick any experiment to be show in plots and it should stay there all the time until I toggle it back off. It should not be disappearing w/o me as a user understanding why. Users should not care about the internals of the system. If there is an entry in the table it should behave the same as any other entry preferably. Happy to discuss if there are any other ideas here.

shcheklein avatar Jul 31 '23 15:07 shcheklein

I thought this is a case where there are no plots for that experiment yet (it hasn't been run yet, or it failed before plots were generated). Is that the case? Does it make sense to give users an option to view plots for an experiment in that state?

dberenbaum avatar Jul 31 '23 16:07 dberenbaum

so, the full scenario is:

  • I queue multiple exps
  • I select them for plots
  • I run-all them
  • I open plots to follow
  • I want to see them or proper errors

We can indeed disable the plots button until we 100% sure that it can be plotted at all, but it feels like an interface that is hard to explain (unstable, changes its state based on the state of an experiment, etc).

shcheklein avatar Jul 31 '23 16:07 shcheklein

How it looks in Studio now:

https://github.com/iterative/vscode-dvc/assets/2308172/6fd3ec9d-4440-40be-8646-effd9a790541

It doesn't allow you to "pre-select" rows that don't yet have plots, but otherwise I have found it useful and intuitive rather than not knowing what rows have plots until I open the plots view. Is it important enough to "pre-select" all the queued experiments to plot that we need a different UX in VS Code?

dberenbaum avatar Jul 31 '23 16:07 dberenbaum

Even in Studio if I know in advance that it's supposed to be a live experiments with plots and it's just a matter of me waiting for it to receive some data, I would prefer to be able to pick it for plots and don't wait for button to appear.

In case of VS Code in case of those queued experiments we know that plot definitions are there, it would be confusing to not allow that button to my mind. Unless I misunderstand your point.

shcheklein avatar Jul 31 '23 17:07 shcheklein

FWIW you cannot plot queued experiments in the extension before they start:

image

mattseddon avatar Aug 01 '23 00:08 mattseddon

yep, but when they start you can immediately plot them, right?

btw, why don't we allow this even before they start? primarily, because we needed a place for an icon?

shcheklein avatar Aug 01 '23 00:08 shcheklein

I thought the issue here was related to an experiment that ended in an error preventing plots diff from returning anything.

The correct behaviour could be to allow selecting the experiment but not passing the name/id to plots diff. Instead, we would show an error in the plots ribbon for that revision. In the case of it being a queued experiment that failed we would give the user the chance to view the log. In order to view the log we would need access to that information... or we try to preload it into the plots data using dvc queue log <exp-name>.

This suggestion might bring us back to https://github.com/iterative/dvc/issues/9442

mattseddon avatar Aug 01 '23 01:08 mattseddon

yep, but when they start you can immediately plot them, right?

Yes, you can follow if you like. We don't track by default.

btw, why don't we allow this even before they start? primarily, because we needed a place for an icon?

  1. Queued experiments don't necessarily have metrics
  2. plots diff using the name errors with ERROR: unknown Git revision 'name'
  3. Assumption is that users running via the queue are not closely monitoring experiments as they run

mattseddon avatar Aug 01 '23 01:08 mattseddon

  1. Queued experiments don't necessarily have metrics

They also frequently have the wrong metrics and plots because they still contain the metrics and plots files from the baseline revision.

We can - it's just also not a good UX - let's say it fails in the middle - plots disappear and there is no atm even a way to tell what went wrong.

Opened https://github.com/iterative/dvc/issues/9787 for experiments that are killed/fail in the middle.

dberenbaum avatar Aug 01 '23 20:08 dberenbaum