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

Apply sorting to the entire table #4620

Closed
Danila89 opened this issue Sep 5, 2023 · 18 comments · Fixed by #4685
Closed

Apply sorting to the entire table #4620

Danila89 opened this issue Sep 5, 2023 · 18 comments · Fixed by #4685
Assignees
Labels
A: experiments Area: experiments table webview and everything related enhancement New feature or request priority-p1 Regular product backlog triage

Comments

@Danila89
Copy link

Danila89 commented Sep 5, 2023

According to this message sorting and filtering in vscode extension is only applied to the experiments under commit.
I'm using the extension as a leaderboard and it would be pretty convenient to be able to sort/filter the entire table.
image

@julieg18
Copy link
Contributor

julieg18 commented Sep 5, 2023

Filters were applied to commits in #4363, though sorting is currently only applied to experiments under commits.

@julieg18 julieg18 added enhancement New feature or request A: experiments Area: experiments table webview and everything related labels Sep 5, 2023
@shcheklein shcheklein added priority-p1 Regular product backlog triage labels Sep 5, 2023
@julieg18
Copy link
Contributor

julieg18 commented Sep 5, 2023

I'm using the extension as a leaderboard and it would be pretty convenient to be able to sort/filter the entire table.

@Danila89, do custom plots work for your use case instead of table sorting? You can use them to compare a metric/param across all commits and experiments:

Screen.Recording.2023-09-05.at.6.15.36.PM.mov

@Danila89
Copy link
Author

Danila89 commented Sep 6, 2023

These plots are really helpful, but it is essential for me to use some filtering of the experiments before plotting them. I didn't find the way to filter experiments before plotting them in the vscode extension.
Plots are nice in addition to sorting, not instead of it)

@julieg18
Copy link
Contributor

julieg18 commented Sep 6, 2023

These plots are really helpful, but it is essential for me to use some filtering of the experiments before plotting them. I didn't find the way to filter experiments before plotting them in the vscode extension.

Filtering experiments in custom plots would be simple enough to implement! Would that enhancement work for your use case or would table sorting still be needed?

@mattseddon mattseddon changed the title Apply sorting and filtering to the entire table Apply sorting to the entire table Sep 7, 2023
@Danila89
Copy link
Author

Danila89 commented Sep 8, 2023

It is OK like a workaround (and a very useful feature by the way), but table sorting is also nice to have

@julieg18
Copy link
Contributor

Did some research for table sorting and tried a couple ideas.

1. Flattening then sorting the table

By flattening the table and turning off nested rows entirely, we could sort all rows from biggest to smallest and vice versa. While I don't think this would be too hard to implement on the backend, we'd need to rethink how our table would look while flattened (how do we tell what row belongs to what commit/branch). This would require some design work and adjusting our table components to handle the possibility.

2. Sorting commits along with experiments

Commits would be sorted along with the experiments based on their largest (smallest could work too) value among its baseline values and its subrows' values. While this does make sorting more complicated (and probably performant since we need to parse all the table values), it is doable. I was able to get a working demo made pretty quickly. The demo is using the commit's and its subrows' max value to sort:

Screen.Recording.2023-09-12.at.11.28.09.AM.mov

3. Sorting commits and branches along with experiments

Branches would be sorted similarly to how commits are sorted in idea 2 aka max value among itself and subrow's values. I also think it's doable as well but would be a bit more complicated/performant than method 2 since now we're sorting through 3 row layers instead of 2.

What do we think? cc @shcheklein, @mattseddon

@julieg18 julieg18 self-assigned this Sep 12, 2023
@julieg18
Copy link
Contributor

As discussed in planning we've decided on working on flattening then sorting the entire table.

Here's how the table looks flattened:

Screen.Recording.2023-09-13.at.10.55.39.AM.mov

Things to consider for this table:

  • We need a way to show what sub row belongs to what. Aka what experiment row belongs to what commit belongs to what branch. One option discussed was adding an extra column that gives git information (branch/commit sha)
  • There is no way to add or minus commits in branches. So far I'm not seeing a place for adding the +/- actions 🤔

