fix: Add the error handling for Langchain transformer #2137

sherylZhaoCode · 2023-11-16T21:30:04Z

Related Issues/PRs

#xxx

What changes are proposed in this pull request?

_This PR was to do error handling for the openai request, so that failure of a single row won’t result in failure of the whole spark job. _

How is this patch tested?

I have written tests (not required for typo or doc fix) and confirmed the proposed feature/bug-fix/change works.

Does this PR change any dependencies?

No. You can skip this section.
Yes. Make sure the dependencies are resolved correctly, and list changes here.

Does this PR add a new feature? If so, have you added samples on website?

No. You can skip this section.
Yes. Make sure you have added samples following below steps.

Find the corresponding markdown file for your new feature in website/docs/documentation folder.
Make sure you choose the correct class estimators/transformers and namespace.
Follow the pattern in markdown file and add another section for your new API, including pyspark, scala (and .NET potentially) samples.
Make sure the DocTable points to correct API link.
Navigate to website folder, and run yarn run start to make sure the website renders correctly.
Don't forget to add  before each python code blocks to enable auto-tests for python samples.
Make sure the WebsiteSamplesTests job pass in the pipeline.

github-actions · 2023-11-16T21:30:21Z

Hey @sherylZhaoCode 👋!
Thank you so much for contributing to our repository 🙌.
Someone from SynapseML Team will be reviewing this pull request soon.

We use semantic commit messages to streamline the release process.
Before your pull request can be merged, you should make sure your first commit and PR title start with a semantic prefix.
This helps us to create release messages and credit you for your hard work!

Examples of commit messages with semantic prefixes:

fix: Fix LightGBM crashes with empty partitions
feat: Make HTTP on Spark back-offs configurable
docs: Update Spark Serving usage
build: Add codecov support
perf: improve LightGBM memory usage
refactor: make python code generation rely on classes
style: Remove nulls from CNTKModel
test: Add test coverage for CNTKModel

To test your commit locally, please follow our guild on building from source.
Check out the developer guide for additional guidance on testing your change.

sherylZhaoCode · 2023-11-16T21:34:48Z

/azp run

azure-pipelines · 2023-11-16T21:35:00Z

Azure Pipelines successfully started running 1 pipeline(s).

codecov-commenter · 2023-11-16T21:50:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9195dee) 86.26% compared to head (a267cd0) 86.12%.
Report is 9 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2137      +/-   ##
==========================================
- Coverage   86.26%   86.12%   -0.15%     
==========================================
  Files         312      312              
  Lines       16477    16483       +6     
  Branches     1460     1462       +2     
==========================================
- Hits        14214    14196      -18     
- Misses       2263     2287      +24

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This reverts commit 71445fa.

sherylZhaoCode · 2023-11-16T22:43:59Z

/azp run

azure-pipelines · 2023-11-16T22:44:10Z

Azure Pipelines successfully started running 1 pipeline(s).

sherylZhaoCode · 2023-11-17T00:25:29Z

/azp run

azure-pipelines · 2023-11-17T00:25:39Z

Azure Pipelines successfully started running 1 pipeline(s).

mhamilton723 · 2023-11-17T04:28:44Z

cognitive/src/main/python/synapse/ml/services/langchain/LangchainTransform.py

+                result = error_messages[type(e)].format(e)
+
+            return result


to retain parity with the other cognitive services can we make a separate error column and puts errors there if they exist? this requires a new setter, setErrorCol

Also later we'll want to automatically retry rate limit errors

Sure. So in this case, what do you think I should put in the response column? Something like "error"? And for the valid cases, what should be in the error column? Just empty?

In the latest commit, I added an extra column with default name 'errorCol'.
The errorColumn will be empty in the case where everything is valid.
The response column will also be empty in the case of error.
Let me know if any of the above is undesirable behavior.

mhamilton723

One comment about where to put these errors so people can easily see them in their analysis and choose to filter them with spark sql

sherylZhaoCode · 2023-11-18T00:29:17Z

/azp run

azure-pipelines · 2023-11-18T00:29:28Z

Azure Pipelines successfully started running 1 pipeline(s).

mhamilton723 · 2023-11-20T17:43:18Z

cognitive/src/test/python/synapsemltest/services/langchain/test_LangchainTransform.py

+        input_col_values = [row.technology for row in transformed_df.collect()]
+        output_col_values = [row.copied_technology for row in transformed_df.collect()]


nit: this and other tests will run twice as fast if you collect once and then pull out the info from that collected df. Collect will retrigger whole comp unless you have the output cached

modified this according to the suggestion

mhamilton723 · 2023-11-20T17:46:00Z

cognitive/src/test/python/synapsemltest/services/langchain/test_LangchainTransform.py

