[Bug] skip_gram_sample assert rank is 1

[seanpmorgan]

While converting the tests for skip_gram_ops, I noticed a test that was failing in TF2 eager execution, but I'm not sure why.

This test:

with self.assertRaises(ValueError):
    invalid_tensor = constant_op.constant([[b"the"], [b"quick"], [b"brown"]])
    text.skip_gram_sample(invalid_tensor)

Should raise an error because of this check in the op definition. When decorating with @test_util.run_deprecated_v1 the test correctly passes (error is raised), but otherwise it fails to raise an Error.

Is this related to inspecting the rank of EagerTensors? Or is something else at play?

[karmel]

@jaingaurav - Is this something you have seen? Can you take a look? (Note that this is for the Addons repo, which is migrating functionality currently in contrib.)

[jaingaurav]

@test_util.run_deprecated_v1 will cause the test to run in graph mode hence the check fires during graph construction. In Eager, you should see a slightly different assertion pop up due to a runtime check. This results in an InvalidArgumentError exception. The test need to be modified to handle both types of assertions. One approach would be to use something like self.assertRaisesRegexp(Exception, <some string that matches both exceptions>)

[seanpmorgan]

@jaingaurav Thank you very much for looking into this. So if I remove this comment and the assertion context then I still see the test as passing and no Error at all being raised.

Seems like I would need to add a new OP_REQUIRES in the Compute method that verifies tensor rank?

[jaingaurav]

@seanpmorgan: Yes, that would be advisable to ensure better safety in eager mode.

[facaiy]

@jaingaurav So, will shape inference be supported for eager mode later? Otherwise, duplicate codes exist in both graph constructing and kernel computing.