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

Remove checkpoint experiment support from extension UI #3577

Closed
mattseddon opened this issue Mar 29, 2023 · 1 comment · Fixed by #3585
Closed

Remove checkpoint experiment support from extension UI #3577

mattseddon opened this issue Mar 29, 2023 · 1 comment · Fixed by #3585
Labels
A: experiments Area: experiments table webview and everything related A: extension Area: core extension A: integration Area: DVC integration layer

Comments

@mattseddon
Copy link
Member

mattseddon commented Mar 29, 2023

Based on the discussion in iterative/dvc#9221 and the need to update exp show --json (iterative/dvc#9170) for #3178 & #3436. We have decided to drop checkpoints from the UI in the extension.

As we are doing this at the same time as updating the parsing logic to deal with a new format of experiments data we have decided to avoid integrating the checkpoints data for now. However, (also for now) we do need still need to know whether or not there are checkpoints in the workspace's projects so that we can offer the option to resume an experiment.

UI

This change will primarily affect the experiments tree and webview. I.e these rows will be removed:

image

There will be flow-on effects in the rest of the extension. Below is the list of things that will need to be updated in the UI

  1. Badges that group "hidden" values for a collapsed checkpoint experiment should be moved from the experiment level up to commits. I.e these:

image

  1. Conditional hasCheckpoints logic needs to be dropped for filtering

image

  1. Checkpoints need to be dropped from the "select experiments to plot" quick pick.

image

  1. Custom checkpoint trend plots should be dropped altogether (including collection, spec, etc)

image

  1. Logic to add the count to the experiments tree needs to be updated

image

  1. All references to checkpoints should be removed from the walkthrough/docs/pictures

image

image

Further code changes:

package.json

As we have always treated checkpoints differently to experiments/commits all of the conditional logic in our package.json will need to be updated. E.g the view/item/context entry for dvc.views.experiments.applyExperiment. Again, we should (for now) keep the ability to resume a checkpoint experiment if there are checkpoint experiments detected in any of the projects in the workspace.

experiments table context menu

Conditional logic for checkpoints context menu can be dropped and the whole component can be greatly simplified.

Screen.Recording.2023-03-29.at.11.40.44.am.mov

patches/redundancy

The biggest one is collectOverrideRevisionDetails which is used to override plots data when there is a checkpoint experiment running in the workspace. Checkpoints should also not be passed to the code that assigns each experiment a status/color, etc.

test fixtures/stories

The test fixtures will all need to be updated to match the new data coming out of iterative/dvc#9170. We will no doubt need to update multiple stories and potentially drop some.

Outside of 1, the task will consist mostly of deleting code/conditional logic branches.

Note: Sorting logic seems fine but should be revisited separately (only experiments are sorted under commits).

@mattseddon mattseddon added A: experiments Area: experiments table webview and everything related A: extension Area: core extension A: integration Area: DVC integration layer labels Mar 29, 2023
@mattseddon
Copy link
Member Author

I have covered all of the above items outside of

  1. Custom checkpoint trend plots should be dropped altogether (including collection, spec, etc)

This is blocked by #3575

patches/redundancy

is blocked mainly by 4

test fixtures/stories

is blocked by removal of patches/redundancy

Providing everything goes smoothly and I don't get pinned working on plots (or merging the PR mountain) I should be able to wrap this up by the end of the sprint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: experiments Area: experiments table webview and everything related A: extension Area: core extension A: integration Area: DVC integration layer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant