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

update for tensorflow v2 compatibility #917

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vladmandic
Copy link

@vladmandic vladmandic commented Mar 1, 2021

This PR updates MMdnn for TensorFlow v2 compatibility by utilizing legacy support in TF2

Main reason is that TF1 requires Python 3.7 or lower and new systems come with Python 3.8 which I did not want to downgrade

Change is basically switching imports from tensorflow to tensorflow.compat.v1 and setting appropriate flags
Only additional change needed is definition of flatten layer as it has to be explicitly defined as legacy since contrib namespace no longer exists

Additional change in this PR is that emitted code does not use Inf constant as any constant may be undefined depending on the TF backend used

For example, Python or NodeJS with Tensorflow backend will resolve it fine, but GLSL code generated by TensorFlow/JS WebGL backend does not handle constants as constants in general do not exist in GLSL

I've tested this with converting Places365 ResNet152 model from Caffe to Tensorflow saved model and further using tensorflowjs_convert to convert this new TF saved model to TFJS graph model and then tested both saved model and graph model using NodeJS TensorFlow backend, WASM backend and WebGL backend

@ghost
Copy link

ghost commented Mar 1, 2021

CLA assistant check
All CLA requirements met.

Copy link

@Doch88 Doch88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes it usable with TensorFlow 2. However, it can still only be used with TensorFlow 1 models. So, the name "update for TensorFlow v2 compatibility" is not so correct. Another problem is that this commit does not check TensorFlow version, so it breaks the compatibility with older versions of the framework.

@vladmandic
Copy link
Author

vladmandic commented Mar 19, 2021

so it breaks the compatibility with older versions of the framework.

Handling both tensorflow v2 and v1 could be done with

try:
  import tensorflow.compat.v1 as tf
except ImportError:
  import tensorflow as tf

(I hate that concept, but it works)

However, it can still only be used with TensorFlow 1 models

Not really as tensorflow.compat.v1 has access to all ops from v2
so only v2 models that could not be converted are ones that use new execution flow - and that is really a) very rare, b) outside of the scope of this change

So, the name "update for TensorFlow v2 compatibility" is not so correct.

True. I'm ok with naming it anything else...

@mwessley
Copy link

Is there a reason why this is not merged yet? would be cool to have it in the master branch.

@Doch88
Copy link

Doch88 commented Jul 13, 2021

Is there a reason why this is not merged yet? would be cool to have it in the master branch.

Because this project is abandoned.

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 this pull request may close these issues.

3 participants