Allow for logging to Studio when not inside a repo #646

dberenbaum · 2023-07-31T21:13:08Z

Partially addresses #638. This does not solve how to copy metrics back to the repo or make the experience completely smooth everywhere, but the goal for this PR is to achieve parity with other trackers where a token and project are all that's needed.

This allows for posting to Studio if the following environment variables are set:

DVC_STUDIO_TOKEN
DVC_STUDIO_REPO_URL
~~DVC_EXP_BASELINE_REV~~
~~DVC_EXP_NAME~~

TODO:

Make DVC_EXP_BASELINE_REV and/or DVC_EXP_NAME optional
Example
- Colab
- Databricks
- Studio
- SageMaker and Studio
Docs
Update branch

codecov-commenter · 2023-07-31T21:22:15Z

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (aba8807) 95.55% compared to head (5003104) 95.51%.

Files	Patch %	Lines
src/dvclive/studio.py	87.50%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #646      +/-   ##
==========================================
- Coverage   95.55%   95.51%   -0.05%     
==========================================
  Files          55       55              
  Lines        3532     3542      +10     
  Branches      314      317       +3     
==========================================
+ Hits         3375     3383       +8     
- Misses        110      111       +1     
- Partials       47       48       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dberenbaum · 2023-07-31T21:31:28Z

Need to look into windows tests not passing. Somehow dvclive expects to pass posix paths even on windows, but not sure where that conversion happens.

daavoo · 2023-08-01T14:29:42Z

Somehow dvclive expects to pass posix paths even on windows, but not sure where that conversion happens.

This is because Studio expected the paths in POSIX format.
There was a bug where live metrics sent from Windows were not being shown in Studio.

It was added here:

dvclive/src/dvclive/studio.py

Lines 28 to 30 in 36e8c4c

    
           def _rel_path(path, dvc_root_path): 
        
               absolute_path = Path(path).resolve() 
        
               return str(absolute_path.relative_to(dvc_root_path).as_posix())

dberenbaum · 2023-08-04T18:27:14Z

Need to look into windows tests not passing. Somehow dvclive expects to pass posix paths even on windows, but not sure where that conversion happens.

The cause was because the conversion only happens within a repo, which brought up that we need to decide where paths should be relative to if there is no repo root. I went with cwd for now.

daavoo · 2023-08-04T19:09:53Z

Need to look into windows tests not passing. Somehow dvclive expects to pass posix paths even on windows, but not sure where that conversion happens.

The cause was because the conversion only happens within a repo, which brought up that we need to decide where paths should be relative to if there is no repo root. I went with cwd for now.

I see. Probably no need to do with cwd (not even sure if it might be potentially problematic)

The posix and relpath operations should be decoupled but I mixed them in a single function (they were actually 2 separate bugfixes one for windows other for monorepo in Studio).
I assume that in norepo scenario we don't really need relpath operation because the default values are already as expected but we do need posix

src/dvclive/dvc.py

dberenbaum · 2023-08-04T20:51:09Z

Make DVC_EXP_BASELINE_REV and/or DVC_EXP_NAME optional

This depends on:

exp: abstract random name gen from repo dvc#9806 (for exp name)
Allow for null baseline_sha dvc-studio-client#72 (for baseline rev)
Studio work to allow for requests with a missing baseline rev (these would have to be permanently "detached" unless we later find some way to connect them back to a baseline)

dberenbaum · 2023-08-04T21:24:52Z

I see. Probably no need to do with cwd (not even sure if it might be potentially problematic)

Updated to remove that.

dberenbaum · 2023-08-04T21:28:30Z

Example

See https://colab.research.google.com/drive/17X5OyyUi73PXGyZl_87Pl4yUoNS5lMWn?usp=sharing

shcheklein · 2023-08-04T21:34:45Z

