live: Add `log_image` and `log_plot`. #189

daavoo · 2021-11-08T20:15:07Z

❗ I have followed the Contributing to DVCLive guide.
📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.

Adds new plots data type.
Refactor each data type to separated methods:

Scalars -> Live.log -> Saves to dvclive.dir / scalars
Images -> Live.log_image -> Saves to dvclive.dir / images
Plots -> Live.log_plot -> Saves to dvclive.dir / plots

live.log_plot("roc", y_true, y_score)

Supported plot_type (first argument):

calibration
confusion_matrix
det
precission_recall
roc

Full example:

from dvclive import Live

from sklearn.datasets import make_classification
from sklearn.ensemble import RandomForestClassifier
from sklearn.model_selection import train_test_split
from sklearn.metrics import classification_report

X, y = make_classification(random_state=0)
X_train, X_test, y_train, y_test = train_test_split(X, y, random_state=0)
clf = RandomForestClassifier(random_state=0)
clf.fit(X_train, y_train)

y_pred = clf.predict(X_test)
y_score = clf.predict_proba(X_test)[:, 1]

live = Live()

live.log_plot("calibration", y_test, y_score)
live.log_plot("confusion_matrix", y_test, y_pred)
live.log_plot("confusion_matrix", y_test, y_score)
live.log_plot("precission_recall", y_test, y_score)
live.log_plot("roc", y_test, y_score)

stages:
  foo:
    cmd: python foo.py
    metrics:
    - dvclive.json
    plots:
    - dvclive/plots/calibration.json:
        cache: false
        x: prob_pred
        y: prob_true
        x_label: Mean Predicted Probability
        y_label: Fraction of Positives
        title: Calibration Curve
    - dvclive/plots/confusion_matrix.json:
        cache: false
        template: confusion
        x: actual
        y: predicted
        title: Confusion Matrix
    - dvclive/plots/precision_recall.json:
        cache: false
        x: recall
        y: precision
        title: Precision Recall Curve
    - dvclive/plots/det.json:
        cache: false
        x: fpr
        y: fnr
        title: DET curve
    - dvclive/plots/roc.json:
        cache: false
        x: fpr
        y: tpr
        title: ROC curve

dvc plots show

codecov-commenter · 2021-11-08T20:20:50Z

Codecov Report

Merging #189 (fb7e19f) into main (4aa81dd) will decrease coverage by 0.97%.
The diff coverage is 92.85%.

❗ Current head fb7e19f differs from pull request most recent head 200ce5d. Consider uploading reports for the commit 200ce5d to get more accurate results

@@            Coverage Diff             @@
##             main     #189      +/-   ##
==========================================
- Coverage   92.81%   91.83%   -0.98%     
==========================================
  Files          18       19       +1     
  Lines         487      588     +101     
==========================================
+ Hits          452      540      +88     
- Misses         35       48      +13

