Introduce public summary. Remove "no step" / "step" logic from plots.

[daavoo]

"no step" / "step" logic was initially introduced to support different logging formats between step and not step updates.

For live.log_image, "step" mode now overwrites the path instead of creating subfolder by step. Closes #326

For live.log, the "no step" was meant not to generate the .tsv file but only the .json.
Added a public property summary so "no step" scenarios can work as follows:

live = Live()

live.summary["foo"] = 1
live.make_summary()

This is a similar design used by wandb (https://docs.wandb.ai/guides/track/log#summary-metrics), where the latest values are in summary by default but single scalars are supposed to be added via manual summary modification instead of log

[shcheklein]

thanks @daavoo, interface is getting better!

QQ: so, how will it look like for the example-get-started repo, for example? We'll need to modify summary to log "scalars"?Also, can rename the file?

(it feels we are optimizing for DL scenarios a bit too much?)

[daavoo]

QQ: so, how will it look like for the example-get-started repo, for example?

If you leave the code as it is right now, everything will be the same with the exception that a .tsv file will be additionally created in dvclive/plots/metrics.

We'll need to modify summary to log "scalars"?

If you don't want to generate a single-value .tsv plot file, yes.

Also, can rename the file?

Output files have already been renamed to metrics in #322
Are you referring to the property summary?

[skshetry]

Should make_summary be called write_summary or just log_summary?

[dberenbaum]

Can you easily separate the log_image changes from the rest? I think that's easy to merge, but the other part seems a little more divisive .

This is a similar design used by wandb (https://docs.wandb.ai/guides/track/log#summary-metrics), where the latest values are in summary by default but single scalars are supposed to be added via manual summary modification instead of log

What about params? Those are non-step values that get logged via a logger method, although I think we discussed setting them available by dict modification.

It seems this PR is only halfway towards what wandb and mlflow do, since they both have step = 0 by default, whereas this leaves some in-between state where the default is step = None and there is no step value in metrics.json, but dvclive writes out the tsv files with step = 0. Would it simplify logic internally to default to step = 0?

it feels we are optimizing for DL scenarios a bit too much?

Assuming a step-based workflow is what both mlflow and wandb do, so I can understand the push to do the same. However, I don't like that dvclive would then be generating single-point tsv plots from all the metrics. Maybe we can leave in some logic for step 0/None to ignore writing to tsv until set_step is called/wait until there is a change in a metric before writing it to tsv (and possibly not add step 0 to metrics.json, although I'm less worried about this)?

[dberenbaum]

Should make_summary be called write_summary or just log_summary?

Hmm, no strong opinion, but should this method also be either made private or documented?

[daavoo]

What about params? Those are non-step values that get logged via a logger method, although I think we discussed setting them available by dict modification.

I think params should be handled like summary here (and wandb.config https://docs.wandb.ai/guides/track/config).

It seems this PR is only halfway towards what wandb and mlflow do, since they both have step = 0 by default, whereas this leaves some in-between state where the default is step = None and there is no step value in metrics.json, but dvclive writes out the tsv files with step = 0. Would it simplify logic internally to default to step = 0?

Not sure I see the problem.

there is no step value in metrics.json can only happen if the user doesn't update the step value. In this no step scenario I see no issue with not having step in metrics.json

Maybe we can leave in some logic for step 0/None to ignore writing to tsv until set_step is called/wait until there is a change in a metric before writing it to tsv (and possibly not add step 0 to metrics.json, although I'm less worried about this)?

The idea is that, for no-step workflow, we document the following (instead of calling live.log_metric):

live = Live()

live.summary["foo"] = 1
live.make_summary()

This won't generate .tsv but only update metrics.json. I was just mentioning in ivan answer that even without changing the code the result would be similar, but I mean to say that we should change the code in example-get-started to the snippet above.

Hmm, no strong opinion, but should this method also be either made private or documented?

Method should be public to document the snippet above

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (1.0@876cf20). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files

@@          Coverage Diff           @@
##             1.0     #331   +/-   ##
======================================
  Coverage       ?   95.35%           
======================================
  Files          ?       36           
  Lines          ?     1702           
  Branches       ?      153           
======================================
  Hits           ?     1623           
  Misses         ?       57           
  Partials       ?       22           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[dberenbaum]

The idea is that, for no-step workflow, we document the following (instead of calling live.log_metric):

live = Live()

live.summary["foo"] = 1
live.make_summary()

I don't mind supporting it, but I wouldn't expect that the summary dict is a primary workflow, and I wouldn't introduce it in our example-get-started project. wandb doesn't introduce this in their get started materials, and I think they generally assume users are fine with logging to step=0 by default. I think we should either keep no-step logic or live with some ugly output in example-get-started (primarily the step-0 tsv files/plots).

However, I don't like that dvclive would then be generating single-point tsv plots from all the metrics. Maybe we can leave in some logic for step 0/None to ignore writing to tsv until set_step is called/wait until there is a change in a metric before writing it to tsv (and possibly not add step 0 to metrics.json, although I'm less worried about this)?

there is no step value in metrics.json can only happen if the user doesn't update the step value. In this no step scenario I see no issue with not having step in metrics.json

Why is there a step logged to the tsv file? This seems more important than metrics.json since it generates new directories, files, and plots that aren't useful.

Would it simplify logic internally to default to step = 0?

What I meant here is there still seems to be some "no step" logic that we could drop if we default to self._step = 0:

https://github.com/iterative/dvclive/blob/e115105495746f73721aa14a76fc431b46f05761/src/dvclive/live.py#L162

https://github.com/iterative/dvclive/blob/e115105495746f73721aa14a76fc431b46f05761/src/dvclive/live.py#L165-L167

https://github.com/iterative/dvclive/blob/e115105495746f73721aa14a76fc431b46f05761/src/dvclive/live.py#L260-L261

[daavoo]

I think they generally assume users are fine with logging to step=0 by default. I think we should either keep no-step logic or live with some ugly output in example-get-started (primarily the step-0 tsv files/plots).

I don't see it that way. I think the difference is that our example-get-started focuses on "no step" scenario whereas any other experiment tracker focuses on "step" scenarios first (or even only).

The introduction to wandb.log (https://docs.wandb.ai/guides/track/log) is clearly oriented towards a "step" scenario and yet the summary workflow has a dedicated section (https://docs.wandb.ai/guides/track/log#summary-metrics)

[dberenbaum]

Discussed with @daavoo and agreed to:

1. Keep using live.log() as the primary workflow for step and no-step scenarios but without any special syntax for no-step scenarios. This means no-step workflows will still generate tsv files and have step: 0 in the summary. This is no different than mlflow and wandb and doesn't seem to bother people.
2. Introduce live.summary["foo"] = 1 from this PR as a special case (for example, a step scenario where some metrics are not step-based).

[daavoo]

Merging it. Again, we can get back and change stuff found during docs review

[dberenbaum]

Sorry, forgot to approve this one.