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

Tf2 compatibility #17

Closed
wants to merge 37 commits into from
Closed

Tf2 compatibility #17

wants to merge 37 commits into from

Conversation

jackd
Copy link

@jackd jackd commented May 3, 2019

Attempt to make things tf 1.x/2.0 compatible as discussed here.

I've ported the original tests to run using 1.x and 2.0 (where possible), though the 2.0 external configurables/utils still need tests. I'm unsure how much of the new API gin wants to cover, whether it wants to all be under keras or not, or how they want to namespace v1 vs v2 etc.

I've tried to mimic tensorflows tf.compat.v1/v2 setup, but maybe this makes things more ugly than necessary.

There's some dirty monkey patching of tf.io to support 1.12 which can disappear if the version constraint is lifted.

ConfigSaverHook has been re-implemented as a keras Callback. Apart from that, this is mostly just a text substitution at this point - not much effort has been made to make it work "the tensorflow 2 way".

sguada and others added 30 commits July 11, 2018 16:56
Add enum34 requirement

version 0.1.1

PiperOrigin-RevId: 204213584
PiperOrigin-RevId: 204219076
PiperOrigin-RevId: 204380049
Make zip explicitly into a list for compatibility with Python 3.

PiperOrigin-RevId: 204927989
PiperOrigin-RevId: 211175333
PiperOrigin-RevId: 215710578
PiperOrigin-RevId: 218168789
This prevents breakage in an upcoming CL that moves the `text_summary` module and hides the PLUGIN_NAME constant.  As a replacement, we use the `text_pb()` method from TensorBoard's own summary API that is the equivalent of the text summary op but generates the Summary proto directly without any op execution.

The tag needs to be manually overridden because text_pb() currently adds a "/text_summary" suffix, but we're planning on removing that in the near future, so I have a TODO to myself to remove the override then.

PiperOrigin-RevId: 219666504
Mainly, this removes reliance on the text_summary module. Once all TB changes are in external TF we can update to rely on the newer (cleaner) API for text pb generation.

PiperOrigin-RevId: 220354498
As a first-time user of gin, it seemed like gin might automatically detect and parse my `config.gin` file.  But it appears that we're actually required to call `gin.parse_config_file`, so I think it would help to explicitly include an example of that in the documentation of basic usage.
[docs] Include example of `gin.parse_config_file`
PiperOrigin-RevId: 220398335
PiperOrigin-RevId: 221822490
This allows to use constants_from_enum in a similar way as external_configurable.

PiperOrigin-RevId: 225354908
… read command-line parameters.

PiperOrigin-RevId: 226252299
PiperOrigin-RevId: 228899251
PiperOrigin-RevId: 230403051
PiperOrigin-RevId: 232975361
PiperOrigin-RevId: 234233017
@sguada sguada requested a review from dhr May 10, 2019 18:25
@jackd
Copy link
Author

jackd commented Aug 20, 2019

Looks like you guys have got on top of this separately.

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.

8 participants