search

google / objax

Apache License 2.0
769 stars 77 forks source link

Give error if assigning variable wrong shape #115

Closed carlini closed 3 years ago

carlini commented 4 years ago

This PR asserts that the shape of a variable isn't changed when assigning to it. Most of it is fairly uncontroversial (#106).

However I do one further thing. In order for the error message when restoring a variable show the name of the variable that caused the error, I add some additional state to BaseVar to support this. As a result, if restore fails, it won't just say that some variable had the wrong shape, but now it will error with exactly which variable did.

The other way to do this would be to wrap the restore assign() in a try/catch and then repeat the error, but with the name attached. The benefit of this approach is the error isn't duplicated, but it's a bit more code.

(I also imagine that having the variable name could help for some other errors in the future.)

carlini commented 4 years ago

Ah well. This is interesting, and is going to have to be a bigger PR than the trivial change I'd thought:

  1. Replicate() assigns a variable of a different shape, but this needs to be allowed.

  2. Some of the other tests abuse this feature and need to be fixed.

carlini commented 3 years ago

It looks like the test has stalled (or is taking >12 hours)

carlini commented 3 years ago

Huh. I'm not sure how I got in that state, but it wasn't intended.

carlini commented 3 years ago

Ah well. I've just completely messed up my local state. I'm going to close this PR and re-open one with a clean history.