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

LiveTask: write output path at start/end #6815

Merged
merged 1 commit into from
Oct 29, 2021
Merged

LiveTask: write output path at start/end #6815

merged 1 commit into from
Oct 29, 2021

Conversation

daavoo
Copy link
Contributor

@daavoo daavoo commented Oct 15, 2021

Closes iterative/dvclive#177

Logging at the end of each step can mess up with some ML Framework progress bars, I moved the message to the instantiation of the monitor task, as the path won't change during training.

@pared Do you consider necessary logging at each create_summary call? If so, we could maybe keep the call but at debug level

@daavoo daavoo requested a review from pared October 15, 2021 18:37
@daavoo daavoo requested a review from a team as a code owner October 15, 2021 18:37
@pared
Copy link
Contributor

pared commented Oct 19, 2021

@daavoo that might be a better idea, now its a bit clunky. Ideally I think it should be shown the same way as progress bar, in one place, but I am afraid that it is impossible since every framework logs something from time to time.

@dberenbaum
Copy link
Collaborator

@daavoo that might be a better idea, now its a bit clunky. Ideally I think it should be shown the same way as progress bar, in one place, but I am afraid that it is impossible since every framework logs something from time to time.

I think having it show up by default at least once is probably useful so users can just click on it rather than have to find the path and open it themselves.

What do you mean about showing it like a progress bar? Is there any instance where the output will change between steps?

@pared
Copy link
Contributor

pared commented Oct 19, 2021

@dberenbaum I mean that ideally, instead of printing URL in new line, we would be using \r to go back to line beginning and write the URL again in same line, but its impossible to implement, because we always log something during training, resulting in new lines with URL

Is there any instance where the output will change between steps?

Probably not

@dberenbaum
Copy link
Collaborator

Thanks, @pared. What's the value of overwriting if the info doesn't change between steps?

@daavoo daavoo self-assigned this Oct 19, 2021
@pared
Copy link
Contributor

pared commented Oct 19, 2021

@dberenbaum there is none, the main point was that ideally, we would only get a single path log, but usually, training produces some other logs, hiding static site URL somewhere in the beginning. Producing it every step makes it even more ugly, so I think let's log it once.

@dberenbaum
Copy link
Collaborator

training produces some other logs, hiding static site URL somewhere in the beginning

Good point, this is what I was missing. Maybe it would be nice to print once more at the end?

Producing it every step makes it even more ugly, so I think let's log it once.

Sounds good for now.

@pared
Copy link
Contributor

pared commented Oct 26, 2021

@daavoo I guess we can put the path into the debug for now. We can prepare another change for logging in the beginning and/or in the end.

@daavoo daavoo changed the title Moved live html summary info to instantiation Switch live html summary from info to debug Oct 26, 2021
@dberenbaum
Copy link
Collaborator

@daavoo I guess we can put the path into the debug for now. We can prepare another change for logging in the beginning and/or in the end.

So the user will not be shown the link to the output at all by default? Does the html open by default?

@daavoo
Copy link
Contributor Author

daavoo commented Oct 26, 2021

@daavoo I guess we can put the path into the debug for now. We can prepare another change for logging in the beginning and/or in the end.

So the user will not be shown the link to the output at all by default? Does the html open by default?

I will open a new P.R. addind the automatic open today

@dberenbaum
Copy link
Collaborator

@pared Why put the path into debug by default rather than printing once? Is it simply cleaner to implement? I think it seems useful to print the path somewhere.

@pared
Copy link
Contributor

pared commented Oct 26, 2021

@dberenbaum makes sense, now that I think about it it probably does not make sense to put it to debug because it will only clutter debug then, instead of std. But clutter is clutter.

@daavoo daavoo changed the title Switch live html summary from info to debug LiveTask: write output path at start/end Oct 26, 2021
Writing at the end of each step (create_summary) can mess up with some ML Framework progress bars, I moved the message to the monitor task, as the path won't change during training.

Fixes iterative/dvclive#177
@efiop efiop requested a review from pared October 29, 2021 12:29
@pared pared merged commit 0638d99 into master Oct 29, 2021
@pared pared deleted the live-html-info branch October 29, 2021 12:41
@efiop efiop added the enhancement Enhances DVC label Nov 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances DVC
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

dvc: prevent printing html path
4 participants