Impacted Files	Coverage Δ
dvclive/error.py	`84.00% <40.00%> (-11.00%)`	⬇️
dvclive/live.py	`96.37% <91.42%> (-1.90%)`	⬇️
dvclive/data/base.py	`91.80% <93.54%> (+0.13%)`	⬆️
dvclive/data/plot.py	`94.64% <94.64%> (ø)`
dvclive/data/__init__.py	`100.00% <100.00%> (ø)`
dvclive/data/image.py	`90.90% <100.00%> (-9.10%)`	⬇️
dvclive/data/scalar.py	`100.00% <100.00%> (ø)`
dvclive/version.py	`72.22% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4aa81dd...200ce5d. Read the comment docs.

pared · 2021-11-09T10:57:52Z

The integration looks alright. One question that I have is whether we want to promote logging confusion matrix as image file. While we do support images on DVC side, I am not sure we should promote that.
Alternatives:

just dump matrix data, but that will only work with dvc/visualization lib that we might extract from DVC
dump the confusion matrix as vega spec, start supporting that on dvc side - that is probably worse than previous since we are thinking about supporting other libraries

daavoo · 2021-11-09T16:30:36Z

The integration looks alright. One question that I have is whether we want to promote logging confusion matrix as image file. While we do support images on DVC side, I am not sure we should promote that. Alternatives:
* just dump matrix data, but that will only work with dvc/visualization lib that we might extract from DVC

I think I implemented it as an image mainly for saving users from having to set custom plot properties (i.e. template: confusion) and for some additional features provided by the sklearn plot (i.e. set custom display_labels).

However, the other functions save the data in "DVC plots format", requiring custom plot properties as well, so it would be consistent.

pared · 2021-11-09T16:39:25Z

Well, maybe it's another reason to reconsider iterative/dvc#6944 - maybe we should discuss somehow providing config for each image which would define how to plot it?

dvclive/sklearn.py

dberenbaum · 2021-11-12T20:09:40Z

Agree with @pared about the confusion matrix and plots in general. Seems better to save the data and work on making it easier to specify plots properties.

For the other plots, it seems awkward to have to call them in dvclive and configure them in dvc.

Also, I'm not sure how much value there is in these functions that lightly wrap existing sklearn functions. Seems more useful to do one or both of:

log_data() to log any of these results. We can work on making it easy to show those results as plots, and it gives users flexibility to keep the raw results to process themselves.
Some sort of auto-logging at a higher level for sklearn (similar to https://www.mlflow.org/docs/latest/python_api/mlflow.sklearn.html#mlflow.sklearn.autolog) so users have an easy default logger.

What do you think?

daavoo · 2021-12-20T18:19:15Z

Revisiting this.

Also, I'm not sure how much value there is in these functions that lightly wrap existing sklearn functions. Seems more useful to do one or both of:
* `log_data()` to log any of these results. We can work on making it easy to show those results as plots, and it gives users flexibility to keep the raw results to process themselves.

I don't fully get log_data (perhaps we discussed offline and I forgot 😓 ) . Could you describe would it work (on a high-level)?

* Some sort of auto-logging at a higher level for sklearn (similar to https://www.mlflow.org/docs/latest/python_api/mlflow.sklearn.html#mlflow.sklearn.autolog) so users have an easy default logger.

What do you think?

About the value of these light wraps, I don't really know.

My view is that there are many stages between this light wraps and automagic logging "a la mlflow".

I would say that a good intermediate would be log_classifier ("a la wandb" https://docs.wandb.ai/guides/integrations/scikit).

However, it seems that it's up to user taste and there is no real need to don't support any stage. Automagic methods are going to be using something similar to (if not exactly) these light wraps so, why not expose all different levels of granularity?

We could start with this low level integration and incrementally add magic

dberenbaum · 2021-12-29T21:23:39Z

I'm still unsure about this. My hesitations are:

The included methods are a pretty arbitrary selection. How do we decide what to include, and do we want to potentially support wrappers for every kind of sklearn metric/plot?
There's no obvious way to include this (or any sklearn integration) as a sort of integrated callback like other frameworks that automatically logs info as the model trains.
If we eventually want something like log_classifier, the closest thing AFAIK is https://scikit-learn.org/stable/modules/generated/sklearn.metrics.classification_report.html#sklearn.metrics.classification_report. This wouldn't even utilize these methods I don't think.

Another option is to add support solely via docs (once #203 is merged), similar to https://dvc.org/doc/dvclive/ml-frameworks/tensorflow.

How much value do you think the current PR adds, and how much added value do you see in further integration?

daavoo · 2021-12-30T14:25:28Z

1. The included methods are a pretty arbitrary selection. How do we decide what to include, and do we want to potentially support wrappers for every kind of sklearn metric/plot?

I would not say it's arbitrary. It's not complete, for sure, but includes 3 out of the 6 visualizations supported in scikit-learn (https://scikit-learn.org/stable/visualizations.html).

I would support any scikit-learn visualization that can be seamlessly integrated with dvc plots usage. Under that condition, the only missing from the list would be the det_curve (could be added in the P.R, I just forgot)

It's a similar selection to the integrations in https://docs.wandb.ai/guides/integrations/scikit . The difference is that some of those plots types are not directly integrable with dvc plots and thus I don't see much sense in adding support.

2. There's no obvious way to include this (or any sklearn integration) as a sort of integrated callback like other frameworks that automatically logs info as the model trains.

Indeed, but it's a common limitation for any logger that doesn't perform the "magic" patching. This is addressable on docs, similar to the existing barebones PyTorch and TensorFlow guides.

3. If we eventually want something like `log_classifier`, the closest thing AFAIK is https://scikit-learn.org/stable/modules/generated/sklearn.metrics.classification_report.html#sklearn.metrics.classification_report. This wouldn't even utilize these methods I don't think.

With log_classifier/log_regressor I was thinking something more in the lines of https://docs.wandb.ai/guides/integrations/scikit#or-visualize-all-plots-at-once which would call internally this log_{X} functions.

Potentially, after #203, this could be extended to include live.log for multiple classification/regression metrics but I don't see how these are mutually exclusive.

Another option is to add support solely via docs (once #203 is merged), similar to https://dvc.org/doc/dvclive/ml-frameworks/tensorflow.

How much value do you think the current PR adds, and how much added value do you see in further integration?

I don't / haven't really use scikit-learn beyond toy projects, so I have a lack of context on what it's valuable in real scenarios 😅

I see immediate value to be used in https://github.com/iterative/example-get-started to reduce code and promote dvclive.
I think it prevents users from having to be aware of the format dvc plots expects.

I don't see how different these functions are from the basic usage of dvclive, where live.log is just saving lines of code and hiding the expected dvc format from the user.

Added value in further integration should move towards "magic" patching and/or log_classifier. These functions can still have value there both for internal usage but also for users preferring more fine-grained control on what to log.

Given that the inputs are arrays of y_true/y_pred, my main doubt is whether we should explicitly save this under sklearn or consider this a new plots module that has sklearn as an optional dependency.

dberenbaum · 2022-01-01T19:47:10Z

I would not say it's arbitrary. It's not complete, for sure, but includes 3 out of the 6 visualizations supported in scikit-learn (https://scikit-learn.org/stable/visualizations.html).

I would support any scikit-learn visualization that can be seamlessly integrated with dvc plots usage. Under that condition, the only missing from the list would be the det_curve (could be added in the P.R, I just forgot)

It's a similar selection to the integrations in https://docs.wandb.ai/guides/integrations/scikit . The difference is that some of those plots types are not directly integrable with dvc plots and thus I don't see much sense in adding support.

Thanks for the explanation. That makes sense.

Why can't calibration curves and partial dependence plots also be supported in DVC? They are all linear unless I'm missing something.

With log_classifier/log_regressor I was thinking something more in the lines of https://docs.wandb.ai/guides/integrations/scikit#or-visualize-all-plots-at-once which would call internally this log_{X} functions.

I think this is where we had different ideas for sklearn integration. I thought we would first need to capture the scalar metrics before worrying about plots. I guess you are thinking that the plots are harder for the user to log, and they can easily log scalar metrics already? Still, it would be nice to automatically capture all of the relevant scalar metrics (in addition to plots) if there is some higher-level integration.

Given that the inputs are arrays of y_true/y_pred, my main doubt is whether we should explicitly save this under sklearn or consider this a new plots module that has sklearn as an optional dependency.

Are you suggesting that these could be generic functions rather than specific to sklearn? I have previously kept a mini-library of classification metrics/plots to have a consistent, lightweight way to evaluate models across ML frameworks. Maybe that's more useful here than any sklearn-specific integration. It's definitely a different direction for dvclive, but it might be worth considering.

daavoo · 2022-01-03T21:24:45Z

Why can't calibration curves and partial dependence plots also be supported in DVC? They are all linear unless I'm missing something.

You are right, calibration curves can be supported. And also partial dependence, but only in "average" mode.

Still, it would be nice to automatically capture all of the relevant scalar metrics (in addition to plots) if there is some higher-level integration.

I agree, after #203 , a higher level log_classifier would also log scalar metrics.

Are you suggesting that these could be generic functions rather than specific to sklearn? I have previously kept a mini-library of classification metrics/plots to have a consistent, lightweight way to evaluate models across ML frameworks. Maybe that's more useful here than any sklearn-specific integration. It's definitely a different direction for dvclive, but it might be worth considering.

We can start documenting as part of sklearn integration but I can see the plots being useful for basically any other framework that supports some classification task (so, pretty much, all).

dberenbaum · 2022-01-11T16:27:46Z

Should these be logged inside the dvclive dir?

dvclive/plots.py

dberenbaum · 2022-01-14T17:25:48Z

Ultimately, I think the workflow for this should depend on #82 and #203. Right now, it's impossible to log from within a Live() instance, which makes it a bit awkward to use. I would expect a workflow like:

live = dvclive.Live()
live.log_calibration(y_test, y_score, "calibration.json")

Do you think it's worth merging as is and then modifying it after? I think it will require breaking changes to address those issues.

dberenbaum · 2022-01-14T18:14:46Z

Do you mention #82 because you consider that these plots should be loggable at every step?

I mention it because this PR seems like a specific application of #82. Similar to #166, it seems that we will need to decide on the canonical workflow and directory structure for this type of data in both non-step and step-based workflows. So, we can try to decide that now, or we can merge knowing we will need to break it later.

daavoo

Moved all the logic to Live.log and made plots a new "data type".

So the idea is that Live.log can log any of the 3 supported data types: scalars, images, and plots.

Scalar

live.log("accuracy", 0.9)

Image

img = np.ones((500, 500, 3), np.uint8)
live.log("image.png", img)

Plot

live.log("roc_curve.json", (y_true, y_score, "roc"))
live.log("cm.json", (y_true, y_pred, "confusion_matrix"))

The internals make sense for me and the no step / step logic is resolved equally for images and plots.

However, I'm not sure how intuitive the API for plots is.

It could be a matter of documentation or it could be better to have explicitly separated methods for each data type log_metric, log_image, log_plot.

I kind of prefer separate methods.

pared · 2022-01-18T11:52:22Z

I think that separate methods make sense too, as the distinction in use will be seen at first sight. I am not sure we should rename log to log_metric.

dberenbaum · 2022-01-18T15:33:30Z

Nice! I agree with @pared that it seems best to have separate methods but leave log for metrics.

dberenbaum · 2022-01-28T15:41:18Z

Do you mention #82 because you consider that these plots should be loggable at every step?

Returning to this, yes, I think it should be possible to see how the ROC curve changes at each step, for example. Right now, saving the data works, but it's almost impossible to configure dvc.yaml to show the plots, and in default mode they make it hard to even see the regular metrics history plots.

daavoo · 2022-01-31T12:05:44Z

Returning to this, yes, I think it should be possible to see how the ROC curve changes at each step, for example. Right now, saving the data works, but it's almost impossible to configure dvc.yaml to show the plots, and in default mode they make it hard to even see the regular metrics history plots.

The initial scope for this kind of plot was for non-step.

Properly supporting multi-step curves would require a new template would be required on DVC side using facet for comparing revision.

dberenbaum · 2022-01-31T13:45:40Z

The initial scope for this kind of plot was for non-step.

Properly supporting multi-step curves would require a new template would be required on DVC side using facet for comparing revision.

Okay, let's discuss separately in #82 or elsewhere the per-step scenario. Why support logging at each step then in this PR? Should an error be thrown if using log_plot per step?

daavoo · 2022-02-01T15:15:05Z

Why support logging at each step then in this PR?

No strong reason, it just reuses the logic already present for images.

Should an error be thrown if using log_plot per step?

Will do

Decouple data type logging into separated methods. Use subfolders for each data type. Raise NotImplementedError in `log_plot` when using steps.

dberenbaum · 2022-02-02T15:36:44Z

Sorry for the confusion over step/no-step scenarios. Limiting to no-step makes sense.

jorgeorpinel · 2022-02-22T04:59:00Z

dvclive/live.py

+    def log_image(self, name: str, val):
+        if not Image.could_log(val):
+            raise InvalidDataTypeError(name, type(val))


Maybe val should be renamed. val made sense for scalars (log()) but image_data or something more descriptive would be better here IMO.

jorgeorpinel · 2022-02-22T05:15:24Z

dvclive/live.py

+    def log_plot(self, name, labels, predictions, **kwargs):
+        val = (labels, predictions)


Same here. plot_data?

jorgeorpinel · 2022-02-22T05:15:38Z

dvclive/live.py

+        if name in self._images:
+            data = self._images[name]
+        else:
+            data = Image(name, self.dir)
+            self._images[name] = data


As for name, should it be filename or out instead? Again for clarity

jorgeorpinel · 2022-02-22T05:16:01Z

dvclive/live.py

+        if name in self._plots:
+            data = self._plots[name]
+        elif name in PLOTS and PLOTS[name].could_log(val):
+            data = PLOTS[name](name, self.dir)
+            self._plots[name] = data
+        else:
+            raise InvalidPlotTypeError(name)


Same. ~~filename? plot_fname?~~

Or I guess (plot_)type would be most accurate here.

daavoo requested review from dberenbaum and pared November 8, 2021 20:15

dberenbaum reviewed Nov 12, 2021

View reviewed changes

dvclive/sklearn.py Outdated Show resolved Hide resolved

daavoo force-pushed the sklearn branch 2 times, most recently from 04ba8a0 to c511533 Compare December 28, 2021 16:05

daavoo requested a review from dberenbaum December 28, 2021 16:05

daavoo force-pushed the sklearn branch 2 times, most recently from 4ead8a4 to 5845af9 Compare December 29, 2021 19:49

daavoo mentioned this pull request Dec 29, 2021

dvclive: Add plots iterative/dvc.org#3142

Closed

daavoo self-assigned this Dec 29, 2021

daavoo force-pushed the sklearn branch from 5845af9 to 70b712c Compare January 3, 2022 21:47

daavoo force-pushed the sklearn branch 3 times, most recently from 8fdd0f8 to a6aab0f Compare January 11, 2022 08:57

daavoo changed the title ~~sklearn: Add basic logging methods.~~ Add plots module. Jan 11, 2022

daavoo force-pushed the sklearn branch from a6aab0f to 3a0d414 Compare January 11, 2022 11:07

pared approved these changes Jan 13, 2022

View reviewed changes

dberenbaum reviewed Jan 14, 2022

View reviewed changes

dvclive/plots.py Outdated Show resolved Hide resolved

daavoo force-pushed the sklearn branch from 3a0d414 to 57ecf3f Compare January 14, 2022 18:40

daavoo commented Jan 17, 2022

View reviewed changes

daavoo force-pushed the sklearn branch 2 times, most recently from 6f9b868 to 2ac8394 Compare January 17, 2022 22:02

dberenbaum mentioned this pull request Jan 20, 2022

dvclive: Updates for implicit no step iterative/dvc.org#3147

Merged

daavoo mentioned this pull request Jan 21, 2022

dvclive: Add log_image and log_plot iterative/dvc.org#3207

Merged

dberenbaum mentioned this pull request Jan 25, 2022

dvclive.log: log array, tensors and similar objects #82

Closed

daavoo changed the title ~~Add plots module.~~ live: Add log_image and log_plot. Jan 28, 2022

daavoo force-pushed the sklearn branch from 5b66006 to 893d5a0 Compare January 28, 2022 13:00

daavoo force-pushed the sklearn branch from 893d5a0 to 81b590e Compare January 31, 2022 12:12

daavoo mentioned this pull request Jan 31, 2022

get-started: more plots, dvclive, better dataset iterative/example-repos-dev#102

Closed

1 task

daavoo force-pushed the sklearn branch from 81b590e to 200ce5d Compare January 31, 2022 14:56

daavoo force-pushed the sklearn branch from 200ce5d to 0a3ab90 Compare February 1, 2022 16:35

live: Add log_image and log_plot.

c69af09

Decouple data type logging into separated methods. Use subfolders for each data type. Raise NotImplementedError in `log_plot` when using steps.

daavoo force-pushed the sklearn branch from 0a3ab90 to c69af09 Compare February 2, 2022 11:15

dberenbaum approved these changes Feb 2, 2022

View reviewed changes

daavoo merged commit e4bc27a into main Feb 2, 2022

daavoo deleted the sklearn branch February 2, 2022 18:47

jorgeorpinel reviewed Feb 22, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

live: Add `log_image` and `log_plot`. #189

live: Add `log_image` and `log_plot`. #189

daavoo commented Nov 8, 2021 •

edited

Loading

codecov-commenter commented Nov 8, 2021 •

edited

Loading

pared commented Nov 9, 2021

daavoo commented Nov 9, 2021

pared commented Nov 9, 2021

dberenbaum commented Nov 12, 2021

daavoo commented Dec 20, 2021 •

edited

Loading

dberenbaum commented Dec 29, 2021

daavoo commented Dec 30, 2021 •

edited

Loading

dberenbaum commented Jan 1, 2022

daavoo commented Jan 3, 2022

dberenbaum commented Jan 11, 2022

dberenbaum commented Jan 14, 2022

dberenbaum commented Jan 14, 2022

daavoo left a comment

pared commented Jan 18, 2022

dberenbaum commented Jan 18, 2022

dberenbaum commented Jan 28, 2022

daavoo commented Jan 31, 2022

dberenbaum commented Jan 31, 2022

daavoo commented Feb 1, 2022 •

edited

Loading

dberenbaum commented Feb 2, 2022

jorgeorpinel Feb 22, 2022 •

edited

Loading

jorgeorpinel Feb 22, 2022

jorgeorpinel Feb 22, 2022

jorgeorpinel Feb 22, 2022 •

edited

Loading

jorgeorpinel Feb 22, 2022 •

edited

Loading

		def log_plot(self, name, labels, predictions, **kwargs):
		val = (labels, predictions)

live: Add log_image and log_plot. #189

live: Add log_image and log_plot. #189

Conversation

daavoo commented Nov 8, 2021 • edited Loading

codecov-commenter commented Nov 8, 2021 • edited Loading

Codecov Report

pared commented Nov 9, 2021

daavoo commented Nov 9, 2021

pared commented Nov 9, 2021

dberenbaum commented Nov 12, 2021

daavoo commented Dec 20, 2021 • edited Loading

dberenbaum commented Dec 29, 2021

daavoo commented Dec 30, 2021 • edited Loading

dberenbaum commented Jan 1, 2022

daavoo commented Jan 3, 2022

dberenbaum commented Jan 11, 2022

dberenbaum commented Jan 14, 2022

dberenbaum commented Jan 14, 2022

daavoo left a comment

Choose a reason for hiding this comment

pared commented Jan 18, 2022

dberenbaum commented Jan 18, 2022

dberenbaum commented Jan 28, 2022

daavoo commented Jan 31, 2022

dberenbaum commented Jan 31, 2022

daavoo commented Feb 1, 2022 • edited Loading

dberenbaum commented Feb 2, 2022

jorgeorpinel Feb 22, 2022 • edited Loading

Choose a reason for hiding this comment

jorgeorpinel Feb 22, 2022

Choose a reason for hiding this comment

jorgeorpinel Feb 22, 2022

Choose a reason for hiding this comment

jorgeorpinel Feb 22, 2022 • edited Loading

Choose a reason for hiding this comment

jorgeorpinel Feb 22, 2022 • edited Loading

Choose a reason for hiding this comment

live: Add `log_image` and `log_plot`. #189

live: Add `log_image` and `log_plot`. #189

daavoo commented Nov 8, 2021 •

edited

Loading

codecov-commenter commented Nov 8, 2021 •

edited

Loading

daavoo commented Dec 20, 2021 •

edited

Loading

daavoo commented Dec 30, 2021 •

edited

Loading

daavoo commented Feb 1, 2022 •

edited

Loading

jorgeorpinel Feb 22, 2022 •

edited

Loading

jorgeorpinel Feb 22, 2022 •

edited

Loading

jorgeorpinel Feb 22, 2022 •

edited

Loading