use tf.debugging for search space assertions

[joelberkeley]

Use tf.debugging for assertions in space.py, so that error handling works in tf.function.

Also changed an implementation while I was at it

[joelberkeley]

felt like doing this, can undo if you'd prefer

[hstojic]

definitely more elegant!
am I missing something or assert_positive means >0 while here we want >=1?

[joelberkeley]

you're right. maybe i though positive meant strictly positive

[joelberkeley]

scrap that ... > 0 is the same as >=1 for integers, and that's also what assert_positive does

[joelberkeley]

this is a bit of a hack, to get around the fact that tf.debugging.assert_shapes considers 0 and [0] to have the same shape

[hstojic]

so we should use tf.debugging asserts throughout the code, rather than plain asserts (and ifelse statements with raising errors/warnings/etc)?

[joelberkeley]

so we should use tf.debugging asserts throughout the code, rather than plain asserts (and ifelse statements with raising errors/warnings/etc)?

tf.debugging works as expected with tf.function. raise doesn't (it will only raise when the function is compiled). assert is only used in source code for internal correctness checks. I don't believe trieste uses assert outside tests (assert is fine in tests, if you're not using tf.function)

[hstojic]

so we should use tf.debugging asserts throughout the code, rather than plain asserts (and ifelse statements with raising errors/warnings/etc)?

tf.debugging works as expected with tf.function. raise doesn't (it will only raise when the function is compiled). assert is only used in source code for internal correctness checks. I don't believe trieste uses assert outside tests (assert is fine in tests, if you're not using tf.function)

I see, I didn't know about the difference with respect to compilation.
So assert has the same issue if tf.function is used?
Actually, I have noticed we are dominantly using if-else accompanied with raising an error, while to me it usually seems like it should be an assert statement?

Regarding tf.function, at the moment it is used only in model optimization, is that correct?

[joelberkeley]

I see, I didn't know about the difference with respect to compilation.
So assert has the same issue if tf.function is used?

yes

Actually, I have noticed we are dominantly using if-else accompanied with raising an error, while to me it usually seems like it should be an assert statement?

I don't think assert is meant to be used for argument validation

Regarding tf.function, at the moment it is used only in model optimization, is that correct?

internally, yes, but users can wrap anything they like in tf.function

[hstojic]

Actually, I have noticed we are dominantly using if-else accompanied with raising an error, while to me it usually seems like it should be an assert statement?

I don't think assert is meant to be used for argument validation

Why not? It seems to me that plenty of people use it that way, e.g. https://wiki.python.org/moin/UsingAssertionsEffectively

Regarding tf.function, at the moment it is used only in model optimization, is that correct?

internally, yes, but users can wrap anything they like in tf.function

true... but then that would be an argument for using tf.debugging throughout the code

[hstojic]

regardless of the discussion above, content wise this is good to go in my opinion

[hstojic]

a thought on tf.function and users passing any piece of our code through it - it would be good to test that (all of) our code can be used within tf.function without errors. Some python code can cause errors within tf.function, we had that issue in Bellman.

[joelberkeley]

a thought on tf.function and users passing any piece of our code through it - it would be good to test that (all of) our code can be used within tf.function without errors. Some python code can cause errors within tf.function, we had that issue in Bellman.

this crossed my mind before. I guess I never thought it high priority. Not sure why, since trieste has already had problems with tf.function

re asserts and exceptions, this Q&A might be of interest

[joelberkeley]

btw i don't have merge permissions. feel free to merge