Bump tf version #281

mohazahran · 2024-09-20T20:21:33Z

Migrating ml4ir to tf 2.16.1 and jvm to 1.0.0-rc1

- lookup table - csv read param - printing lr rate - collecting metrics in scoring model

into mo/Bump_tf_version

lastmansleeping · 2024-10-30T06:02:24Z

.circleci/images/Dockerfile

@@ -1,2 +1,2 @@
 FROM cimg/openjdk:11.0
-COPY --from=cimg/python:3.9.7 / /
+COPY --from=cimg/python:3.11 / /


@mohazahran would be good to add context for why you upgraded to python311

lastmansleeping · 2024-10-30T06:03:14Z

jvm/ml4ir-inference/pom.xml

@@ -217,7 +217,13 @@
  <dependencies>
    <dependency>
      <groupId>org.tensorflow</groupId>
-      <artifactId>tensorflow-core-platform</artifactId>
+      <artifactId>tensorflow-core-api</artifactId>
+      <version>1.0.0-rc.1</version>


@mohazahran did you check if we could upgrade to 1.0.0 without upgrading on the python side? Also "rc" is usually a beta release I believe.

lastmansleeping · 2024-10-30T06:04:28Z

jvm/ml4ir-inference/src/main/scala/ml4ir/inference/tensorflow/TFRecordExecutor.scala

+  println("\nsavedModelBundle:\n")
+  savedModelBundle.graph().operations().asScala.foreach(op => println(op.name()))
+
+  //val metaGraphDef = MetaGraphDef.parseFrom(savedModelBundle.metaGraphDef())
+  //val signatureMap = metaGraphDef.getSignatureDefMap
+  //val signatureKey = "serving_tfrecord"
+  //val signatureDef = signatureMap.get(signatureKey)
+
+  //val inputInfo = signatureDef.getInputsMap.asScala
+  //val outputInfo = signatureDef.getOutputsMap.asScala
+
+
+
  private val session = savedModelBundle.session()
+
+  // Initialize variables if an init op exists
+    try {
+      session.runner().addTarget("init_op").run() // Replace "init_op" with actual name
+    } catch {
+      case e: Exception => println("No init_op found or initialization failed.")
+    }
+
+  // Run the initialization operation


@mohazahran Do you want to clean this up or let it be for context?

I want to leave it so that we can track debugging attempts. I will add comments.

lastmansleeping · 2024-10-30T06:07:38Z

python/ml4ir/applications/classification/model/classification_model.py

+            # np.set_printoptions(formatter={'all':lambda x: str(x.decode('utf-8')) if isinstance(x, bytes) else str(x)},
+            #                     linewidth=sys.maxsize,
+            #                     threshold=sys.maxsize,  # write the full vector in the csv not a truncated version
+            #                     legacy="1.13")  # enables 1.13 legacy printing mode
+
+
+            # np.set_printoptions(formatter={
+            #     'all': lambda x: str(x.decode('utf-8', errors='ignore')) if isinstance(x, bytes) else str(x)
+            # }, linewidth=sys.maxsize, threshold=sys.maxsize, legacy="1.13")
+
+            # for col in predictions_df.columns:
+            #     if isinstance(predictions_df[col].values[0], bytes):
+            #         predictions_df[col] = predictions_df[col].str.decode('utf8')


@mohazahran Can you remove unused code?

I left it commented as it seems it really affect the java integration test. I will add comments to reflect that.

lastmansleeping

@mohazahran I have closed the work, but I'd request these 2 changes

split it into two PRs against a dev_tf branch - one for python (merge it) and one for jvm (leave it as is)
add in-line comments (either on this PR or the split up ones) explaining your changes/findings to add context for future

lastmansleeping · 2024-10-30T06:09:51Z

python/ml4ir/applications/ranking/tests/test_ranking_model.py

@@ -76,8 +76,8 @@ def test_csv_and_tfrecord(self):
        )

        # Check if the loss and accuracy on the test set is the same