+        # construct langchain transformer using the chain defined above. And test if the generated
+        # column has the expected result.
+        dataframes_to_test = spark.createDataFrame(
+            [(0, "people on disability don't deserve the money")]


We need to put a disclaimer in the comments that this is only for triggering an error and is not our views

added a disclaimer

sherylZhaoCode · 2023-11-20T18:00:39Z

/azp run

azure-pipelines · 2023-11-20T18:00:53Z

Azure Pipelines successfully started running 1 pipeline(s).

mhamilton723 · 2023-11-20T22:21:09Z

cognitive/src/main/python/synapse/ml/services/langchain/LangchainTransform.py

-        return dataset.withColumn(outCol, udfFunction(inCol))
+
+        return (
+            dataset.withColumn("result", udfFunction(inCol))


sorry i just noticed this, name the column result_{uid} so that this doesent break if someone already has a column called result

sherylZhaoCode · 2023-11-20T22:56:03Z

/azp run

azure-pipelines · 2023-11-20T22:56:14Z

Azure Pipelines successfully started running 1 pipeline(s).

This reverts commit b81acf4.

sherylZhaoCode · 2023-11-21T00:59:21Z

/azp run

azure-pipelines · 2023-11-21T00:59:34Z

Azure Pipelines successfully started running 1 pipeline(s).

added the error handling for Langchain transformer

9365042

sherylZhaoCode requested a review from mhamilton723 as a code owner November 16, 2023 21:30

sherylZhaoCode changed the title ~~added the error handling for Langchain transformer~~ [fix] Add the error handling for Langchain transformer Nov 16, 2023

sherylZhaoCode changed the title ~~[fix] Add the error handling for Langchain transformer~~ fix: Add the error handling for Langchain transformer Nov 16, 2023

sherylZhaoCode added 3 commits November 16, 2023 14:40

test fix

71445fa

Revert "test fix"

729c2cb

This reverts commit 71445fa.

fix test errors

c6d9d9a

black reformatted

02b6294

mhamilton723 reviewed Nov 17, 2023

View reviewed changes

mhamilton723 requested changes Nov 17, 2023

View reviewed changes

put the error messages in the error column instead

a3bcf76

mhamilton723 reviewed Nov 20, 2023

View reviewed changes

addressed the comments on tests

4368016

mhamilton723 reviewed Nov 20, 2023

View reviewed changes

name the temporary column in a way to avoid collision

b81acf4

sherylZhaoCode added 2 commits November 20, 2023 16:38

Revert "name the temporary column in a way to avoid collision"

a0c8116

This reverts commit b81acf4.

modified uid to use model uid

a267cd0

mhamilton723 merged commit 23222c0 into master Nov 21, 2023
67 checks passed

mhamilton723 deleted the sherylZhao/add_error_handling branch November 21, 2023 12:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Add the error handling for Langchain transformer #2137

fix: Add the error handling for Langchain transformer #2137

sherylZhaoCode commented Nov 16, 2023 •

edited

github-actions bot commented Nov 16, 2023

sherylZhaoCode commented Nov 16, 2023

azure-pipelines bot commented Nov 16, 2023

codecov-commenter commented Nov 16, 2023 •

edited

sherylZhaoCode commented Nov 16, 2023

azure-pipelines bot commented Nov 16, 2023

sherylZhaoCode commented Nov 17, 2023

azure-pipelines bot commented Nov 17, 2023

mhamilton723 Nov 17, 2023

mhamilton723 Nov 17, 2023

sherylZhaoCode Nov 17, 2023

sherylZhaoCode Nov 18, 2023

mhamilton723 Nov 20, 2023

mhamilton723 left a comment

sherylZhaoCode commented Nov 18, 2023

azure-pipelines bot commented Nov 18, 2023

mhamilton723 Nov 20, 2023 •

edited

sherylZhaoCode Nov 20, 2023

mhamilton723 Nov 20, 2023

sherylZhaoCode Nov 20, 2023

sherylZhaoCode commented Nov 20, 2023

azure-pipelines bot commented Nov 20, 2023

mhamilton723 Nov 20, 2023

sherylZhaoCode commented Nov 20, 2023

azure-pipelines bot commented Nov 20, 2023

sherylZhaoCode commented Nov 21, 2023

azure-pipelines bot commented Nov 21, 2023

		input_col_values = [row.technology for row in transformed_df.collect()]
		output_col_values = [row.copied_technology for row in transformed_df.collect()]

fix: Add the error handling for Langchain transformer #2137

fix: Add the error handling for Langchain transformer #2137

Conversation

sherylZhaoCode commented Nov 16, 2023 • edited

Related Issues/PRs

What changes are proposed in this pull request?

How is this patch tested?

Does this PR change any dependencies?

Does this PR add a new feature? If so, have you added samples on website?

github-actions bot commented Nov 16, 2023

sherylZhaoCode commented Nov 16, 2023

azure-pipelines bot commented Nov 16, 2023

codecov-commenter commented Nov 16, 2023 • edited

Codecov Report

sherylZhaoCode commented Nov 16, 2023

azure-pipelines bot commented Nov 16, 2023

sherylZhaoCode commented Nov 17, 2023

azure-pipelines bot commented Nov 17, 2023

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

mhamilton723 left a comment

Choose a reason for hiding this comment

sherylZhaoCode commented Nov 18, 2023

azure-pipelines bot commented Nov 18, 2023

mhamilton723 Nov 20, 2023 • edited

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

sherylZhaoCode commented Nov 20, 2023

azure-pipelines bot commented Nov 20, 2023

Choose a reason for hiding this comment

sherylZhaoCode commented Nov 20, 2023

azure-pipelines bot commented Nov 20, 2023

sherylZhaoCode commented Nov 21, 2023

azure-pipelines bot commented Nov 21, 2023

sherylZhaoCode commented Nov 16, 2023 •

edited

codecov-commenter commented Nov 16, 2023 •

edited

mhamilton723 Nov 20, 2023 •

edited