OpenMined / Threepio

A multi-language library for translating commands between PyTorch, TensorFlow, and TensorFlow.js
Other
56 stars 15 forks source link

Incorrect mapping of add command torch->tensorflow #144

Closed Praful932 closed 3 years ago

Praful932 commented 3 years ago

Description

The example usage as given in the usage docs and the tests file does not run as expected for the add command, rather gives an error. Example below 👇

How to Reproduce

Run this code

import tensorflow as tf
import torch as th
from pythreepio.threepio import Threepio, Command

# Create a threepio translating from pytorch -> tensorflow python
threepio = Threepio("torch", "tf", tf)

# Create your command to translate
args = [
    tf.constant([[1, 0], [0, 1]]),
    tf.constant([[0, 1], [1, 0]])
]
kwargs = {}
cmd = Command("add", args, kwargs)

# Translate the command
translated_cmd = threepio.translate(cmd, lookup_command=True)

# Produces error
translated_cmd[0].execute_routine()

TypeError: add() takes 1 positional argument but 2 were given

On inspecting

translated_cmd[0].attrs

Output - ['tf', 'keras', 'layers', 'add']

Expected Behavior

The expected behaviour should be that, both the args should be added and according to me translated_cmd[0].attrs should contain ['tf', 'math', 'add'].

Screenshots

image

System Information

Additional Context

I am willing to contribute to this issue.

Nolski commented 3 years ago

Good catch! This may be related to issue #16 which hopes to add additional accuracy. We also should be able to resolve this by manually mapping the add command in translations.py

@Praful932 I'll assign this to you as you mentioned you are interested in contributing to this issue but let me know if you no longer have time and I'll remove it :)

Praful932 commented 3 years ago

Hi @Nolski This issue seems to be fixed in the dev branch 😄. Only persists on the master branch and pypi. Should be fixed with a new release. This issue can be closed ✔

image