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

TensorFlow install: Add Python versioning info #170

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

peterjunpark
Copy link
Contributor

@peterjunpark peterjunpark commented May 31, 2024

This PR adds notes about checking which Python versions are supported by a given TensorFlow installation

@peterjunpark peterjunpark marked this pull request as ready for review May 31, 2024 17:14
@peterjunpark peterjunpark requested a review from a team as a code owner May 31, 2024 17:14
@peterjunpark
Copy link
Contributor Author

@yhuiYH Does this work? I'm struggling to convince myself that this is a good user experience

Copy link
Collaborator

@yhuiYH yhuiYH left a comment

Choose a reason for hiding this comment

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

Some suggestions provided. We may need to keep working on this page to see how it works out....

docs/how-to/3rd-party/tensorflow-install.rst Show resolved Hide resolved
docs/how-to/3rd-party/tensorflow-install.rst Outdated Show resolved Hide resolved

.. code-block:: shell
a. To check compatible Python versions, navigate to the desired ROCm release version
Copy link
Collaborator

Choose a reason for hiding this comment

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

These instructions only apply to ROCm 6.1+ because manylinux only has wheels packages for ROCm 6.1+. We should highlight that these are only for 6.1+

Copy link
Contributor

Choose a reason for hiding this comment

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

I think needing users to go through the repository to read the filenames isn't a great experience, could we have another table with the supported versions?

Copy link
Contributor

@lpaoletti lpaoletti left a comment

Choose a reason for hiding this comment

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

image

Copy link
Contributor

@lpaoletti lpaoletti left a comment

Choose a reason for hiding this comment

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

Current sentence:
Using Docker provides portability and access to a prebuilt Docker image that has been rigorously tested within AMD.

Consider:
Using Docker provides portability and access to a prebuilt Docker image rigorously tested by AMD.

Copy link
Contributor

@lpaoletti lpaoletti left a comment

Choose a reason for hiding this comment

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

Current sentence:
To quickly validate your TensorFlow environment, let’s run a basic TensorFlow example

New sentence:
Run a basic TensorFlow example to validate your TensorFlow environment quickly.

docs/how-to/3rd-party/tensorflow-install.rst Outdated Show resolved Hide resolved
docs/how-to/3rd-party/tensorflow-install.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@lpaoletti lpaoletti left a comment

Choose a reason for hiding this comment

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

Current:
If successful, you should see the following output indicating the image classifier is now trained to around 98% accuracy on this dataset.

Suggestion:
If successful, you should see the following output indicating the image classifier is now trained to about 98 percent accuracy on this dataset.

@peterjunpark peterjunpark marked this pull request as draft June 3, 2024 13:03
Spellcheck

Reorg
@peterjunpark peterjunpark marked this pull request as ready for review June 5, 2024 19:09
@peterjunpark
Copy link
Contributor Author

Might need more details on installing older tensorflow-rocm wheels with PyPI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants