added validation after grappler optimization

[pyu10055]

This would catch non supported ops injected by grappler optimizer.

---

This change is 

[nsthorat]

Reviewable status: 0 of 1 approvals obtained (waiting on @pyu10055, @caisq, and @dsmilkov)

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 125 at r1 (raw file):

  if unsupported:
    print('Unsupported Ops in the model\n' + ', '.join(unsupported))

I think it may make sense to give a bit more info saying that there are unsupported ops post optimization so we can track this down a little bit easier, WDYT?

[caisq]

Reviewable status: 0 of 1 approvals obtained (waiting on @pyu10055, @caisq, and @dsmilkov)

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 125 at r2 (raw file):

print('Unsupported Ops in the model\n' + ', '.join(unsupported))

Shouldn't you throw an Exception here, instead of printing?

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 126 at r2 (raw file):

else:

You can remove this else branch and de-indent the line below.

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 228 at r2 (raw file):

after optimization\

How can you be sure that the unsupported ops are due to optimization?

[pyu10055]

Reviewable status: 0 of 1 approvals obtained (waiting on @pyu10055 and @dsmilkov)

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 125 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

print('Unsupported Ops in the model\n' + ', '.join(unsupported))

Shouldn't you throw an Exception here, instead of printing?

make sense, print and raise an exception

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 126 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

else:

You can remove this else branch and de-indent the line below.

since exception is raised, I can removed the else branch.

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 228 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

after optimization\

How can you be sure that the unsupported ops are due to optimization?

since we also did validation prior the optimization.

[caisq]

Reviewable status: 0 of 1 approvals obtained (waiting on @caisq, @pyu10055, and @dsmilkov)

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 125 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

make sense, print and raise an exception

The pattern of printing and raising an Exception is unusual. I think you should just raise an Exception, with the information included as the string argument for the exception's constructor.

Also, Exception is a low-level class in Python. We usually raise things like ValueError.

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 288 at r3 (raw file):

try:

To test exception raised, you can do

with self.assertRaisesRegexp(r'....'):
   # ...

[caisq]

Reviewable status: 0 of 1 approvals obtained (waiting on @caisq, @pyu10055, and @dsmilkov)

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 288 at r3 (raw file):

Previously, caisq (Shanqing Cai) wrote…

try:

To test exception raised, you can do

with self.assertRaisesRegexp(r'....'):
   # ...

Actually, it should be something like:

with self.assertRaisesRegexp(ValueError, r'...'):
  # ...

[pyu10055]

Reviewable status: 0 of 1 approvals obtained (waiting on @caisq and @dsmilkov)

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 125 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

The pattern of printing and raising an Exception is unusual. I think you should just raise an Exception, with the information included as the string argument for the exception's constructor.

Also, Exception is a low-level class in Python. We usually raise things like ValueError.

Done.

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 126 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

since exception is raised, I can removed the else branch.

Done.

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 228 at r2 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

since we also did validation prior the optimization.

Done.

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 288 at r3 (raw file):

Previously, caisq (Shanqing Cai) wrote…

Actually, it should be something like:

with self.assertRaisesRegexp(ValueError, r'...'):
  # ...

Done.

[caisq]

Reviewable status: 0 of 1 approvals obtained (waiting on @caisq, @pyu10055, and @dsmilkov)

---

python/tensorflowjs/converters/tf_saved_model_conversion.py, line 131 at r4 (raw file):

in the model

--> 'in the model after optimization'

[caisq]

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @caisq, @pyu10055, and @dsmilkov)

---

python/tensorflowjs/converters/tf_saved_model_conversion_test.py, line 24 at r5 (raw file):

from unittest.mock import Mock

Remove this line. It's not used and is causing test failure.