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

Fix auto-selection of running experiments #3705

Closed

Conversation

mattseddon
Copy link
Member

@mattseddon mattseddon commented Apr 18, 2023

2/2 main <- #3665 <- this

This PR fixes the auto selection of running experiments. The new behaviour is as follows:

  1. Experiments running in the workspace are automatically selected (not the workspace record).
  2. Experiments running in the queue are not automatically selected.
  3. Experiments running in the queue can no longer be selected through any other means (as calling plots diff with those revisions will cause an error).

This results in a good reduction in complexity. Once the prerequisites for #3436 land we can revisit.

Demos

Screen.Recording.2023-04-18.at.3.11.17.pm.mov
Screen.Recording.2023-04-18.at.3.31.09.pm.mov

@mattseddon mattseddon added the bug Something isn't working label Apr 18, 2023
@mattseddon mattseddon self-assigned this Apr 18, 2023
@mattseddon mattseddon changed the base branch from main to integrate-exp-show April 18, 2023 06:25
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR diff size of 37598 lines exceeds the maximum allowed for the inline comments feature.

@codeclimate
Copy link

codeclimate bot commented Apr 18, 2023

Code Climate has analyzed commit 3bf6eab and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1
Duplication 2

The test coverage on the diff in this pull request is 95.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 94.7% (0.0% change).

View more on Code Climate.

@mattseddon mattseddon changed the title Fix auto-selection of experiments Fix auto-selection of running experiments Apr 18, 2023
@mattseddon mattseddon marked this pull request as ready for review April 18, 2023 06:36
)?.status
)
experiment &&
(isQueued(experiment?.status) || isRunningInQueue(experiment))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While nothing happens, I can still select an experiment running in the queue when I run the Select Experiments to Display in Plots command

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is in the demo and expected as it is a patch for upcoming behaviour. As I have to redo most of this PR I will remove them from selection.

@@ -112,8 +112,6 @@ export class PlotsModel extends ModelWithPersistence {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably an issue on my end, but when I run an experiment, the plots break with this error: plots diff dolce-hugs techy-hugs 53aac68fc16182b38c509da7554c22f822fc8d63 miffy-runs -o .dvc/tmp/plots --split --json failed with ERROR: unknown Git revision 'miffy-runs'.

trim.mov

I can't see plots until the experiment finishes running.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I had tested this but obviously not. Seems like a bug in DVC. I'll confirm whether or not is expected and if they can provide a workaround. If not we'll be going back to the old behaviour.

@mattseddon
Copy link
Member Author

Will close this as the commit may still end up being useful later. For now, I'll raise a PR that disables the selection of experiments running in the queue/temp workspaces.

@mattseddon mattseddon closed this Apr 19, 2023
@mattseddon mattseddon deleted the remove-auto-workspace-selection branch April 28, 2023 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants