Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wip: debugging windows CI #207

Merged
merged 2 commits into from
Dec 18, 2024
Merged

wip: debugging windows CI #207

merged 2 commits into from
Dec 18, 2024

Conversation

TomMelt
Copy link
Member

@TomMelt TomMelt commented Dec 16, 2024

Previously I hard-coded the path to libtorch i.e.,
-DCMAKE_PREFIX_PATH="C:\hostedtoolcache\windows\Python\3.12.7\x64\Lib\site-packages".

This commit replaces the hard-code path using pip show to get the updated location and store it in a variable %torch_path% for later use i.e., -DCMAKE_PREFIX_PATH=%torch_path%

@TomMelt TomMelt self-assigned this Dec 16, 2024
@TomMelt TomMelt changed the title wip wip: debugging windows CI Dec 16, 2024
Previously I hard-coded the path to libtorch. This commit replaces the
hard-code path using `pip show` to get the updated location.
@TomMelt TomMelt added the bug Something isn't working label Dec 16, 2024
Copy link
Member

@jatkinson1000 jatkinson1000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @TomMelt
The exact semantics are still a mystery, but I can satisfy myself in what this is doing and how it resolves the issue and the CI is passing. Obviously not tested locally, but LGTM.

@TomMelt
Copy link
Member Author

TomMelt commented Dec 16, 2024

This fixes an issue detected in #206

Copy link
Contributor

@jwallwork23 jwallwork23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto. It's good to see that these changes reduce the number of hard-coded paths and avoid pinning to specific Python versions.

@jwallwork23
Copy link
Contributor

I merged this branch into the one for #206 and can confirm it gets it working!

@TomMelt TomMelt merged commit 13e12e2 into main Dec 18, 2024
1 check passed
@TomMelt TomMelt deleted the test-simple-change branch December 18, 2024 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants