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.

Sign up for GitHub

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

Make the model names unique and fix the tox package deps for tensorflow tests #33216

Merged
merged 8 commits into from
Nov 27, 2024
Merged

Make the model names unique and fix the tox package deps for tensorflow tests #33216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
78a9c70
make the model names unique
liferoad Nov 25, 2024
8550497
fixed lint
liferoad Nov 25, 2024
10cde1d
pin keras==3.6.0
liferoad Nov 25, 2024
36e8bec
pin keras==2.12.0
liferoad Nov 25, 2024
3db7dfe
pin tf_keras==2.18.0
liferoad Nov 26, 2024
c699cd9
pin tensorflow==2.18.0
liferoad Nov 26, 2024
a0fdb6b
use 1 worker for pytest
liferoad Nov 26, 2024
1875ee1
Fixed the tensorflow version
liferoad Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/trigger_files/beam_PostCommit_Python_Dependency.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"comment": "Modify this file in a trivial way to cause this test suite to run",
"modification": 1
}
9 changes: 5 additions & 4 deletions sdks/python/apache_beam/ml/inference/tensorflow_inference_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import shutil
import tempfile
import unittest
import uuid
from typing import Any
from typing import Dict
from typing import Iterable
Expand Down Expand Up @@ -127,7 +128,7 @@ def test_predict_tensor(self):

def test_predict_tensor_with_batch_size(self):
model = _create_mult2_model()
model_path = os.path.join(self.tmpdir, 'mult2.keras')
model_path = os.path.join(self.tmpdir, f'mult2_{uuid.uuid4()}.keras')
tf.keras.models.save_model(model, model_path)
with TestPipeline() as pipeline:

Expand Down Expand Up @@ -173,7 +174,7 @@ def fake_batching_inference_fn(

def test_predict_tensor_with_large_model(self):
model = _create_mult2_model()
model_path = os.path.join(self.tmpdir, 'mult2.keras')
model_path = os.path.join(self.tmpdir, f'mult2_{uuid.uuid4()}.keras')
tf.keras.models.save_model(model, model_path)
with TestPipeline() as pipeline:

Expand Down Expand Up @@ -220,7 +221,7 @@ def fake_batching_inference_fn(

def test_predict_numpy_with_batch_size(self):
model = _create_mult2_model()
model_path = os.path.join(self.tmpdir, 'mult2_numpy.keras')
model_path = os.path.join(self.tmpdir, f'mult2_{uuid.uuid4()}.keras')
tf.keras.models.save_model(model, model_path)
with TestPipeline() as pipeline:

Expand Down Expand Up @@ -263,7 +264,7 @@ def fake_batching_inference_fn(

def test_predict_numpy_with_large_model(self):
model = _create_mult2_model()
model_path = os.path.join(self.tmpdir, 'mult2_numpy.keras')
model_path = os.path.join(self.tmpdir, f'mult2_{uuid.uuid4()}.keras')
tf.keras.models.save_model(model, model_path)
with TestPipeline() as pipeline:

Expand Down
5 changes: 3 additions & 2 deletions sdks/python/tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,8 +410,9 @@ deps =
tensorflow>=2.12rc1,<2.13
# Help pip resolve conflict with typing-extensions for old version of TF https://github.com/apache/beam/issues/30852
pydantic<2.7
protobuf==4.25.5
extras = test,gcp,ml_test
extras = test,gcp
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tox honors the package deps with this order: deps -> extras -> install_requires. So if tensorflow is set several times, only the last one takes the effect. @jrmccluskey FYI.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we remove ml-test here since these tests only rely on tensorflow.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, I see

commands_pre =
pip install -U 'protobuf==4.25.5'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

install_requires will install protobuf with 5.x, which is not supported by tesnsorflow 2.12.x. So we reinstall protobuf with the fix version here.

commands =
# Log tensorflow version for debugging
/bin/sh -c "pip freeze | grep -E tensorflow"
Expand Down
Loading