Repository: wandb/wandb
Pull request: fix: windows throws ValueError when running from a different drive.
Description
-----------
<!--
Include reference to internal ticket "Fixes WB-NNNNN" and/or GitHub issue "Fixes #NNNN" (if applicable)
-->

- Fixes: #9676, #9349 
- Fixes: WB-24266, WB-23175, WB-7853

What does the PR do? Include a concise description of the PR contents.

When running a W&B script on Windows from a different drive than the drive the script is located on will cause a ValueError when attempting to determine the relative [path of the program](https://github.com/wandb/wandb/blob/main/wandb/sdk/wandb_settings.py#L1669)

This change checks the drives for both the root directory and the program file location to determine if they both exist on the same drive.

<!--
NEW: We're using a new changelog format that's more useful for users. Please
see CHANGELOG.unreleased.md for details and update on relevant changes such as feature
additions, bug fixes, or removals/deprecations.
-->
- [x] I updated CHANGELOG.unreleased.md, or it's not applicable


Testing
-------
How was this PR tested?

<!--
Ensure PR title compliance with the [conventional commits standards](https://github.com/wandb/wandb/blob/main/CONTRIBUTING.md#conventional-commits)
-->


Problem statement:
[Bug]:
### Describe the bug

Hi, I'm getting an error in wandb version 0.19.9.

```
Traceback (most recent call last):

------------MY CUSTOM CODE---------------------
  File "c:\users\paul boursin\desktop\dynamical\ml-agents\ml-agents\mlagents\plugins\stats_writer.py", line 64, in register_stats_writer_plugins
    plugin_stats_writers = plugin_func(run_options)
  File "c:\users\paul boursin\desktop\dynamical\ml-agents\ml-agents\mlagents\plugins\stats_writer.py", line 39, in get_default_stats_writers
    WandbWriter(run_options)
  File "c:\users\paul boursin\desktop\dynamical\ml-agents\ml-agents\mlagents\trainers\stats.py", line 302, in __init__
    wandb.setup(wandb.Settings(program=__name__, program_relpath=__name__))

------------WANDB PYTHON CODE-----------------
  File "C:\Users\Paul Boursin\anaconda3\envs\env\lib\site-packages\wandb\sdk\wandb_setup.py", line 383, in setup
    return _setup(settings=settings)
  File "C:\Users\Paul Boursin\anaconda3\envs\env\lib\contextlib.py", line 75, in inner
    return func(*args, **kwds)
  File "C:\Users\Paul Boursin\anaconda3\envs\env\lib\site-packages\wandb\sdk\wandb_setup.py", line 319, in _setup
    _singleton = _WandbSetup(settings=settings, pid=pid)
  File "C:\Users\Paul Boursin\anaconda3\envs\env\lib\site-packages\wandb\sdk\wandb_setup.py", line 96, in __init__
    self._settings = self._settings_setup(settings)
  File "C:\Users\Paul Boursin\anaconda3\envs\env\lib\site-packages\wandb\sdk\wandb_setup.py", line 130, in _settings_setup
    s.update_from_system_environment()
  File "C:\Users\Paul Boursin\anaconda3\envs\env\lib\site-packages\wandb\sdk\wandb_settings.py", line 1532, in update_from_system_environment
    self.program_relpath = self.program_relpath or self._get_program_relpath(
  File "C:\Users\Paul Boursin\anaconda3\envs\env\lib\site-packages\wandb\sdk\wandb_settings.py", line 1676, in _get_program_relpath
    relative_path = os.path.relpath(full_path_to_program, start=root)
  File "C:\Users\Paul Boursin\anaconda3\envs\env\lib\ntpath.py", line 703, in relpath
    raise ValueError("path is on mount %r, start on mount %r" % (
ValueError: path is on mount '\\\\?\\C:', start on mount 'C:'
```

Looking at the code, the problem seems to be in wandb_settings.py:1669
I get :
full_path_to_program=`\\?\C:\Users\Paul Boursin\anaconda3\envs\env\Scripts\mlagents-learn-script.py`

I downgraded to 0.18.5 which is what I was previously using, and it works nicely.

I don't think I need to upgrade right now, so this bug report is not urgent for me.

I can provide more info if needed.

Cheers

Hints from issue discussion:
Hey Paul!

Thank you for writing in. Are you able to share the toy code example with us to so I can try and reproduce this on my side?

Looks like you are running this on windows? Could you please provide the debug.log and debug-internal.log files associated with the run where you are running into this issue? These files should be located in the wandb folder relative to your working directory. 

Thanks.

It crashes on this call (the first wandb call, so I don't think the rest of the code is necessary here) :
`wandb.setup(wandb.Settings(program=__name__, program_relpath=__name__))`
with
`__name__=mlagents.trainers.stats`
Might be a problem with how I'm passing the program name.

I made a custom wandb plugin for mlagents, if you're interested in the full code.

[debug.log](https://github.com/user-attachments/files/19589567/debug.log)
[debug-internal.log](https://github.com/user-attachments/files/19589566/debug-internal.log)
Hi @Paul-Hubert thanks for reporting this issue!
This is indeed an issue on our side, I'm looking into how we want to handle this case better.

The issue is occurring because it appears you are running the program from a different drive than where the program is located on. In your case it looks like the program file is located on a network drive possibly (due to the `\\` path prefix on windows).

The root cause on our side is related to a change in how we are applying settings across various locations (e.g. environment variables, user provided settings with `wandb.Settings(...)`, etc.). 
Which we would attempt to infer the running program's relative path to the current working directory the program was ran from first before using the user provided `program_relpath` setting.

The specific change was made in v0.19.0, and should be limited to only when running on windows from a different drive than where the program is located. In the event that you do need to upgrade sooner you can possibly mitigate this issue but running the program from the same drive/root as where the program is located, if possible.

Hi, thanks for the support, however everything is on C:\ I'm very sure (there is no network drive).
I can try printing more variables tomorrow to help you debug, so please give me a list of variables values you need.
I think something is wrong with how the os.path.join is done : 
```
full_path_to_program = os.path.join(
          root, os.path.relpath(os.getcwd(), root), program
      )
````
https://github.com/wandb/wandb/blob/5db4d763516ba2a3e8d94a85ba67bdf6fc124423/wandb/sdk/wandb_settings.py#L1665C9-L1667C10

Here are the values of these variables (note that I ran this on a different machine which has the same problem with the same version of wandb)

root=`C:/Users/PaulHubert/Desktop/Dynamical`
os.getcwd()=`C:\Users\PaulHubert\Desktop\Dynamical`
os.path.relpath(os.getcwd(), root)="."
program=`\\?\C:\Users\PaulHubert\.conda\envs\env\Scripts\mlagents-learn-script.py`

So the problem is coming from program.
I checked the previous function, and "self._get_program()" is returning the same string, while self.program is None.
https://github.com/wandb/wandb/blob/5db4d763516ba2a3e8d94a85ba67bdf6fc124423/wandb/sdk/wandb_settings.py#L1519

I can look into "self._get_program()" tomorrow.

So I checked anyway :

The problem is here : 
https://github.com/wandb/wandb/blob/5db4d763516ba2a3e8d94a85ba67bdf6fc124423/wandb/sdk/wandb_settings.py#L1638

`__main__.__file__=\\?\C:\Users\PaulHubert\.conda\envs\env\Scripts\mlagents-learn-script.py`

Hope this helps :)
OH, I realize this must be the problem. ML-Agents uses an executable ".exe" for training, which calls mlagents-learn-script.py, which has a `__main__`. I think maybe the fact that it is called from an executable is breaking the main object.
Hey Paul!

Wanted to follow up with you and check if this was the reason behind the error, to see if we need to dig deeper into this.
Well I can't know for sure, and I'm not sure how ml-agents constructed their executable, but I'm 100% sure the problem is coming from here :
https://github.com/wandb/wandb/blob/5db4d763516ba2a3e8d94a85ba67bdf6fc124423/wandb/sdk/wandb_settings.py#L1638

As this is the definite source of the weird `\\?\C:\` prefix, as shown in my previous comment.

I'll be available tomorrow to look deeper into it if you have any ideas for debugging this.
Thanks @Paul-Hubert for the follow up, we might possible have multiple issues here (besides the one I mentioned above).
Let me try reproducing on my side to see if I can get the same sort of path issue (`\\?\C:\...`).

One quick question, for your setup how does the call chain to `mlagents-learn-script.py` look like
are you doing something like `.\example.exe` which in turn is calling `mlagents-learn-script.py`?

Commit message: update changelog

Patch to review:
diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md
index 0d0e94bfbe2..bda08cddeba 100644
--- a/CHANGELOG.unreleased.md
+++ b/CHANGELOG.unreleased.md
@@ -16,3 +16,7 @@ Section headings should be at level 3 (e.g. `### Added`).
 ### Changed
 
 - Upgrade go version for `wandb-core` from 1.23.x to 1.24.x (@kptkin in https://github.com/wandb/wandb/pull/9590)
+
+### Fixed
+
+- Fixed ValueError on Windows when running a W&B script from a different drive (@jacobromero in https://github.com/wandb/wandb/pull/9678)
diff --git a/tests/unit_tests/test_util.py b/tests/unit_tests/test_util.py
index 183b0e577ab..dc3591aed52 100644
--- a/tests/unit_tests/test_util.py
+++ b/tests/unit_tests/test_util.py
@@ -809,3 +809,18 @@ def test_json_dump_uncompressed_with_numpy_datatypes():
     iostr = io.StringIO()
     util.json_dump_uncompressed(data, iostr)
     assert iostr.getvalue() == '{"a": [1, 2.0, 3]}'
+
+
+@pytest.mark.skipif(
+    platform.system() != "Windows",
+    reason="Drive letters are only relevant on Windows",
+)
+@pytest.mark.parametrize(
+    "path1,path2,expected",
+    [
+        ("C:\\foo", "C:\\bar", True),
+        ("C:\\foo", "D:\\bar", False),
+    ],
+)
+def test_are_windows_paths_on_same_drive(path1, path2, expected):
+    assert util.are_windows_paths_on_same_drive(path1, path2) == expected
diff --git a/tests/unit_tests/test_wandb_settings.py b/tests/unit_tests/test_wandb_settings.py
index d839de3e0ee..fedfa04ae00 100644
--- a/tests/unit_tests/test_wandb_settings.py
+++ b/tests/unit_tests/test_wandb_settings.py
@@ -2,6 +2,7 @@
 import json
 import os
 import pathlib
+import platform
 import subprocess
 import sys
 import tempfile
@@ -527,3 +528,23 @@ def test_rewind(setting):
 def test_computed_settings_included_in_model_dump():
     settings = Settings(mode="offline")
     assert settings.model_dump()["_offline"] is True
+
+
+@pytest.mark.skipif(
+    platform.system() != "Windows",
+    reason="Drive letters are only relevant on Windows",
+)
+@pytest.mark.parametrize(
+    "root_dir,expected_result",
+    [
+        ("C:\\other", lambda x: x is not None),
+        ("D:\\other", lambda x: x is None),
+    ],
+)
+def test_program_relpath_windows(root_dir, expected_result):
+    with tempfile.TemporaryDirectory() as temp_dir:
+        test_file_path = os.path.join(temp_dir, "test_file.py")
+        with open(test_file_path, "w") as f:
+            f.write("# Test file for program_relpath testing")
+        result = Settings._get_program_relpath(test_file_path, root_dir)
+        assert expected_result(result)
diff --git a/wandb/sdk/wandb_settings.py b/wandb/sdk/wandb_settings.py
index 8d885bf5c45..1a19a2698af 100644
--- a/wandb/sdk/wandb_settings.py
+++ b/wandb/sdk/wandb_settings.py
@@ -1662,6 +1662,12 @@ def _get_program_relpath(program: str, root: Optional[str] = None) -> Optional[s
         if not root:
             return None
 
+        # On Windows if the program and root are on different drives,
+        # Python will raise a ValueError.
+        if platform.system() == "Windows":
+            if not util.are_windows_paths_on_same_drive(program, root):
+                return None
+
         full_path_to_program = os.path.join(
             root, os.path.relpath(os.getcwd(), root), program
         )
diff --git a/wandb/util.py b/wandb/util.py
index 38e25651a90..7cc1c0fb24a 100644
--- a/wandb/util.py
+++ b/wandb/util.py
@@ -1438,6 +1438,12 @@ def auto_project_name(program: Optional[str]) -> str:
     return str(project.replace(os.sep, "_"))
 
 
+def are_windows_paths_on_same_drive(path1: str, path2: str) -> bool:
+    path1 = pathlib.Path(path1).resolve()
+    path2 = pathlib.Path(path2).resolve()
+    return path1.drive == path2.drive
+
+
 # TODO(hugh): Deprecate version here and use wandb/sdk/lib/paths.py
 def to_forward_slash_path(path: str) -> str:
     if platform.system() == "Windows":


Code review comments:
diff_hunk: @@ -1662,6 +1662,12 @@ def _get_program_relpath(program: str, root: Optional[str] = None) -> Optional[s
         if not root:
             return None
 
+        # On Windows if the program and root are on different drives,
+        # Python will raise a ValueError.
+        if platform.system() == "Windows":
+            if not util.are_windows_paths_on_same_drive(program, root):
path: wandb/sdk/wandb_settings.py
text: @user1:
probably can be part of the same if condition as there is no other branching logic

@user1:
what are your thoughts of adding the platform check as part of the helper function and making it platform agnostic, the call-site becomes a bit simpler, but with a minor "complication" of patching the system check for testing, which i don't think is a big issue

@user1:
i think add this comment after you already add new changes, see the one above

@author:
I was thinking that route as well, but ended up going with adding the check here because its more clear when reading the code that this check is limited only to windows, since if you read this in the context of a *nix based OS the concept of a drive path isnt really relevant.

Alternatively we can do the check on every system (i.e. remove `if platform.system() == "Windows"`). Since on mac/linux `Path("/some/path").resolve().drive == Path("/other").resolve().drive == ""`

@user1:
yep that's why i though have a short-circuit if it's not windows always return true

@author:
makes sense, made the change!