@julieg18 julieg18 mentioned this issue Sep 18, 2023
1 task
@julieg18
Copy link
Contributor

Finished the first iteration of the sorted table design! Demo/Images:

Screen.Recording.2023-09-20.at.11.42.45.AM.mov
Screenshot 2023-09-20 at 11 35 38 AM Screenshot 2023-09-20 at 11 43 26 AM

I've added a column called "Branch/Commit" (Other options could be "Parents" or "Git Details") that gives branch/commit information and has a tooltip in the header that explains the currently missing functionality.

I do think we'll be able to add an action somewhere for increasing/decreasing commits but I figured we could do so in a follow-up since I wasn't sure where yet.

What do we think? cc @shcheklein

@shcheklein
Copy link
Member

I do think we'll be able to add an action somewhere for increasing/decreasing commits but I figured we could do so in a follow-up since I wasn't sure where yet.

Good point about the need for the action! yes, can be a followup. I don't know how to do this also. Can be a context menu in a branch column? Can be +/- in the column itself? We can come up with something. Can be after all a way to quickly switch back to do that for now.

I've added a column called "Branch/Commit" (Other options could be "Parents" or "Git Details") that gives branch/commit information and has a tooltip in the header that explains the currently missing functionality.

my 2cs - let's split into Parent and Branch/Tags. Otherwise we are mixing semantics in the same column.

@julieg18
Copy link
Contributor

Updated the design by splitting the "Branch/Commit" column into "Branch/Tags" and "Parent" columns:

Screen.Recording.2023-09-21.at.11.22.21.AM.mov
Screenshot 2023-09-21 at 11 22 50 AM

I kept the commit/branch icons to better differentiate them from regular cell columns, but here's an quick example of how they look without the icons:

Screenshot 2023-09-21 at 11 23 38 AM

cc @shcheklein

@mattseddon
Copy link
Member

I still have concerns over the amount of space that we'll be taking up by adding two columns instead of one. We should also note that the "parent" or "commit" column will be empty when the record is a commit. I'd suggest changing the name to baseline even if we keep them separate.

@shcheklein
Copy link
Member

I'd suggest changing the name to baseline even if we keep them separate.

Baseline is already used in Studio for delta / comparison mode. So, I think we should try to avoid it.

For commits we can show their actual parent commit?

@julieg18
Copy link
Contributor

We should also note that the "parent" or "commit" column will be empty when the record is a commit.

Updated the table to make commits rows' "Parent" column blank:

image

For commits we can show their actual parent commit?

Apologies, I'm not sure what you mean by "their actual parent commit". Commit rows don't have parent commits as far as I know?

@mattseddon
Copy link
Member

mattseddon commented Sep 27, 2023

@shcheklein is this what you're talking about:

image

The term Baseline is found all through the docs in reference to the commit an experiment was created from.

https://dvc.org/doc/user-guide/experiment-management
https://dvc.org/doc/user-guide/experiment-management/comparing-experiments
https://dvc.org/doc/studio/user-guide/experiments/run-experiments#manage-a-completed-experiment

@shcheklein
Copy link
Member

Yes,Matt! That's the baseline that I meant.

@shcheklein
Copy link
Member

So, I suggest not making it worse. I think parent is descriptive and applies to commits and exps, wdyt?

Overall, not a strong opinion.

@julieg18
Copy link
Contributor

julieg18 commented Oct 5, 2023

As of release 1.1.0, the table now flattens when sorted, applying sorting to all rows (minus workspace):

Screen.Recording.2023-09-26.at.12.12.39.PM.mov

@Danila89, feel free to give us any feedback you have on the feature.

@Danila89
Copy link
Author

Danila89 commented Oct 5, 2023

Works for me now, thank you very much!

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 enhancement New feature or request priority-p1 Regular product backlog triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants