remove \ in cache_dir

[taomiao]

Fixes #110933

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @aakhundov @ColinPeppler

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/110945

• 📄 Preview Python docs built from this PR
• 📄 Preview C++ docs built from this PR
• ❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 35719b6 with merge base 2bc1378 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[taomiao]

@pytorchbot label "topic: not user facing"

[masnesral]

@taomiao can you explain where the backslash is coming from? Is it for win, for example? If so, should we be using os.path.join to form the path?

[taomiao]

@masnesral getpass.getuser() return username like "ORG\user1", This kind of username is more common among institutional users. This happens in both windows and linux.

[masnesral]

@masnesral getpass.getuser() return username like "ORG\user1", This kind of username is more common among institutional users. This happens in both windows and linux.

Oh, ok. Thanks for explaining. In that case, probably we should apply the \ replacement only to the user string portion. And since you're changing this code, would you mind switching it to use os.path.join rather that hard-coding the front slash?

[masnesral]

nit: The formatting looks weird; perhaps you can fix before landing (I would have expected the linter to fix). But the logic looks good otherwise. Thanks for the changes.

[taomiao]

nit: The formatting looks weird; perhaps you can fix before landing (I would have expected the linter to fix). But the logic looks good otherwise. Thanks for the changes.

ok

[taomiao]

@shunting314 Do you have a minute for this?

[shunting314]

LG!

[pytorchmergebot]

Merge failed

Reason: Comment with id 1765974981 not found

Details for Dev Infra team

Raised by workflow job

[taomiao]

@cpuhrsch ok to be merged.

[kit1980]

@taomiao FYI when the PR is approved by a team member, you can merge yourself by posting "@pytorchbot merge" comment.

[kit1980]

I'm not sure I agree with this PR.
Do we really care about "ORG\user1" usernames? Then what about other symbols that can be in usernames but not in paths?
Also the current PR will map "ORG\user1" and "ORGuser1" to the same dir.

So I'm not sure if we really care about this use case.
And if we do care, this needs to be done in a more generic and robust way.

[masnesral]

And if we do care, this needs to be done in a more generic and robust way.

I'm not sure if there's a canonical way to sanitize the string to make it appropriate for paths. Maybe something like this is a bit more generic:

    illegal_chars = r'[\/:*?"<>|]'
    sanitized_name = re.sub(illegal_chars, '_', name)

[taomiao]

I'm not sure I agree with this PR. Do we really care about "ORG\user1" usernames? Then what about other symbols that can be in usernames but not in paths? Also the current PR will map "ORG\user1" and "ORGuser1" to the same dir.

So I'm not sure if we really care about this use case. And if we do care, this needs to be done in a more generic and robust way.

Some usernames do cause inductor's cache directory path "can not be found". This kind of error will make uninformed users feel baffled. Sure if there's a canonical way to sanitize the string to make it appropriate for paths would be better.

[taomiao]

And if we do care, this needs to be done in a more generic and robust way.

I'm not sure if there's a canonical way to sanitize the string to make it appropriate for paths. Maybe something like this is a bit more generic:

    illegal_chars = r'[\/:*?"<>|]'
    sanitized_name = re.sub(illegal_chars, '_', name)

Ok

[pytorch-bot]

You don't have permissions to rebase this PR since you are a first time contributor. If you think this is a mistake, please contact PyTorch Dev Infra.

[pytorchmergebot]

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

[pytorchmergebot]

Merge failed

Reason: 2 mandatory check(s) failed. The first few are:

• pull
• Lint

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

[taomiao]

@kit1980 changed to more generic.

[taomiao]

@pytorchbot merge

[pytorchmergebot]

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here