[WIP] Create a minimal TF binding #4

ericstj · 2018-08-13T19:08:21Z

This replaces usage of TF# with a locally built copy of the same types.

It's a work in progress as I plan to further trim the source, change names and namespaces, refactor, etc.

Best reviewed commit-by-commit.

ericstj · 2018-08-14T21:51:12Z

With ce3925f I've trimmed down TF based on what we actually use.

Here's the diff between what TF# had and what we have: https://gist.github.com/ericstj/ada86e95c9178a58d2e1d038a14f4f31

Here's the diff between what we have and what we could further trim. I took a guess at what we might use in the future and left that stuff in:
https://gist.github.com/ericstj/c59f5adf7d0ec6d5c38e7d8ffa4e176d
#Resolved

ericstj · 2018-08-14T23:07:52Z

This now has a trimmed down version of TF# internal to MS.ML.Transforms. We can further refactor if folks wish but this should illustrate the rough size of the support cost of owning the portion of TF# we depend on. #Resolved

This is just a stub to begin iterating on. To follow: 1. Pull over a copy of TF# source 2. Tree-shake unused code. 3. Rewrite / Rename TF# API as appropriate. 4. Come up with a strategy for providing TF bins (using TF#'s copy now, merely as a starting point).

*NOT BUILDING*

zeahmed · 2018-08-15T17:49:32Z

src/Microsoft.ML.Transforms/TensorFlow/Tensor.cs

+		/// <summary>
+		/// Creates a constant tensor with a single dimension from a float value.
+		/// </summary>
+		public unsafe TFTensor (float value)


public unsafe TFTensor (float value) [](start = 2, length = 36)

Can we have a templated versions of creating TFTensor from type ?

We can have generic factory method that does that. It'll need to handle T's specifically via switch. What Ts do you expect it to handle? Scalars, Arrays of scalars, both?

In reply to: 210351058 [](ancestors = 210351058)

For scalar only right now. Looks like SetupTensor method can work for arrays as it's already being used for arrays underneath every constructor.

In reply to: 210355494 [](ancestors = 210355494,210351058)

Yes, SetupTensor can handle arrays and does the right thing pinning the managed array to avoid a copy. Note that will only work if the caller knows they have an array, if the caller doesn't and itself is a generic on T that might be an array we'll need to make the factory method handle it to.

I'll add a method to do this, it's very similar to those we have in System.Numerics.Vectors and System.Numerics.Tensors. It doesn't seem to difficult to do the same for an array.

In reply to: 210356670 [](ancestors = 210356670,210355494,210351058)

I can use this method directly. This method takes TFDataType as input and I have that information from model. I can use this method for arrays. For scalar there is no such method so I needed a templated one.

In reply to: 210360411 [](ancestors = 210360411,210356670,210355494,210351058)

Yeah, I'll make something, you may want to use it in both scenarios. Stay tuned.

In reply to: 210360411 [](ancestors = 210360411,210356670,210355494,210351058)

I've fixed some minor rule issues and disabled the naming analyzers since those churn the source too much.

For now it is BYOTF (bring your own TensorFlow) so the tests are grabbing the TF bits for TF#.

zeahmed · 2018-08-15T17:50:37Z

src/Microsoft.ML.Transforms/TensorFlow/Tensor.cs

+		/// Creates a 1 dimensional tensor from an array of booleans.
+		/// </summary>
+		/// <param name="data">Data.</param>
+		public TFTensor (bool [] data) : base (SetupTensor (TFDataType.Bool, data, size: 1)) { }


SetupTensor [](start = 41, length = 11)

I think I need this method exposed so that I can use it in the transform.

It's effectively exposed now that I have moved this code into the same assembly and its internal. I need to understand the use case to determine if calling directly is the right thing to do.

In reply to: 210351552 [](ancestors = 210351552)

…ake internal

zeahmed · 2018-08-15T18:21:46Z

src/Microsoft.ML.Transforms/TensorFlow/Tensorflow.cs

+
+		// extern void TF_SessionRun (TF_Session *session, const TF_Buffer *run_options, const TF_Output *inputs, TF_Tensor *const *input_values, int ninputs, const TF_Output *outputs, TF_Tensor **output_values, int noutputs, const TF_Operation *const *target_opers, int ntargets, TF_Buffer *run_metadata, TF_Status *);
+		[DllImport (NativeBinding.TensorFlowLibrary)]
+		private static extern unsafe void TF_SessionRun (TF_Session session, LLBuffer* run_options, TFOutput [] inputs, TF_Tensor [] input_values, int ninputs, TFOutput [] outputs, TF_Tensor [] output_values, int noutputs, TF_Operation [] target_opers, int ntargets, LLBuffer* run_metadata, TF_Status status);


