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

[jax2tf] Added a flag and environment variable to control the serialization version #16746

Closed
wants to merge 1 commit into from

Conversation

gnecula
Copy link
Collaborator

@gnecula gnecula commented Jul 15, 2023

This allows us to control the serialization version to be compatible with
the deployed version of tf.XlaCallModule. In particular, we can run
most tests with the maximum available version, while keeping the
default lower.

@gnecula gnecula self-assigned this Jul 15, 2023
@gnecula gnecula added the pull ready Ready for copybara import and testing label Jul 15, 2023
@gnecula
Copy link
Collaborator Author

gnecula commented Jul 15, 2023

@junwhanahn @rxsang PTAL

…zation version.

This allows us to control the serialization version to be compatible with
the deployed version of tf.XlaCallModule. In particular, we can run
most tests with the maximum available version, while keeping the
default lower.
Copy link
Contributor

@junwhanahn junwhanahn left a comment

Choose a reason for hiding this comment

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

The changes look good to me.

Somewhat related question: do you plan to maintain min/max supported version in jax2tf (if we ever need one when we clean up some old jax2tf logic) separately from TF2XLA, or in sync? I would guess that the former makes more sense, but not sure if it will make the testing more complex.

@gnecula
Copy link
Collaborator Author

gnecula commented Jul 16, 2023

The changes look good to me.

Somewhat related question: do you plan to maintain min/max supported version in jax2tf (if we ever need one when we clean up some old jax2tf logic) separately from TF2XLA, or in sync? I would guess that the former makes more sense, but not sure if it will make the testing more complex.

I don't quite see how to keep the versioning perfectly in sync. My plan is to support in jax2tf the versions that have been active in google3 over the last month. This means 1 or 2 versions, the most recent. The tests in google3 will verify that we can use the most recent version and the one that is marked as default. There is no test per se that we can use the version that has been active a month ago; we rely on people not updating the default serialization version too early.

Also in OSS people will have different versions of JAX and TF. I do assume that people will be willing to use tf-nightly if they get an error; this has been true for jax2tf so far because it relies always on some very recent details for the TF ops.

@junwhanahn
Copy link
Contributor

The changes look good to me.

Somewhat related question: do you plan to maintain min/max supported version in jax2tf (if we ever need one when we clean up some old jax2tf logic) separately from TF2XLA, or in sync? I would guess that the former makes more sense, but not sure if it will make the testing more complex.

I don't quite see how to keep the versioning perfectly in sync. My plan is to support in jax2tf the versions that have been active in google3 over the last month. This means 1 or 2 versions, the most recent. The tests in google3 will verify that we can use the most recent version and the one that is marked as default. There is no test per se that we can use the version that has been active a month ago; we rely on people not updating the default serialization version too early.

Also in OSS people will have different versions of JAX and TF. I do assume that people will be willing to use tf-nightly if they get an error; this has been true for jax2tf so far because it relies always on some very recent details for the TF ops.

Thanks. Sounds good to me.

copybara-service bot pushed a commit that referenced this pull request Jul 16, 2023
--
06bf5fe by George Necula <gcnecula@gmail.com>:

[jax2tf] Added a flag and environment variable to control the serialization version.

This allows us to control the serialization version to be compatible with
the deployed version of tf.XlaCallModule. In particular, we can run
most tests with the maximum available version, while keeping the
default lower.

COPYBARA_INTEGRATE_REVIEW=#16746 from gnecula:tf_version 06bf5fe
PiperOrigin-RevId: 548504243
@gnecula
Copy link
Collaborator Author

gnecula commented Jul 19, 2023

This was already merged, but copybara did not close it.
See 603eeb1

@gnecula gnecula closed this Jul 19, 2023
@gnecula gnecula deleted the tf_version branch July 22, 2023 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull ready Ready for copybara import and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants