New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow passing of kwargs to theano.function. #2358
Changes from 2 commits
aebb3be
8fef999
1ec8d46
24fe0b6
e0668f7
3f8ae7a
4647de8
6ab3bc0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
from __future__ import print_function, division | ||
import inspect | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I know. Tests passed on Travis with it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why don't we add a travis test with Theano? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mostly because I don't know much about travis. If you're knowledgeable enough about this then I strongly encourage it, especially if people are going t start using this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I'll set travis up for some tests with theano when I have a few minutes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. great idea. I have see this also come up a few time on travis-ci mailing On Fri, Aug 9, 2013 at 4:14 PM, Aaron Meurer notifications@github.comwrote:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, a PPA with the latest release SciPy stack compiled for many python versions would be helpful to a ton of other projects. But this brings up the issue of version of the SciPy stack. What if I want to test against the latest dev version of packages in the SciPy stack. Or what if my software needs to be tested against older versions of the SciPy stack. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe a PPA can contain multiple versions of the same package. See for example the deadsnakes PPA. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They aren't really the same versions of the same package, they have different names. You have to apt-get install python2.6 or python3.2. They are different packages in the deadsnakes for each version of python. So the equivalent for scipy would be apt-get scipy0.7.2 or apt-get numpy1.7.1. So I guess as long as you build them all and have different names it would work. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'd have to do that anyway for the different python versions. |
||
|
||
from sympy.utilities import default_sort_key | ||
from sympy.external import import_module | ||
|
@@ -191,9 +192,21 @@ def dim_handling(inputs, dim=None, dims={}, broadcastables={}, keys=()): | |
|
||
def theano_function(inputs, outputs, dtypes={}, **kwargs): | ||
""" Create Theano function from SymPy expressions """ | ||
broadcastables = dim_handling(inputs, **kwargs) | ||
function_arg_names = inspect.getargspec(theano.function)[0] | ||
if set(function_arg_names) & set(kwargs.keys()): | ||
theano_function_kwargs = {} | ||
dim_handling_kwargs = {} | ||
for k, v in kwargs.items(): | ||
if k in function_arg_names: | ||
theano_function_kwargs[k] = v | ||
else: | ||
dim_handling_kwargs[k] = v | ||
else: | ||
theano_function_kwargs = {} | ||
dim_handling_kwargs = kwargs | ||
broadcastables = dim_handling(inputs, **dim_handling_kwargs) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems reasonable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, how about, rather than specifically looking at the keyword args that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But aren't you faced with this issue:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we can face this issue locally by explicitly requiring the inputs for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems fine. |
||
code = partial(theano_code, dtypes=dtypes, broadcastables=broadcastables) | ||
tinputs = map(code, inputs) | ||
toutputs = map(code, outputs) | ||
toutputs = toutputs[0] if len(toutputs) == 1 else toutputs | ||
return theano.function(tinputs, toutputs) | ||
return theano.function(tinputs, toutputs, **theano_function_kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So normally would this raise an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, because
on_unsused_input
is not an arg fordim_handling()
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And you can't pass in more values than specified inputs for theano.function.