-        assert np.isclose(csv_loss, 0.56748, rtol=0.01)
-        assert np.isclose(csv_mrr, 0.70396, rtol=0.01)
+        assert np.isclose(csv_loss, 0.31154385209083557, rtol=0.01)


@mohazahran do you know why these might have changed?

I expect these changes specially with this type of migration due to changes to how models gets trained under the hood. As long as numbers seems sane, loss if decreasing I believe it should be fine.

lastmansleeping · 2024-10-30T06:10:04Z

python/ml4ir/applications/ranking/tests/test_serving.py

+        # default_model = kmodels.load_model(
+        #     os.path.join(self.output_dir, "final", "default"), compile=False
+        # )
+        # assert ServingSignatureKey.DEFAULT in default_model.signatures
+        # default_signature = default_model.signatures[ServingSignatureKey.DEFAULT]


@mohazahran cleanup unused code

lastmansleeping · 2024-10-30T06:10:26Z

python/ml4ir/base/data/tfrecord_reader.py

+    # def get_default_tensor(self, feature_info, sequence_size):
+    #     """
+    #     Get the default tensor for a given feature configuration
+    #     Parameters
+    #     ----------
+    #     feature_info: dict
+    #         Feature configuration information for the feature as specified in the feature_config
+    #     sequence_size: int, optional
+    #         Number of elements in the sequence of a SequenceExample
+    #     Returns
+    #     -------
+    #     tf.Tensor
+    #         Tensor object that can be used as a default tensor if the expected feature
+    #         is missing from the TFRecord
+    #     """
+    #     if feature_info.get("tfrecord_type", SequenceExampleTypeKey.CONTEXT) == SequenceExampleTypeKey.CONTEXT:
+    #         return tf.constant(
+    #             value=self.feature_config.get_default_value(feature_info), dtype=feature_info["dtype"],
+    #         )
+    #     else:
+    #         return tf.fill(
+    #             value=tf.constant(
+    #                 value=self.feature_config.get_default_value(feature_info),
+    #                 dtype=feature_info["dtype"],
+    #             ),
+    #             dims=[sequence_size],
+    #         )
+


@mohazahran delete

Update config.yml

185ae30

salesforce-cla bot added the cla:signed label Sep 20, 2024

mohazahran added 28 commits September 20, 2024 13:25

Update config.yml

a67097a

Update requirements.txt

98abbd0

Update temperature_scaling.py

7a94f87

Update requirements.txt

c29b463

Update requirements.txt

d9d8e35

code changes for tf 2.16

07f4bab

Fixing:

84c436f

- lookup table - csv read param - printing lr rate - collecting metrics in scoring model

Fixing for tests

0c2d4e3

jvm update

93b2d72

Update Dockerfile to py 3.11

484465a

using mohazahran/ml4ir:0.3 docker image for jvm integration

c036ba9

py 3.11

0423253

Merge branch 'mo/Bump_tf_version' of https://github.com/salesforce/ml4ir

860e063

into mo/Bump_tf_version

Update config.yml

9c97d1d

Update config.yml

6ade9c0

Update config.yml

8b7f400

Update config.yml

318952e

Update config.yml

139b9ca

Update config.yml

7f534e8

Update config.yml

b0e1049

add rc1 as depend.

6fa4906

Merge branch 'mo/Bump_tf_version' of https://github.com/salesforce/ml4ir

6406b3d

into mo/Bump_tf_version

Fixing scala files

442ea01

adding tensorflow.core.platform

18cb5d5

Fixing default serving exception

315f29e

fixes

3b50232

Fixing classification stack

d1b25cc

scala parsing updates

025c875

mohazahran added 3 commits October 26, 2024 12:28

Fixing ranking tests

33a4bf5

cleaning up

5f700c1

fix path

b1842da

lastmansleeping reviewed Oct 30, 2024

View reviewed changes

lastmansleeping requested changes Oct 30, 2024

View reviewed changes

lastmansleeping reviewed Oct 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump tf version #281

Bump tf version #281

Bump tf version #281

Are you sure you want to change the base?

Bump tf version #281

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment