frameworks: Call live.end at the end of training.

[daavoo]

Ensure logic inside live.end is only called once.

This is mainly for Studio to get a "completed" event when using the frameworks

[dberenbaum]

Minor: Could you use the @functools.cache decorator on def end(self): instead of adding self._ended?

[daavoo]

Depending on what we agree in #364 (comment)

[dberenbaum]

How should this type of scenario work?

with Live() as live:

  live.log_params(...)
  
  model.fit(DVCLiveCallback(live=live))
  
  # Post-training logs
  live.log_image(...)

[daavoo]

How should this type of scenario work?

I would expect the live metrics functionality to be only working inside the callback.

If I decouple the post_to_studio(self, "done", logger) from the live._ended, I think the snippet would behave as expected.

So:

    self.make_summary()
    if self.report_mode == "studio":
        if not self._ended:
            if not post_to_studio(self, "done", logger):
                logger.warning("`post_to_studio` `done` event failed.")
            self._ended = True
    else:
        self.make_report()

[dberenbaum]

None of this is a blocker since I don't think anything breaks the basic live metrics workflow, but it would be good to have it to "unbreak" more complex scenarios.

If I decouple the post_to_studio(self, "done", logger) from the live._ended, I think the snippet would behave as expected.

It's a little ugly but more likely to work the way users want (log summary/report always).

I would expect the live metrics functionality to be only working inside the callback.

Can we also have something like this to enable the context manager to override other calls to end?

    self.make_summary()
    if self.report_mode == "studio":
        if not self._ended:
            if not post_to_studio(self, "done", logger):
                logger.warning("`post_to_studio` `done` event failed.")
-           self._ended = True
+           if not self._keep_alive: # set to True in __enter__ and False in __exit__
+               self._ended = True
    else:
        self.make_report()

[codecov-commenter]

Codecov Report

Base: 96.37% // Head: 96.47% // Increases project coverage by +0.10% 🎉

Coverage data is based on head (9c5cd45) compared to base (da345f5).
Patch coverage: 96.55% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #364      +/-   ##
==========================================
+ Coverage   96.37%   96.47%   +0.10%     
==========================================
  Files          36       36              
  Lines        1739     1789      +50     
  Branches      153      155       +2     
==========================================
+ Hits         1676     1726      +50     
  Misses         37       37              
  Partials       26       26              

Impacted Files	Coverage Δ
src/dvclive/live.py	96.13% <60.00%> (+0.06%)	⬆️
src/dvclive/catalyst.py	100.00% <100.00%> (ø)
src/dvclive/fastai.py	94.11% <100.00%> (+0.78%)	⬆️
src/dvclive/huggingface.py	91.30% <100.00%> (+0.82%)	⬆️
src/dvclive/keras.py	100.00% <100.00%> (ø)
src/dvclive/lightning.py	90.90% <100.00%> (+0.66%)	⬆️
src/dvclive/xgb.py	95.00% <100.00%> (+0.88%)	⬆️
tests/test_catalyst.py	95.55% <100.00%> (+0.43%)	⬆️
tests/test_fastai.py	100.00% <100.00%> (ø)
tests/test_huggingface.py	97.82% <100.00%> (+0.07%)	⬆️
... and 4 more

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.