@dberenbaum do you have a sense on what is the mid-term plan for this? If we are not attached to Git - what is the benefit of using the DVC ecosystem? (sorry, I know it's a non-trivial question, definitely not a blocker of this PR or effort). Data management? something else?

Is there a way to clone the repo in those system under the hood / seamlessly for end-users?

Do we want to cover scenarios like when people don't use Git at all - they don't have a repo at all even? (we pretty much then become two products - Git based and separately MlFlow/W&B, right?

dberenbaum · 2023-08-04T21:39:55Z

Do we want to cover scenarios like when people don't use Git at all - they don't have a repo at all even? (we pretty much then become two products - Git based and separately MlFlow/W&B, right?

It's not in my plans at this point. This is mainly to cover scenarios where you have a project in a repo but want to run an experiment in an environment where it's painful/impossible to have a full copy of the Git repo. In my experience, it's not uncommon to have a notebook that gets checked into Git but that runs independently in databricks, colab, etc. It's also helpful for the SageMaker scenario where you can kick off a job on a remote machine, but that machine only has access to a single .py file, so this way you can at least see metrics populated in Studio.

shcheklein · 2023-08-04T22:24:47Z

Okay, I see. What is the plan of connecting it back? Those experiments will stay "studio"-only, is it correct?

And why can't we try to do a git clone? any specific obstacles for that?

shcheklein · 2024-02-07T00:20:41Z

@mattseddon i think you should be aware by this change as well (baseline is optional)

dberenbaum · 2024-02-07T01:22:07Z

@mattseddon i think you should be aware by this change as well (baseline is optional)

In the end, baseline is still required to be sent to dvc-studio-client in this PR and defaults to "0" * 40. It seems this is an accepted convention for a null ref (see https://stackoverflow.com/questions/18977462/git-sha1-0000000000000000000000000000000000000000-all-zeroes-is-this-normal).

src/dvclive/live.py

dberenbaum · 2024-02-07T18:34:02Z

src/dvclive/dvc.py

@@ -131,9 +132,14 @@ def _update_entries(old, new, key):
 def get_exp_name(name, scm, baseline_rev) -> str:


Updated get_exp_name to always be able to return an experiment name instead of only when inside a repo.

dberenbaum · 2024-02-07T18:39:08Z

src/dvclive/live.py

+        scm = self._dvc_repo.scm if self._dvc_repo else None
+        if isinstance(scm, NoSCM):
+            scm = None
+        if scm:
+            self._baseline_rev = scm.get_rev()
+        self._exp_name = get_exp_name(self._exp_name, scm, self._baseline_rev)
+        logger.info(f"Logging to experiment '{self._exp_name}'")


Once we have tried to find a repo, we can generate all exp info we need instead of scattering it throughout the code

src/dvclive/studio.py

src/dvclive/utils.py

dberenbaum · 2024-02-07T19:34:07Z

tests/test_dvc.py

+    assert live._baseline_rev is not None
+    assert live._exp_name is not None


These now get set always

dberenbaum · 2024-02-07T19:35:16Z

tests/test_dvc.py

@@ -60,31 +57,6 @@ def test_exp_save_skip_on_env_vars(tmp_dir, monkeypatch, mocker):
    assert live._inside_dvc_pipeline


-def test_exp_save_run_on_dvc_repro(tmp_dir, mocker):


This test was actually outdated and not working. It conflicts with test_dvc_repro below, so I consolidated the meaningful assertions into that test.

dberenbaum · 2024-02-07T19:39:35Z

tests/test_dvc.py

    monkeypatch.setenv(DVC_EXP_BASELINE_REV, "foo")
    monkeypatch.setenv(DVC_EXP_NAME, "bar")
    monkeypatch.setenv(DVC_ROOT, tmp_dir)

-    mocker.patch("dvclive.live.get_dvc_repo", return_value=None)


dberenbaum · 2024-02-07T19:39:42Z

tests/test_dvc.py

@@ -166,7 +138,7 @@ def test_errors_on_git_add_are_catched(tmp_dir, mocked_dvc_repo, monkeypatch):
    mocked_dvc_repo.scm.untracked_files.return_value = ["dvclive/metrics.json"]
    mocked_dvc_repo.scm.add.side_effect = DvcException("foo")

-    with Live(dvcyaml=False) as live:


src/dvclive/live.py

dberenbaum · 2024-02-07T19:41:33Z

tests/test_dvc.py

+    assert live._baseline_rev is not None
+    assert live._exp_name is not None
+    assert not live._studio_events_to_skip


Consolidated from the deleted test_exp_save_run_on_dvc_repro above. On repro, everything will still work except actually saving an exp ref.

dberenbaum · 2024-02-07T19:43:22Z

tests/test_post_to_studio.py

-def test_post_to_studio_requires_exp(tmp_dir, mocked_dvc_repo, mocked_studio_post):
-    assert Live(save_dvc_exp=False)._studio_events_to_skip == {"start", "data", "done"}
-    assert not Live()._studio_events_to_skip
+def test_post_to_studio_without_exp(tmp_dir, mocked_dvc_repo, mocked_studio_post):
+    assert not Live(save_dvc_exp=False)._studio_events_to_skip


Live updates to Studio will happen regardless of save_dvc_exp value.

shcheklein · 2024-02-12T00:58:11Z

src/dvclive/live.py

@@ -571,7 +558,8 @@ def make_report(self):

    @catch_and_warn(DvcException, logger)
    def make_dvcyaml(self):
-        make_dvcyaml(self)
+        if self.dvc_file:


what is the case for this?

I mean when can it be None?

this is the only (?) unresolved question? Approved, since it doesn't look anything major ...

good stuff, Dave. thanks!

Sorry, missed this one. Good catch. Maybe this came from an earlier iteration. I dropped this condition in the latest commit.

dberenbaum added 2 commits July 31, 2023 16:00

post to studio even without git/dvc repo

2160ae4

tests for no-git scenario

8653732

studio: make no-repo paths relative to cwd

ca57b17

make ruff happy

98ae256

don't require exp name

df1a20b

dberenbaum mentioned this pull request Aug 4, 2023

exp: abstract random name gen from repo iterative/dvc#9806

Merged

dberenbaum commented Aug 4, 2023

View reviewed changes

src/dvclive/dvc.py Outdated Show resolved Hide resolved

dberenbaum added 2 commits August 4, 2023 16:42

don't require baseline rev

9ddcf07

Merge branch 'main' into no-git

bf01031

dberenbaum mentioned this pull request Aug 4, 2023

Allow for null baseline_sha iterative/dvc-studio-client#72

Closed

refactor studio path formatting

3c68cb6

dberenbaum mentioned this pull request Aug 5, 2023

remote/no-git logging #638

Closed

daavoo added the A: studio Area: Studio integration label Aug 14, 2023

daavoo assigned dberenbaum Aug 14, 2023

daavoo and others added 5 commits August 22, 2023 12:33

live: Set new defaults report=None and save_dvc_exp=True.

f6a0a29

frameworks: Drop model_file.

aa3610a

update examples

2e538ce

Merge branch 'main' into 484-30-release

0d21bb6

fix merge conflicts

bb34d87

mattseddon reviewed Feb 7, 2024

View reviewed changes

src/dvclive/live.py Outdated Show resolved Hide resolved

AlexandreKempf approved these changes Feb 7, 2024

View reviewed changes

dberenbaum added 2 commits February 7, 2024 13:23

consolidate repro tests

ade1b3d

set null sha as variable

56acdae

dberenbaum commented Feb 7, 2024

View reviewed changes

src/dvclive/studio.py Outdated Show resolved Hide resolved

dberenbaum commented Feb 7, 2024

View reviewed changes

src/dvclive/utils.py Outdated Show resolved Hide resolved

dberenbaum commented Feb 7, 2024

View reviewed changes

shcheklein reviewed Feb 7, 2024

View reviewed changes

src/dvclive/live.py Show resolved Hide resolved

dberenbaum commented Feb 7, 2024

View reviewed changes

shcheklein reviewed Feb 12, 2024

View reviewed changes

dberenbaum added 3 commits February 13, 2024 10:36

add type hints to studio

33288cd

limit windows path handling to studio

73357b2

merge

b5ff21d

dberenbaum force-pushed the no-git branch from ec3e83d to b5ff21d Compare February 13, 2024 15:53

dberenbaum added 2 commits February 13, 2024 11:14

fix typing errors in studio module

53e77a6

fix mypy in live module

bea5a83

shcheklein approved these changes Feb 13, 2024

View reviewed changes

dberenbaum added 3 commits February 13, 2024 14:52

drop checking for dvc_file

bd431da

Merge branch 'main' into no-git

5003104

Merge branch 'main' into no-git

a4cae82

dberenbaum merged commit 35ce2f3 into main Feb 15, 2024
14 checks passed

dberenbaum deleted the no-git branch February 15, 2024 17:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow for logging to Studio when not inside a repo #646

Allow for logging to Studio when not inside a repo #646

dberenbaum commented Jul 31, 2023 •

edited

Loading

codecov-commenter commented Jul 31, 2023 •

edited

Loading

dberenbaum commented Jul 31, 2023

daavoo commented Aug 1, 2023 •

edited

Loading

dberenbaum commented Aug 4, 2023

daavoo commented Aug 4, 2023

dberenbaum commented Aug 4, 2023

dberenbaum commented Aug 4, 2023

dberenbaum commented Aug 4, 2023

shcheklein commented Aug 4, 2023

dberenbaum commented Aug 4, 2023

shcheklein commented Aug 4, 2023

shcheklein commented Feb 7, 2024

dberenbaum commented Feb 7, 2024 •

edited

Loading

dberenbaum Feb 7, 2024

dberenbaum Feb 7, 2024

dberenbaum Feb 7, 2024

dberenbaum Feb 7, 2024

dberenbaum Feb 7, 2024

dberenbaum Feb 7, 2024

dberenbaum Feb 7, 2024

dberenbaum Feb 7, 2024

shcheklein Feb 12, 2024

shcheklein Feb 12, 2024

shcheklein Feb 13, 2024

dberenbaum Feb 13, 2024

		@@ -131,9 +132,14 @@ def _update_entries(old, new, key):
		def get_exp_name(name, scm, baseline_rev) -> str:

		assert live._baseline_rev is not None
		assert live._exp_name is not None

		@@ -60,31 +57,6 @@ def test_exp_save_skip_on_env_vars(tmp_dir, monkeypatch, mocker):
		assert live._inside_dvc_pipeline


		def test_exp_save_run_on_dvc_repro(tmp_dir, mocker):

Allow for logging to Studio when not inside a repo #646

Allow for logging to Studio when not inside a repo #646

Conversation

dberenbaum commented Jul 31, 2023 • edited Loading

codecov-commenter commented Jul 31, 2023 • edited Loading

Codecov Report

dberenbaum commented Jul 31, 2023

daavoo commented Aug 1, 2023 • edited Loading

dberenbaum commented Aug 4, 2023

daavoo commented Aug 4, 2023

dberenbaum commented Aug 4, 2023

dberenbaum commented Aug 4, 2023

dberenbaum commented Aug 4, 2023

shcheklein commented Aug 4, 2023

dberenbaum commented Aug 4, 2023

shcheklein commented Aug 4, 2023

shcheklein commented Feb 7, 2024

dberenbaum commented Feb 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dberenbaum commented Jul 31, 2023 •

edited

Loading

codecov-commenter commented Jul 31, 2023 •

edited

Loading

daavoo commented Aug 1, 2023 •

edited

Loading

dberenbaum commented Feb 7, 2024 •

edited

Loading