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

Dvclive output subdirs #6628

Closed
wants to merge 13 commits into from
Closed

Dvclive output subdirs #6628

wants to merge 13 commits into from

Conversation

daavoo
Copy link
Contributor

@daavoo daavoo commented Sep 16, 2021

None of the changes are strictly required by DVCLive but I feel like the existing output structure could be revisited after the addition of image support and the changes to html generation (which is now a folder).

This P.R. makes live outputs to be ordered in subdirs divided by plot type. Currently linear and images.
Summary is now generated under {live_path}/summary.json instead of {live_path}.json.
HTML is now generated under {live_path}/html instead of {live_path}_dvc_plots.

@daavoo daavoo requested review from pared and dberenbaum September 16, 2021 10:01
@pared
Copy link
Contributor

pared commented Sep 17, 2021

We still need to make compatible changes on dvclive side, right?

@daavoo
Copy link
Contributor Author

daavoo commented Sep 17, 2021

We still need to make compatible changes on dvclive side, right?

Yep. Just wanted to check how this would look on DVC side and what are your thoughs

@pared
Copy link
Contributor

pared commented Sep 17, 2021

I believe this is a great idea. Generating summary and HTML is along the live was awkward. Inside, I think its great. One thing to consider is .dvcignore -ing HTML if it's produced so that we don't store it.

@dberenbaum
Copy link
Collaborator

Related: iterative/dvclive#148 and #6431 (comment).

Great idea. I have two questions:

  1. Is the images path needed now?
  2. Is linear the best name for the .tsv directory? You might be thinking ahead to dvclive.log: log array, tensors and similar objectsΒ dvclive#82, but I think linear is not that intuitive in dvclive for users, especially since it contains the .tsv and not the plot. Other ideas:
    a. metrics: the .tsv files are step-based logs of what would generally be considered metrics in dvc and similar tools.
    b. scalars: describes the data type being logged (this is also what's used in tensorboard for their linear plots).

@daavoo daavoo mentioned this pull request Sep 21, 2021
2 tasks
@daavoo daavoo self-assigned this Sep 21, 2021
@pared
Copy link
Contributor

pared commented Sep 23, 2021

@daavoo you requested my review? It LGTM so far, but I guess we need to make corresponding changes in dvclive to let tests pass. Am I missing something?

Also, changing how we store HTML and summary will need probably a minor version update on dvclive side.

@daavoo daavoo closed this Oct 11, 2021
@dberenbaum
Copy link
Collaborator

@daavoo What happened with this PR? Are you planning to keep the dvclive_dvc_plots path?

@daavoo
Copy link
Contributor Author

daavoo commented Oct 13, 2021

@daavoo What happened with this PR? Are you planning to keep the dvclive_dvc_plots path?

I was planning on waiting for the HTML layout / further plots development changes to consolidate and get merged before addressing this kind of changes for DVCLive.

@dberenbaum
Copy link
Collaborator

Related complaint: https://discord.com/channels/485586884165107732/485596304961962003/915439735256801352. Maybe we should consider reopening this?

@daavoo
Copy link
Contributor Author

daavoo commented Dec 1, 2021

Related complaint: https://discord.com/channels/485586884165107732/485596304961962003/915439735256801352. Maybe we should consider reopening this?

From my understanding, the user would like to be able to put all dvc/dvclive related stuff in some folder (and suggests using .dvc) to keep things organized because dvc/dvclive related stuff pollutes the root of the repo.

There are a few different things that don't seem strictly related to the P.R:

  • DVCLive allows configuring output_dir so it's outputs can be reorganized. However .dvc can't be used as the output of any stage, so DVCLive can't use .dvc as output_dir.
  • Users can't customize the internal DVC project structure at the root of a DVC repo. They can use init --subdir but that doesn't solve the pollution of the subdir.

@dberenbaum
Copy link
Collaborator

There are a few different things that don't seem strictly related to the P.R:

πŸ’― It's not strictly related. Let me open a separate issue to discuss.

@daavoo daavoo deleted the dvclive-output-subdirs branch February 11, 2022 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants