Skip to content

Conversation

@mraunak
Copy link
Contributor

@mraunak mraunak commented Apr 20, 2023

From the recent GitHub issues, we observed that TensorFlow Windows users are getting errors while building the TensorFlow package from the source due to incorrect path setup of the Environmental Variables. This pull request will update the https://www.tensorflow.org/install/source_windows with the steps and commands to set up the paths correctly and guide users to resolve similar issues. The document has also been updated with the latest output of running Python Configure file and GPU support on Native-Windows

This pull request also addresses one minor change in https://www.tensorflow.org/install/pip#windows-native.

Copy link
Collaborator

@haifeng-jin haifeng-jin left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I believe the changes in the pip.md are totally accidents. Please change it back and only make the changes needed.

@haifeng-jin
Copy link
Collaborator

@MarkDaoust Should we just remove the native windows tab on the top of pip page? since we do not support the latest TF on it any more.

Copy link
Member

@MarkDaoust MarkDaoust left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to fix this.

source_windows.md looks good, but something went wring with install/pip.md

It looks like you edited an older version of the file, and resolved the merge conflict by choosing your copy. This PR reverts all the changes that came in after you branched (CUDA 11.8 -> 11.2, TF2.12 -> 2.11).

I think you need to look at your diff relative to where you branched off, and apply those changes to the version from master.

@MarkDaoust
Copy link
Member

@MarkDaoust Should we just remove the native windows tab on the top of pip page? since we do not support the latest TF on it any more.

Not at all anymore? I was under the impression that there was still tensorflow_cpu, I could be totally wrong.

@mraunak
Copy link
Contributor Author

mraunak commented Apr 24, 2023

@MarkDaoust Should we just remove the native windows tab on the top of pip page? since we do not support the latest TF on it any more.

Not at all anymore? I was under the impression that there was still tensorflow_cpu, I could be totally wrong.

Hi Mark, yeah tensorflow_cpu is still available.

@mraunak
Copy link
Contributor Author

mraunak commented Apr 24, 2023

Hi @MarkDaoust and @haifeng-jin, I have made changes as per the suggestions.

@github-actions github-actions bot added the lgtm Community-added approval label Apr 24, 2023
@MarkDaoust
Copy link
Member

Thanks. Just a couple of minor things still to fix and we'll get this merged.

@mraunak
Copy link
Contributor Author

mraunak commented Apr 25, 2023

Hi @MarkDaoust, I have made all the changes, please let me know if it looks good. Thank you very much for your feedback.

MarkDaoust
MarkDaoust previously approved these changes Apr 25, 2023
Copy link
Member

@MarkDaoust MarkDaoust left a comment

Choose a reason for hiding this comment

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

I'm good with this.

I thought you said tensorflow on windows was still working, but in 7dd83d7 you remove it? I'm a bit confused, but I'll move to merge and you can always send another PR if you'd prefer it with tensorflow.

Thanks!

@mraunak
Copy link
Contributor Author

mraunak commented Apr 25, 2023

Hi @MarkDaoust , if it is still not merged, please consider including this revert. I just put back tensorflow, it was removed by mistake on my end.

Copy link
Member

@MarkDaoust MarkDaoust left a comment

Choose a reason for hiding this comment

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

Let's go.

@MarkDaoust MarkDaoust added the ready to pull Start merge process label Apr 25, 2023
@copybara-service copybara-service bot merged commit abfbe6e into tensorflow:master Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Community-added approval ready to pull Start merge process

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants