frontends.tensorflow.activations: add support for elu and selu

[hmahmood24]

Close #5387
Close #5388

[YushaArif99]

@hmahmood24, the tests for selu activation function seem to be failing. Kindly refer to the test-frontend-tensorflow tests to see the failing test cases and resolve them accordingly.

[hmahmood24]

Hi @YushaArif99,

Thanks for the response. I have looked into the points you made and here is what I think:

• The support for adding a dictionary to define supported or unsupported dtypes for a particular function has been removed from Ivy. The link you shared also has no information whatsoever regarding adding dictionaries, but only regarding adding tuples. You can verify that yourself. Yes, one can add version-specific supported and unsupported dtypes but not for the following scenario.

• For my implementation (and hopefully many others), adding supported or unsupported dtypes in the form of a dictionary seems a lot more usable. Here is why: tensorflow supports float16, float32, float64 in selu method which internally calls the elu method which again calls the ivy.exp method which however is not defined for a float16 dtype when using a torch backend.

• Now, even though this has been flagged for in the backend exp method for torch, during my testing, Hypothesis still keeps generating a float16 dtype, thereby causing the tests to fail (I have verified this myself). Intuitively, it makes a lot more sense that this could be tagged in the frontend function selu.supported_dtypes like so: { ... "tensorflow": ( "float16", "float32", "float64", ), "torch": ( "float32", "float64", ), ... }

• However, when I try to go and define a dictionary like above, I get the following error thrown at the time of testing: ivy.exceptions.IvyBackendException: numpy: function_unsupported_dtypes: type of x: <class 'dict'> must be one of the allowed types: <class 'tuple'>. Seems like only tuples are supported when flagging supported or unsupported dtypes for a function as seen from this block of code in the master branch.

• Thus, there is a need to add support for defining dictionaries (along with tuples as previously) to flag the supported or unsupported dtypes of a function accordingly. I have committed the solution to the same in the latest commits to this forked branch. I have also verified that this does not break any of the tests where tuples are used to flag the supported or unsupported dtypes. This commit, thus, only adds the support of flagging using dicts along with tuples.

• I have verified that with this definition of supported or unsupported dtypes, all the tests for the selu method pass without any errors against all the backends.

Looking forward to a review from you. Thanks! 😊

[YushaArif99]

Hi @hmahmood24

Thanks a lot for the detailed response! Although the points you've highlighted make complete sense, It is odd that you had to modify the data_type.py file to define the supported_dtype attribute in the form of a dictionary. From my understanding, this should be allowed by default as is the case here:
https://github.com/unifyai/ivy/blob/6089953297b438c58caa71c058ed1599f40a270c/ivy/functional/frontends/tensorflow/math.py#L161

Perhaps it was caused by the latest commit that was made which changed the implementation of the supported_dtype and unsupported_dtype helper functions (6c5975a).

Although your change does seem logical, It might cause some unwanted bugs elsewhere which is why I'll request someone else in the team to review the changes as well just to be safe.

Other than that, everything seems fine and the PR is ready to be merged.

[YushaArif99]

@sherry30 could you kindly take a look at this PR and ensure that everything is ready to be merged?

[hmahmood24]

Thanks for the apt response @YushaArif99!
The function you quoted seems to have changed in the current master branch and the dictionary has now been removed from that

[YushaArif99]

Hi @hmahmood24
Sorry for the late-than-usual reply. Was swamped with work so reviewing this PR completely slipped under my radar.
Given that I still haven't gotten a review from @sherry30, It's better to approve your changes and merge the PR rather than keep you waiting. The removal of the einops condition from the data_type.py seems logical to me and I'm honestly not sure why that condition was there in the first place 😅.
Could you kindly resolve the merge conflicts so that the PR is ready to be merged?

[hmahmood24]

Hi @YushaArif99,

It's alright. I can understand. Thanks for responding to my commits. I have made the changes you requested and additionally merged the conflicts with the master branch so that the PR can be merged accordingly. Let me know if there is anything else from my end. Thanks! 😊

[YushaArif99]

@hmahmood24 Thanks for making the changes. I will now merge the PR.
Thanks a lot for your contribution to the Ivy codebase😊