Updated the redistributed version of Tensorflow to 1.14

[harishsk]

fixes #3962

[zeahmed]

Looks good! Any reason why some of the tests are failing?

[harishsk]

I have probable cause for why Mac tests are failing. The 1.13 package for Mac used to have .so files that we would rename to .dylib files in our build process. As part of the rename, it looks like we were also registering the path in the load library search path.
The new 1.14 packages already have .dylib files so the rename isn't necessary. But when I remove the code for renaming, the tests aren't finding the relevant libraries in their path. So I need to do something different.

I don't yet know whey the Linux tests are failing.

[harishsk]

A longer explanation of the fix is necessary.
In v1.13.1 of Tensorflow, both linux and mac binaries are named libtensorflow.so and libtensorflow_framework.so.

However, in v1.14.0 the naming scheme and archive contents have changed as follows:

• Linux
  • libtensorflow.so.1.14.0
  • libtensorflow.so.1
  • libtensorflow.so
  • libtensorflow_framework.so.1.14.0
  • libtensorflow_framework.so.1
  • libtensorflow_framework.so
• Mac
  • libtensorflow.1.14.0.dylib
  • libtensorflow.1.dylib
  • libtensorflow.dylib
  • libtensorflow_framework.1.14.0.dylib
  • libtensorflow_framework.1.dylib
  • libtensorflow_framework.dylib

In the list above, the files without a version number, sym-link to the files with a version. The fix has been complicated by the linux versions not using the .so suffix for their files and using the version number instead.

If we only copy the files with version numbers, the tests fail because they are looking for a file with the standard extension. If we copy only the files with standard extension, the tests fail because the libtensorflow.so file attempts to load the libtensorflow_framework.so.1 file internally and doesn't find it. So we have to copy all the files.

And further, all these files have to be copied to the directories that the tests run from so that they can find them locally.

The ideal fix is to extract the files from the archive and just add that directory to the system's library search path. This is easier on Linux and Mac but a bit more complicated on Windows. I am stopping this here, because the current proposal is to migrate tensorflow redist package to the TensorFlow.NET repo.

[eerhardt]

Just an FYI -

The tests in the repo do not run against the produced NuGet packages. Instead, you need to manually test that the NuGet packages are created correctly.

Can you build the packages locally, and then manually smoke test them to ensure they are created correctly?

[eerhardt]

How is this different than the FullAssemblyPath above it? It looks like we'd be trying to copy the same file twice.

[harishsk]

Yes, for Windows, it ends up copying the same file twice. Without that line, I get an error in Windows builds that AssemblyPathMajorVersion is undefined.
I guess the other way to fix this is to make the optional copy conditional on the OS being linux or mac. Let me try that.

[eerhardt]

If you want, you can try adding another section:

      <NativeAssemblyReference Condition="'%(NativeAssemblyReference.MajorVersion)' != ''">
        <!-- Tensorflow has a different naming scheme for v1.14.0. Those binaries need to be copied along with the standard names -->
        <AssemblyPathWithMajorVersion Condition="'$(OS)' != 'Windows_NT'">$(NativeOutputPath)$(LibPrefix)%(NativeAssemblyReference.Identity)$(LibExtension).%(NativeAssemblyReference.MajorVersion)</AssemblyPathWithMajorVersion>
        <AssemblyPathWithMajorVersion Condition="$([MSBuild]::IsOSPlatform('osx'))">$(NativeOutputPath)$(LibPrefix)%(NativeAssemblyReference.Identity).%(NativeAssemblyReference.MajorVersion)$(LibExtension)</AssemblyPathWithMajorVersion>
      </NativeAssemblyReference>

And then lower in your copy statement:

 Condition="'%(AssemblyPathWithMajorVersion)' != '' AND Exists('%(AssemblyPathWithMajorVersion)')">

[eerhardt]

It would be good to test the produced NuGet packages separately to ensure they still work correctly. Just a simple smoke test that loads up a TF model and executes it should suffice.

[harishsk]

I have tested the Nuget packages separately by having the dotnet/machinelearning samples reference the locally built nuget packages.