Skip to content

Document environment variables#4273

Merged
steventk-g merged 1 commit into
masterfrom
env-var-doc
Dec 8, 2022
Merged

Document environment variables#4273
steventk-g merged 1 commit into
masterfrom
env-var-doc

Conversation

@steventk-g

@steventk-g steventk-g commented Dec 5, 2022

Copy link
Copy Markdown
Collaborator

Created a yaml file that captures all relevant environment variables with description, type, and default value.

Variables are grouped into the following:

  • Build variables
  • Debug variables
  • Device variables
  • PJRT variables
  • XRT variables
  • Feature variables

Comment thread configuration.yaml Outdated
Comment thread configuration.yaml Outdated
Comment thread configuration.yaml Outdated
Comment thread configuration.yaml Outdated
Comment thread configuration.yaml Outdated
- Verbosity level for GRPC, e.g. INFO, ERROR, etc.
type: string
default_value: "ERROR"
TPU_ML_PLATFORM:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lol I felt like we should not make this configurable

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I wonder if we should include it here for the sake of documentation, and remark that it shouldn't be changed. My intention in including it was just to cover every environment variable that shows up in this repo.

_set_missing_env('TPU_ML_PLATFORM', 'PyTorch/XLA')

@JackCaoG JackCaoG left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Mostly LGTM. Thanks for putting this together! Is it possible to group similar env var for better reading expericence?

For example we can split them into

  1. XRT related
  2. Build related
  3. Debug related
  4. Feature enablement
  5. Device configuration
  6. Distributed related
  7. ...

@will-cromar will-cromar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wow! Thanks for laying all of this out. LGTM at least for the variables I know about

I agree with Jack's comment that it would be clearer if you categorized the variables, especially separating the build-time variables from the run-time variables and separating the XRT configuration from the rest.

@JackCaoG Do we also want some designation of which variables are stable and which might get removed in the future? Or are we committed to actually supporting all of these?

Comment thread configuration.yaml Outdated
@JackCaoG

JackCaoG commented Dec 7, 2022

Copy link
Copy Markdown
Collaborator

yea I think in this or I am fine with only group them by cateratory. We can update it based on stable/to_be_removed in a follow up pr.

@JackCaoG JackCaoG left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@steventk-g steventk-g marked this pull request as ready for review December 8, 2022 01:13
@steventk-g steventk-g merged commit 36c53d6 into master Dec 8, 2022
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.

3 participants