TF_SessionRun [](start = 36, length = 13)

I looked at the TF C-API and it states that the following about the outputs which indicates that we don't need to copy data out of TF. It transfers ownership to caller. Let me know if I get it correctly.

// On success, the tensors corresponding to outputs[0,noutputs-1] are placed in
// output_values[]. Ownership of the elements of output_values[] is transferred
// to the caller, which must eventually call TF_DeleteTensor on them.

Right. We have to wrap the TFTensor in an ML.NET type and avoid the copy. It won't be exposed as a managed array, but as some other type that can be accessed like an array. That wrapping type would need to be disposable. Is there a type in ML.NET that's a good fit? I have an appropriate type in S.N.Tensors but we probably want to use whatever type ML.NET standardizes for this. Looked at VBuffer and it won't work since its a struct that exposes the managed array, it doesn't have a "pointer" / "IReadOnlyList" mode. I don't think IDV is correct either.

I looked into C-API to see if we could get TF to directly allocate the resulting tensor as a managed array and it doesn't look like they provide a hook for the allocator, so we'll need to either wrap their memory (and manage its liftime) or copy it.

Let me know your thoughts about a wrapping type for TFTensor or a native pointer.

In reply to: 210361853 [](ancestors = 210361853)

This method will accept a .NET type and produce a tensor. In the case of a scalar it allocates a single element on the managed heap to back the tensor. In the case of single or multi-dimensional array it pins the array on the managed heap and wraps it in a tensor, allowing TF to directly interact with the managed array. In all other cases we throw. To produce this method I used T4 templates, which autogenerate the CS source when you edit the tt file. See https://docs.microsoft.com/en-us/visualstudio/modeling/design-time-code-generation-by-using-t4-text-templates#creating-a-design-time-t4-text-template for more details. We use the same approach in CoreFx and other repos for dealing with type-specific functionality breakout from a generic method.

ericstj · 2018-08-15T23:19:45Z

OK, I've rebased this on your latest and added to it the generic method we discussed. Please have a look and merge at will. I can work in your branch from now on.

ericstj mentioned this pull request Aug 15, 2018

ML.Net and Tensorflow integration demo. #3

Open

ericstj added 4 commits August 15, 2018 10:25

Bring over copy of TF# source

989e61b

*NOT BUILDING*

Add license to TF# files and ifdef to make them build

6201aa8

Trim down TensorFlowSharp API

b2a8016

zeahmed reviewed Aug 15, 2018

View reviewed changes

ericstj added 3 commits August 15, 2018 10:49

Enable StyleCop and fix issues

d616fe5

Enable ML Codeanalyzer

4714fe8

I've fixed some minor rule issues and disabled the naming analyzers since those churn the source too much.

Move TensorFlow support into Microsoft.ML.Transforms

fcc7d75

For now it is BYOTF (bring your own TensorFlow) so the tests are grabbing the TF bits for TF#.

zeahmed reviewed Aug 15, 2018

View reviewed changes

Put TensorFlow support under Microsoft.ML.Transforms.TensorFlow and m…

78b8870

…ake internal

zeahmed reviewed Aug 15, 2018

View reviewed changes

ericstj force-pushed the wrapTF branch from d3730a0 to f632824 Compare August 15, 2018 23:18

zeahmed merged commit 07b586b into zeahmed:tensorflow Aug 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Create a minimal TF binding #4

[WIP] Create a minimal TF binding #4

ericstj commented Aug 13, 2018

ericstj commented Aug 14, 2018 •

edited

Loading

ericstj commented Aug 14, 2018 •

edited

Loading

zeahmed Aug 15, 2018

ericstj Aug 15, 2018

zeahmed Aug 15, 2018

ericstj Aug 15, 2018 •

edited

Loading

zeahmed Aug 15, 2018

ericstj Aug 15, 2018

zeahmed Aug 15, 2018

ericstj Aug 15, 2018

zeahmed Aug 15, 2018

ericstj Aug 15, 2018 •

edited

Loading

ericstj commented Aug 15, 2018

[WIP] Create a minimal TF binding #4

[WIP] Create a minimal TF binding #4

Conversation

ericstj commented Aug 13, 2018

ericstj commented Aug 14, 2018 • edited Loading

ericstj commented Aug 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericstj Aug 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericstj Aug 15, 2018 • edited Loading

Choose a reason for hiding this comment

ericstj commented Aug 15, 2018

ericstj commented Aug 14, 2018 •

edited

Loading

ericstj commented Aug 14, 2018 •

edited

Loading

ericstj Aug 15, 2018 •

edited

Loading

ericstj Aug 15, 2018 •

edited

Loading