feat: Decorators - part 2 #607

IshSiva · 2023-03-15T06:32:43Z

This PR has the code to perform type checking and shape checking in the decorators. It also has the documentation for using decorators in the EVA UDFs.

…decorators

moved the exception to the errors file.

made some fixes to code

…decorators

linting changes.

…decorators_2

gaurav274 · 2023-03-30T22:53:53Z

docs/source/reference/udf_decorators.rst

+
+2. Create a Python class that inherits from `PytorchClassifierAbstractUDF`.
+
+* The `PytorchClassifierAbstractUDF` is a parent class that defines and implements standard methods for model inference.


Why are we introducing PytorchClassifierAbstractUDF upfront? How about we break it into using AbstractUDF etc.? And it is not a classification job so we should also change it in the system.
We had a bad design earlier, let us carefully fix most of it now.

Do we want to do this in this PR or a different one?

eva/udfs/decorators/decorators.py

docs/source/reference/udf_decorators.rst

gaurav274 · 2023-03-30T22:57:33Z

Create a new PR with only the documentation. Thanks!

gaurav274 · 2023-03-30T22:59:12Z

We need more descriptive documentation.

Talk about supported data types
Talk about creating Pytorch Based UDF
Talk about generic UDFs like easyocr

linting changes.

xzdandy · 2023-03-31T05:34:31Z

eva/udfs/decorators/decorators.py

How many overhead will this bring? Are we doing the input and output check on every forward call?

Yes we will be checking it on every forward call.
I don't think there will be a lot of overhead as we are using the library functions and just doing basic shape and type checking.

If we can have some numbers, that will be great. Do you know why we are not considering a more static type checking?

Instead of checking on every forward call, we could optimistically assume that once the first forward works, the remaining forward calls will be on similar data shape etc. Just a boolean variable for the first check (self.check) might be sufficient for this.

We can have it as input from the user (check_inputs). And default to always doing it.

added this parameter

test/udfs/decorators/test_decorators.py

suryatejreddy

Added some comments, mostly looks good to me. Thanks @IshSiva !

suryatejreddy · 2023-04-13T04:04:35Z

eva/udfs/decorators/decorators.py

+                                # the first object in the forward function is self so we start from the second object
+                                input_signature.validate_object(args[i + 1], True)
+                            )
+                        except UDFIODefinitionError as e:


I think it's better to use the assert style syntax that we have adopted in other parts of the code recently. @gaurav274 what do you think?

since converting the shape or data type might throw an exception I am using the try-except

eva/udfs/decorators/decorators.py

eva/udfs/decorators/io_descriptors/abstract_types.py

suryatejreddy · 2023-04-13T04:13:45Z

eva/udfs/decorators/io_descriptors/abstract_types.py

+            try:
+                object = self.check_array_and_convert_type(object)
+            except Exception as e:
+                if input_flag:
+                    msg = "Data type mismatch of Input parameter. "
+                else:
+                    msg = "Data type mismatch of Output parameter."
+                raise UDFIODefinitionError(msg)


Moving to assert syntax will mostly cleanup code by avoiding try and except everywhere.

The assert statement can have the appropriate message.

same response as before, the shape conversions might throw an error if its not possible, hence used try-except

suryatejreddy · 2023-04-13T04:14:13Z

eva/udfs/decorators/io_descriptors/abstract_types.py

+                object = self.check_array_and_convert_shape(object)
+            except Exception as e:
+                if input_flag:
+                    msg = "Shape mismatch of Input parameter. "
+                else:
+                    msg = "Shape mismatch of Output parameter."
+                raise UDFIODefinitionError(msg)
+
+        return object


Same as above.

suryatejreddy · 2023-04-13T04:16:02Z

eva/udfs/decorators/io_descriptors/data_types.py

@@ -41,6 +44,46 @@ def __init__(
            array_dimensions=dimensions,
        )

+    def check_array_and_convert_shape(self, input_object):


Do we want to be converting data on our own? Or should that be an additional flag? For example user specifies that all data should be converted to this the given IOSignature format. We assert if it isn't possible

I feel that it is redundant to add another flag given that we have check_input and check_output flags.

suryatejreddy · 2023-04-13T04:16:20Z

eva/udfs/decorators/io_descriptors/data_types.py

+    def check_array_and_convert_shape(self, input_object):
+        try:
+            return torch.reshape(input_object, self.array_dimensions)
+
+        except Exception as e:
+            raise UDFIODefinitionError(
+                "The input object cannot be reshaped to %s. Error is %s"
+                % (self.array_dimensions, str(e))
+            )
+
+    def check_array_and_convert_type(self, input_object):
+
+        if not isinstance(input_object, torch.Tensor):
+            if isinstance(input_object, list):
+                input_object = torch.Tensor(input_object)
+            elif isinstance(input_object, np.ndarray):
+                input_object = torch.from_numpy(input_object)
+            else:
+                raise UDFIODefinitionError(
+                    "Unknown data type. Only input types List and Tensor can be converted to Numpy array"
+                )


