Skip to content
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

Don't set compute_test_value to raise #566

Closed
nouiz opened this issue Jul 14, 2014 · 11 comments · Fixed by #2103
Closed

Don't set compute_test_value to raise #566

nouiz opened this issue Jul 14, 2014 · 11 comments · Fixed by #2103

Comments

@nouiz
Copy link
Contributor

nouiz commented Jul 14, 2014

In this Theano issue, a user got problem, as pymc set compute_test_value to raise by default.

The problem is that there is one pymc op compiled. So when Theano load its cache, it load pymc, that change Theano compute_test_value, even if the user didn't load pymc itself:

Theano/Theano#1951 (comment)

It is not yet documented, but Theano now give much better error message for shape error. Here is an example:

import theano
x,y = theano.tensor.vectors('xy')
f = theano.function([x,y], x+y)
f([1], [2,3])
Traceback (most recent call last):
  File "test_err.py", line 4, in <module>
    f([1], [2,3])
  File "/home/nouiz2/repos/Theano/theano/compile/function_module.py", line 589, in __call__
    self.fn.thunks[self.fn.position_of_error])
  File "/home/nouiz2/repos/Theano/theano/compile/function_module.py", line 579, in __call__
    outputs = self.fn()
ValueError: Input dimension mis-match. (input[0].shape[0] = 1, input[1].shape[0] = 2)
Apply node that caused the error: Elemwise{add,no_inplace}(x, y)
Inputs types: [TensorType(float64, vector), TensorType(float64, vector)]
Inputs shapes: [(1,), (2,)]
Inputs strides: [(8,), (8,)]
Inputs scalar values: [array([ 1.]), 'not scalar']

Backtrace when the node is created:
  File "test_err.py", line 3, in <module>
    f = theano.function([x,y], x+y)
  File "/home/nouiz2/repos/Theano/theano/tensor/var.py", line 117, in __add__
    return theano.tensor.basic.add(self, other)

HINT: Use the Theano flag 'exception_verbosity=high' for a debugprint of this apply node.

Check, I print the backtrave when the node was created. So the user know when the node was created without using test value! This isn't in the released version of Theano.

I would suggest you to don't change theano default value for compute_test_value and if people ask questions about shape error, tell them about this. If you don't like this, I would recommand to at least change compute_test_value to "warn" instead of "raise".

@twiecki
Copy link
Member

twiecki commented Aug 17, 2014

If I understand you correctly, you're saying that test_value should not be changed after creating the theano op. For a little while I thought we should instead have the current point in Model rather than in the theano ops. find_MAP() would then set that internal value, similar to how in pymc 2. @jsalvatier?

@twiecki
Copy link
Member

twiecki commented Aug 17, 2014

Oh I see, this is related to #561.

@jsalvatier
Copy link
Member

The reason I set 'raise' was to avoid silent and mysterious failures when data arrays don't line up. The problem is that if we don't set raise, we only get an error when we try to sample. Since the likelihood may be quite complicated, it will be difficult to figure out where this error is coming from.

Is there a way we can set 'raise' only temporarily? Perhaps within with Model() as model:?

@twiecki how come you want to store the point within Model?

@nouiz
Copy link
Contributor Author

nouiz commented May 26, 2015

With the dev version, now there is a decorator that allow you to do so:

@theano.configparser.change_flags(**{'vm.lazy': True})

@jsalvatier
Copy link
Member

Oh cool. Its a function decorator that changes it for the duration of the function? It would be really nice to have a function that returns the old config values, modifying the current one. And a function that applies the old context.

PyMC uses __enter__ and __exit__ functions in its with Model() as model: syntax and this would be the ideal place to do this. Perhaps we need to roll our own.

@nouiz
Copy link
Contributor Author

nouiz commented May 26, 2015

That is a good idea. I won't have time shortly for this. If you do it, can
you do it on in theano?
Le 26 mai 2015 02:54, "John Salvatier" notifications@github.com a écrit :

Oh cool. Its a function decorator that changes it for the duration of the
function? It would be really nice to have a function that returns the old
config values, modifying the current one. And a function that applies the
old context.

PyMC uses enter and exit functions in its with Model() as model:
syntax and this would be the ideal place to do this. Perhaps we need to
roll our own.


Reply to this email directly or view it on GitHub
#566 (comment).

@jsalvatier
Copy link
Member

Sure.

On Tue, May 26, 2015 at 5:20 AM, Frédéric Bastien notifications@github.com
wrote:

That is a good idea. I won't have time shortly for this. If you do it, can
you do it on in theano?
Le 26 mai 2015 02:54, "John Salvatier" notifications@github.com a écrit
:

Oh cool. Its a function decorator that changes it for the duration of the
function? It would be really nice to have a function that returns the old
config values, modifying the current one. And a function that applies the
old context.

PyMC uses enter and exit functions in its with Model() as model:
syntax and this would be the ideal place to do this. Perhaps we need to
roll our own.


Reply to this email directly or view it on GitHub
#566 (comment).


Reply to this email directly or view it on GitHub
#566 (comment).

@twiecki
Copy link
Member

twiecki commented Jul 18, 2016

Oh cool. Its a function decorator that changes it for the duration of the function? It would be really nice to have a function that returns the old config values, modifying the current one. And a function that applies the old context.

@nouiz did this get implemented by any chance?

@nouiz
Copy link
Contributor Author

nouiz commented Jul 18, 2016

There is this decorator that change a value of a flag:

@theano.configparser.change_flags(compute_test_value='off')

On Mon, Jul 18, 2016 at 9:31 AM, Thomas Wiecki notifications@github.com
wrote:

Oh cool. Its a function decorator that changes it for the duration of the
function? It would be really nice to have a function that returns the old
config values, modifying the current one. And a function that applies the
old context.

@nouiz https://github.com/nouiz did this get implemented by any chance?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#566 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AALC-w-MkkkumXPTPUmssFx9KrjunUv6ks5qW4BCgaJpZM4CM7IJ
.

@twiecki
Copy link
Member

twiecki commented Jul 18, 2016

Right, ideally it could used as a function call though because we're using a context manager and not a function that can be decorated.

@nouiz
Copy link
Contributor Author

nouiz commented Jul 18, 2016

Go see in theano/configparser.py. You probably change change the
implementation of the current decorator to do what you want.

On Mon, Jul 18, 2016 at 9:51 AM, Thomas Wiecki notifications@github.com
wrote:

Right, ideally it could used as a function call though because we're using
a context manager and not a function that can be decorated.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#566 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AALC--qeQOePVovuLKDgxBLVjMHAYmL9ks5qW4TEgaJpZM4CM7IJ
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants