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

Add mechanism for launching per-row actions from the table webview #1267

Closed
Tracked by #1054
rogermparent opened this issue Jan 28, 2022 · 8 comments
Closed
Tracked by #1054
Assignees
Labels
🎨 design Needs design input or is being actively worked on discussion priority-p1 Regular product backlog product PR that affects product

Comments

@rogermparent
Copy link
Contributor

rogermparent commented Jan 28, 2022

This issue tracks the ability to run commands on individual experiments from the experiments table webview.

We currently have a few commands that do this:

  • Apply Experiment
  • Branch Experiment
  • Queue Experiment From Existing
  • Remove Experiment

But they currently run on vscode-native quickpicks that display a list of experiments. These commands will have to be modified so that they can be called from the webview with a pre-filled experiment.

We currently have a design proposal for this using a context menu, though it's not set in stone:

Single row actions preview:
image

Multiple rows actions preview:
image

More details and flows in the Figma spec.

This Issue is a slight superset of #1135, but they're close enough that we can merge that one into this issue:

#1135: Action per item to run experiments

Originated at #1120 (comment)

Single new experiment

A single experiment to queue, rather than how to submit a whole batch of experiments. For example, there could be a GUI that shows all the parameters, and users can edit their values and click "queue experiment" without needing to edit yaml files or run terminal commands.

We should be able to run experiments based on any of the experiments in the table or workspace.

Batch

Concerns:

As @daavoo alluded, when it comes to larger scale hyperparameter tuning, users probably can't avoid doing this programmatically since there are so many experiments to queue, the methods of defining the hyperparameter space are too varied, and frequently they can't even all be queued up front because later iterations base their values on the results of previous iterations.

I think it makes sense for advanced scenarios and especially when experiments are expensive to run (deep learning), in case of "classical" DS random forest, xgboost it's easier to run a small grid batch .

I would add some basic way to specify a list of values at least and/or simple start-step-end UI.

@rogermparent rogermparent added priority-p1 Regular product backlog 🎨 design Needs design input or is being actively worked on product PR that affects product discussion labels Jan 28, 2022
@mattseddon
Copy link
Member

I have had the request to toggle experiments on/off from the table today. I would expect this would be done via left-click on or around the dot shown at the start of the experiment row. This will interfere with the current behaviour of collapsing the row but should be achievable. I am going to pair with @sroy3 on the problem later in the week. LMK if you see any issue with this.

Thanks,

@rogermparent
Copy link
Contributor Author

I'm starting on this, and likely looking at the "right click for context menu" route since it doesn't step on our current left-click toggle behavior.

@rogermparent
Copy link
Contributor Author

rogermparent commented Apr 1, 2022

Do we expect that we'll have any context menu actions in the future that operate on individual cells as well as rows? I just had the thought and made #1517 in anticipation (though it seems like a good change on its own) but I can't gleam any from the OP, so I'll be working under the assumption that it'll be rows only for now.

@mattseddon
Copy link
Member

Do we expect that we'll have any context menu actions in the future that operate on individual cells as well as rows? I just had the thought and made #1517 in anticipation (though it seems like a good change on its own) but I can't gleam any from the OP, so I'll be working under the assumption that it'll be rows only for now.

Inline edit of params is one example

@rogermparent
Copy link
Contributor Author

True, but I was thinking specifically of cells that aren't in the workspace. Maybe a "copy value to workspace", similar to apply but for a single param?

@mattseddon
Copy link
Member

Yeah, that would be low priority

@mattseddon
Copy link
Member

This is the last open item from #239

wolmir added a commit that referenced this issue May 5, 2022
Add context menu payload with item details from webview

Add non-wired context menu

Revert broken label

Add rough functional wiring

Refactor context menu

Create context menu component

Open context menu on row

Use original implementation of the context-menu

Create MessagesMenu component

Update context-menu story

Use messages-menu in the table rows

Process experiment row ctx-menu messages

Remove context-menu webview message

Refactor Row.tsx

Refactor Experiments constructor

Remove commented line

Add tests for apply to workspace and create branch

Fis option labels

Add test for Modify params and queue option

Add test for Experiment Removed message

Prevent default menu from showing up when we close ours

Make runCommand private in extension Experiments repository

Stub internalCommand instance method instead of the prototype

Refactor mock input box utility

Disable row context-menu if the experiment is running

Hide context menu on click

Disable the 'Apply to workspace' and 'Create branch' options when row is workspace or already a branch