Same as above.

…decorators_2

gaurav274 · 2023-05-04T03:17:58Z

@suryatejreddy Please help review this.

jarulraj · 2023-09-16T00:51:04Z

@IshSiva mentioned that we could close this PR and revert the Decorators in the documentation.

@gaurav274 @xzdandy Does that sound good?

xzdandy · 2023-09-16T05:29:57Z

@IshSiva mentioned that we could close this PR and revert the Decorators in the documentation.

@gaurav274 @xzdandy Does that sound good?

Sounds good, We will redesign in #1017 .

IshSiva added 30 commits February 9, 2023 12:15

created the udf service class.

93a481f

Merge branch 'master' of https://github.com/georgia-tech-db/eva into …

8a37c2e

…decorators

created the decorators for udfs.

87c93d6

removed old decorators implementation files

bf87ea8

new implementation file for decorators

41bce38

new Eva Argumet type files.

67409fb

new exception type for input mismatches.

202de65

renamed folder name

3a261a1

removed the test_udf_service file.

32f5bca

created new data types for eva

a45a404

created the decorators for udfs

6adde41

created a sample udf using the decorators

3152fef

updates to decorators

a5f9a09

changed the yolo decorators implementation

bc7ef4b

fixed linting errors.

317cf75

fixed linting errors.

5e92cd5

added the new pandas dataframe class

1b29afd

updates to decorators

423044b

updated the create udf parser format.

ee60a42

Added documentation.

bf58dca

removed the type parameter in create udf command.

67c8204

added udf type to setup decorator.

1696e9d

added the tags to function signature.

0c0f322

updated the decorators to do dynamic conversion.

1d9680b

moved the exception to the errors file.

added tests for udf decorators

835878b

added unit tests for udf decorators.

a4f331f

made some fixes to code

Merge branch 'master' of https://github.com/georgia-tech-db/eva into …

d6a3dd2

…decorators

removed the main fn for test

1238daa

linting changes

f3ce459

fixed some errors in tests.

1dd961f

linting changes.

IshSiva marked this pull request as draft March 17, 2023 15:01

IshSiva added 3 commits March 17, 2023 12:21

added documentation for udfs using decorators.

55d50b7

Merge branch 'master' of https://github.com/georgia-tech-db/eva into …

981186d

…decorators_2

added type and shape checking in decorators.

ad9ada1

IshSiva marked this pull request as ready for review March 29, 2023 21:41

jarulraj mentioned this pull request Mar 30, 2023

wip: Allow UDF Decorators to create AbstractUDF #469

Closed

gaurav274 reviewed Mar 30, 2023

View reviewed changes

eva/udfs/decorators/decorators.py Outdated Show resolved Hide resolved

gaurav274 reviewed Mar 30, 2023

View reviewed changes

docs/source/reference/udf_decorators.rst Outdated Show resolved Hide resolved

gaurav274 requested review from suryatejreddy and jarulraj March 30, 2023 22:56

IshSiva added 3 commits March 31, 2023 00:29

linting changes and removed comments

dee9272

removed documentation

33d13d5

fixed tests.

dafa814

linting changes.

xzdandy reviewed Mar 31, 2023

View reviewed changes

jarulraj approved these changes Apr 1, 2023

View reviewed changes

test/udfs/decorators/test_decorators.py Show resolved Hide resolved

added the check inputs flag

e1389dc

suryatejreddy requested changes Apr 13, 2023

View reviewed changes

updates for decorators

021178f

gaurav274 added the High Priority ⚡️ label Apr 28, 2023

IshSiva added 2 commits May 3, 2023 22:03

Merge branch 'master' of https://github.com/georgia-tech-db/eva into …

04aa636

…decorators_2

reformatted code.

ffa958d

IshSiva requested a review from gaurav274 May 4, 2023 02:09

gaurav274 requested a review from suryatejreddy May 4, 2023 03:17

jarulraj closed this Sep 17, 2023


		2. Create a Python class that inherits from `PytorchClassifierAbstractUDF`.

		* The `PytorchClassifierAbstractUDF` is a parent class that defines and implements standard methods for model inference.

feat: Decorators - part 2 #607

feat: Decorators - part 2 #607

Uh oh!

Conversation

IshSiva commented Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gaurav274 commented Mar 30, 2023

Uh oh!

gaurav274 commented Mar 30, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

suryatejreddy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaurav274 commented May 4, 2023

Uh oh!

jarulraj commented Sep 16, 2023

Uh oh!

xzdandy commented Sep 16, 2023

Uh oh!

Uh oh!

IshSiva commented Mar 15, 2023 •

edited

Loading