Missing symbol for Abseil ParseTime Op

[seanpmorgan]

Describe the bug
Since the 20101030 nightly, our library fails when opening the _parse_time_op.so because of an undefined symbol error. Typically this occurs when there is an ABI incompatibility between core TensorFlow and our custom-ops.

tensorflow.python.framework.errors_impl.NotFoundError: .../custom_ops/text/_parse_time_op.so: undefined symbol: _ZN4absl9ParseTimeERKSsS1_PNS_4TimeEPSs

This particular custom-op utilizes absl and the mangled symbol name does not match what our compiled operation thinks it should be. I'm not really sure how this would happen since the rest of the TensorFlow library was compiled with a compatible gcc version.

CC'ing some people who may have insight into how this can happen:
@yifeif @r4nt @perfinion @gunan

I suppose we could make absl a dependency in our TFA build and compile it ourselves but this seems to be going down a bad path.

Failing Log:
https://source.cloud.google.com/results/invocations/a71d103a-ecac-421c-bc53-ef06648fdc9f/targets/tensorflow_addons%2Fubuntu%2Fgpu%2Fpy3%2Fpresubmit/log

[r4nt]

@scentini for whether this might be due to us not exporting all symbols from absl any more

[scentini]

Yep, seems like it is related to f58c9f8, which changes what we export, see bazelbuild/bazel#7362.

The absl symbols probably came from here, and due to f58c9f8 we just don't export the symbol anymore.
I think the right way forward is to make _parse_time_op.so depend on absl.

[seanpmorgan]

Great thanks that makes sense! cc @helinwang as code owner.

Helin if you have time could you submit a patch to add absl as a build dependency.

[scentini]

After discussion with @r4nt, it might be better to not pass the absl dependency to _parse_time_op.so, but rather add alwayslink = 1 to the absl library. (thus make sure that absl symbols are present in @local_config_tf//:libtensorflow_framework)

The reason for this is, if we pass absl as a dependency, we may end up linking global initializers multiple times.

I can send out a fix for this tomorrow.

[helinwang]

Thanks, everyone! If there is anything I can help, please let me know.

[leozwang]

thanks guys! when will the bug-fix be ready? next release or next nightly build?

[seanpmorgan]

Hmmm so it seems whatever has landed in the tf_nightly-2.1.0.dev20191114 nightly has not fixed this build issue:
https://source.cloud.google.com/results/invocations/f7858a66-3ac1-4717-a85a-bf160e754918/targets/tensorflow_addons%2Fubuntu%2Fcpu%2Fpy3%2Fpresubmit/log

I saw there was some issues with merging the patch... can we confirm that the fix landed but didn't solve our issue @scentini @helinwang

[scentini]

The PR that fixed this was reverted in 5acf6bd.
An internal PR that'll fix this and similar issues by reverting the change that introduced these errors is in the process of being merged.

[georgesterpu]

@seanpmorgan it appears to work now with

tf-nightly                2.1.0.dev20191119
tfa-nightly               0.7.0.dev20191119

[seanpmorgan]

@seanpmorgan it appears to work now with

tf-nightly                2.1.0.dev20191119
tfa-nightly               0.7.0.dev20191119

Thanks @georgesterpu! #658

[seanpmorgan]

The PR that fixed this was reverted in 5acf6bd.
An internal PR that'll fix this and similar issues by reverting the change that introduced these errors is in the process of being merged.

@scentini @r4nt Now that the fix has landed do we know if it'll be cherry-picked onto r2.1?

[scentini]

Unfortunately this fix caused internal breakages, so we'll be rolling it back. Whatever the solution is, it will be cherrypicked onto r2.1. I'll keep you updated.

[Squadrick]

Failure log.

Version: tf_nightly-2.1.0.dev20191120

Error:

tensorflow_addons/custom_ops/text/_parse_time_op.so: undefined symbol: _ZN4absl9ParseTimeERKSsS1_PNS_4TimeEPSs

We'll need to pin it to tf_nightly-2.1.0.dev20191119.

[ezzeldinadel]

still suffering from this in the current nightly build

[seanpmorgan]

still suffering from this in the current nightly build

Hi @ezzeldinadel can you confirm the nightly version you're using. We've dropped the ParseTime Op until a fix has landed in TF-core so there should be no issue with the nightlies:

https://colab.research.google.com/drive/1KV4J4Gs9VrNmxzstXQOhdYufg1BASaVI

[seanpmorgan]

#728 is passing so I'm assuming this fix landed in nightly. Can we confirm that it was picked into 2.1?

[seanpmorgan]

Hi @scentini so I checked and this build fails on TF2.1rc0 but passes on tf-nightly. Could you point me to the commit that ultimately fixed this so I can request it to be picked into TF2.1? (Assuming that isn't already happening)

[scentini]

The commit was tensorflow/tensorflow@c9d1b6d, I asked for it to be cherrypicked, you can follow tensorflow/tensorflow#34764. Thank you for your patience!

[seanpmorgan]

Thanks very much for your help!

[helinwang]

Thanks @scentini and @seanpmorgan !