joschu / cgt

Computation Graph Toolkit
Other
628 stars 87 forks source link

to support variable length argument in cgt.dimshuffle #27

Open avostryakov opened 8 years ago

avostryakov commented 8 years ago

Theano support: tensor.dimshuffle('x', 0, ...) together with tensor.dimshuffle(['x', 0, ...]). I think it is a good idea to support both syntacsis in cgt because theano-people use both.

bstadie commented 8 years ago

We will add this functionality in the next little while.

If you can't wait, dimshuffle is entirely contained in cgt.api. Adding this feature would amount to adding a check for a tuple and converting it to a list at the beginning of the function.

f0k commented 8 years ago

Adding this feature would amount to adding a check for a tuple and converting it to a list at the beginning of the function.

Not quite. It would need changing dimshuffle in core.py from:

    def dimshuffle(self, pattern):
        "see cgt.dimshuffle"
        return cgt.dimshuffle(self, pattern)

to

    def dimshuffle(self, *pattern):
        "see cgt.dimshuffle"
        if len(pattern) == 1 and isinstance(pattern[0], list):
            pattern = pattern[0]
        return cgt.dimshuffle(self, pattern)

Alternatively, you could have Node.dimshuffle pass on *pattern unchanged and modify cgt.dimshuffle to take an argument list and check whether it's a single argument of type list as above. Note that you won't be able to add any positional arguments to dimshuffle in future if you go that route.

bstadie commented 8 years ago

Ah right. Forgot about modifying the shortcut in core.py. In any case this is a trivial thing to implement.