Extract row context-menu into a separate component

Do not use the constructor shortcut in order to keep consistency

Refactor option label and contextMenuOptions function

Refactor Experiments repository constructor

Refactor Experiments modifyExperimentParamsAndQueue

Remove unused interface

Disable context-menu if an y experiment is running
wolmir added a commit that referenced this issue May 5, 2022
Add context menu payload with item details from webview

Add non-wired context menu

Revert broken label

Add rough functional wiring

Refactor context menu

Create context menu component

Open context menu on row

Use original implementation of the context-menu

Create MessagesMenu component

Update context-menu story

Use messages-menu in the table rows

Process experiment row ctx-menu messages

Remove context-menu webview message

Refactor Row.tsx

Refactor Experiments constructor

Remove commented line

Add tests for apply to workspace and create branch

Fis option labels

Add test for Modify params and queue option

Add test for Experiment Removed message

Prevent default menu from showing up when we close ours

Make runCommand private in extension Experiments repository

Stub internalCommand instance method instead of the prototype

Refactor mock input box utility

Disable row context-menu if the experiment is running

Hide context menu on click

Disable the 'Apply to workspace' and 'Create branch' options when row is workspace or already a branch

Extract row context-menu into a separate component

Do not use the constructor shortcut in order to keep consistency

Refactor option label and contextMenuOptions function

Refactor Experiments repository constructor

Refactor Experiments modifyExperimentParamsAndQueue

Remove unused interface

Disable context-menu if an y experiment is running

Report the command id when throwng a not found error

Undo workspace refactor
wolmir added a commit that referenced this issue May 5, 2022
Add context menu payload with item details from webview

Add non-wired context menu

Revert broken label

Add rough functional wiring

Refactor context menu

Create context menu component

Open context menu on row

Use original implementation of the context-menu

Create MessagesMenu component

Update context-menu story

Use messages-menu in the table rows

Process experiment row ctx-menu messages

Remove context-menu webview message

Refactor Row.tsx

Refactor Experiments constructor

Remove commented line

Add tests for apply to workspace and create branch

Fis option labels

Add test for Modify params and queue option

Add test for Experiment Removed message

Prevent default menu from showing up when we close ours

Make runCommand private in extension Experiments repository

Stub internalCommand instance method instead of the prototype

Refactor mock input box utility

Disable row context-menu if the experiment is running

Hide context menu on click

Disable the 'Apply to workspace' and 'Create branch' options when row is workspace or already a branch

Extract row context-menu into a separate component

Do not use the constructor shortcut in order to keep consistency

Refactor option label and contextMenuOptions function

Refactor Experiments repository constructor

Refactor Experiments modifyExperimentParamsAndQueue

Remove unused interface

Disable context-menu if an y experiment is running

Report the command id when throwng a not found error

Undo workspace refactor

Refactor contextMenuOptions function in Row.tsx
wolmir added a commit that referenced this issue May 5, 2022
Add context menu payload with item details from webview

Add non-wired context menu

Revert broken label

Add rough functional wiring

Refactor context menu

Create context menu component

Open context menu on row

Use original implementation of the context-menu

Create MessagesMenu component

Update context-menu story

Use messages-menu in the table rows

Process experiment row ctx-menu messages

Remove context-menu webview message

Refactor Row.tsx

Refactor Experiments constructor

Remove commented line

Add tests for apply to workspace and create branch

Fis option labels

Add test for Modify params and queue option

Add test for Experiment Removed message

Prevent default menu from showing up when we close ours

Make runCommand private in extension Experiments repository

Stub internalCommand instance method instead of the prototype

Refactor mock input box utility

Disable row context-menu if the experiment is running

Hide context menu on click

Disable the 'Apply to workspace' and 'Create branch' options when row is workspace or already a branch

Extract row context-menu into a separate component

Do not use the constructor shortcut in order to keep consistency

Refactor option label and contextMenuOptions function

Refactor Experiments repository constructor

Refactor Experiments modifyExperimentParamsAndQueue

Remove unused interface

Disable context-menu if an y experiment is running

Report the command id when throwng a not found error

Undo workspace refactor

Refactor contextMenuOptions function in Row.tsx
@mattseddon
Copy link
Member

Lifted multi-select into #1663

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 design Needs design input or is being actively worked on discussion priority-p1 Regular product backlog product PR that affects product
Projects
None yet
Development

No branches or pull requests

2 participants