Create API for extracting information about the nodes in a TensorFlow model #862

yaeldekel · 2018-09-07T22:51:02Z

This PR addresses issue #791 .
Please feel free to add feedback or suggestions.

ericstj · 2018-09-10T22:23:37Z

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

 using System.Collections.Generic;
-using System.Linq.Expressions;
+using Microsoft.ML.Runtime.Internal.Utilities;
+using Microsoft.ML.Runtime.Data;


Are these using statements actually necessary? I'm missing the additions that actually used them. #Resolved

Thanks for pointing it out. I added the new API in this file, but ended up moving it to TensorflowUtils.

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

ericstj · 2018-09-10T22:32:45Z

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

@@ -700,6 +698,24 @@ public override string ToString()
            IntPtr len;
            return TF_GraphDebugString(Handle, out len);
        }
+
+        [DllImport(NativeBinding.TensorFlowLibrary)]
+        internal static extern string TF_OperationName(TF_Operation oper);


Rather than directly exposing the TF C-API we should consider bringing back the TF# defined API (TFOperation class) on top of it. See zeahmed@b2a8016#diff-ec7ea5716f3c05f773d3e1507b4f486aL729, zeahmed@b2a8016#diff-ec7ea5716f3c05f773d3e1507b4f486aL748. #Resolved

zeahmed · 2018-09-10T23:43:34Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorflowUtils.cs

+                    continue;
+
+                var numInputs = TFGraph.TF_OperationNumInputs(oper);
+                if (numInputs == 0)


numInputs [](start = 20, length = 9)

What does numInputs == 0 indicates? The input node does not have any input I believe??? #Closed

The input has OpType "Placeholder". There are other nodes with numInputs==0, which have OpType "Const". I am not sure what they do but I think we don't want them in our output schema. What do you think?

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

What do you think if such nodes are not filter? Do you foresee adverse effect of doing this or not doing this?

In reply to: 216758114 [](ancestors = 216758114,216508337)

I added the nodes with 0 inputs to the schema as well.

In reply to: 216817673 [](ancestors = 216817673,216758114,216508337)

zeahmed · 2018-09-10T23:44:24Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorflowUtils.cs

+            while ((oper = TFGraph.TF_GraphNextOperation(graph.handle, &pos)) != IntPtr.Zero)
+            {
+                var name = TFGraph.TF_OperationName(oper);
+                var type = TFGraph.TF_OperationOpType(oper);


type [](start = 20, length = 4)

Its not being used anywhere. #Closed

zeahmed · 2018-09-10T23:49:08Z

test/Microsoft.ML.Tests/ScenariosWithDirectInstantiation/TensorflowTests.cs

+            var model_location = "mnist_model/frozen_saved_model.pb";
+            using (var env = new TlcEnvironment(seed: 1, conc: 1))
+            {
+                var schema = TensorFlowUtils.GetModelSchema(env, model_location);


schema [](start = 20, length = 6)

It would be nice to have schema checked against the actual model information. It seems like mnist_model/frozen_saved_model.pb is a big model. Matrix multiplication model used in above test may be a good for this test.
It can be implemented as a new test in addition to this one. #Closed

zeahmed · 2018-09-10T23:53:26Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorflowUtils.cs

+        public static ISchema GetModelSchema(IExceptionContext ectx, string modelFile)
+        {
+            var bytes = File.ReadAllBytes(modelFile);
+            var session = LoadTFSession(ectx, bytes, modelFile);


LoadTFSession [](start = 26, length = 13)

What about models that are not frozen? I assume it will be there once @abgoswam changes are there, right? #Closed

Yes. I assume that LoadTFSession will take care of the logic to decide which kind of model to load.

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

Ivanidzo4ka · 2018-09-11T00:08:26Z

src/Microsoft.ML.TensorFlow/TensorflowTransform.cs

        {
            Contracts.CheckValue(env, nameof(env));
            _host = env.Register(nameof(RegistrationName));
            _host.CheckValue(modelBytes, nameof(modelBytes));
-            Session = LoadTFSession(modelBytes);
+            Session = TensorFlowUtils.LoadTFSession(_host, modelBytes, modelFile);


Abhishek is working on brining another way to load session, so I think it's better to extend CheckFileAndRead function and force it to return you Session instead of byte array. So private constructor would just accept session. #Closed

Ivanidzo4ka · 2018-09-11T00:22:09Z

src/Microsoft.ML.TensorFlow/TensorflowTransform.cs

@@ -182,16 +166,16 @@ private static byte[] CheckFileAndRead(IHostEnvironment env, string modelFile)
        }

        public TensorFlowTransform(IHostEnvironment env, string modelFile, string[] inputs, string[] outputs) :
-            this(env, CheckFileAndRead(env, modelFile), inputs, outputs)
+            this(env, CheckFileAndRead(env, modelFile), inputs, outputs, modelFile)


modelFile [](start = 73, length = 9)

is it model file or model args? #Closed

…s needed for it.

zeahmed

Ivanidzo4ka · 2018-09-13T23:20:05Z

Is it still WIP or it's ok to review it properly? I see people signing off, and WIP status and find this a bit confusing #Resolved

…nodes, and a console app that displays it

yaeldekel · 2018-09-14T18:09:07Z

src/Microsoft.ML.DnnAnalyzer/Microsoft.ML.DnnAnalyzer/DnnAnalyzer.cs

+                    return;
+                }
+
+                foreach (var (name, opType, type, inputs) in TensorFlowUtils.GetModelNodes(args[0]))


foreach [](start = 16, length = 7)

I can add more arguments to this app, to let the user filter nodes by operation type (for example, sometimes there are lots of "Const" nodes that users might not be interested in if they are just trying to find the name of a certain layer).

Is this valuable? Is having a method that returns this information enough so users can filter programatically, or would user want this as well?

yaeldekel · 2018-09-14T18:09:30Z

Please review properly, I removed the WIP from the title.

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

Ivanidzo4ka · 2018-09-17T19:00:23Z

src/Microsoft.ML.DnnAnalyzer/Microsoft.ML.DnnAnalyzer/Microsoft.ML.DnnAnalyzer.csproj

+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFramework>netcoreapp2.1</TargetFramework>
+    <AssemblyName>DnnAnalyzer</AssemblyName>


should it be part of TensorFlow package? #Closed

It should, thanks for reminding.

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

Ivanidzo4ka · 2018-09-17T19:01:49Z

src/Microsoft.ML.DnnAnalyzer/Microsoft.ML.DnnAnalyzer/DnnAnalyzer.cs

+            {
+                if (Utils.Size(args) != 1)
+                {
+                    ch.Error("Usage: dotnet DnnAnalyzer.dll <model_location>");


.dll [](start = 55, length = 4)

is dll necessary? and should it be dotnet run or just dotnet works? #Closed

"dotnet DnnAnalyzer " didn't work.
"dotnet run DnnAnalyzer.dll " didn't work.

Is there a different syntax I should try?

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

No, apparently i need to study dotnet better. dotnet run for projects only,

In reply to: 218535326 [](ancestors = 218535326,218189666)

Ivanidzo4ka · 2018-09-17T19:47:06Z

src/Microsoft.ML.DnnAnalyzer/Microsoft.ML.DnnAnalyzer/DnnAnalyzer.cs

+    {
+        public static void Main(string[] args)
+        {
+            using (var env = new TlcEnvironment())


TlcEnvironment [](start = 33, length = 14)

isn't it feel weird to create environment only to write something to console?
Why can't you just do Console.Writeline? #Resolved

Ivanidzo4ka · 2018-09-17T19:47:55Z

src/Microsoft.ML.TensorFlow/TensorflowTransform.cs

        {
        }

-        private TensorFlowTransform(IHostEnvironment env, byte[] modelBytes, string[] inputs, string[] outputs)
+        private TensorFlowTransform(IHostEnvironment env, TFSession session, string[] inputs, string[] outputs, string modelFile = null)


string modelFile = null [](start = 112, length = 23)

not necessary anymore #Closed

Ivanidzo4ka · 2018-09-17T19:52:04Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorflowUtils.cs

+            var opTypeGetters = new List<MetadataUtils.MetadataGetter<DvText>>();
+            var inputOpsGetters = new List<MetadataUtils.MetadataGetter<VBuffer<DvText>>>();
+            var inputOpsLengths = new List<int>();
+            foreach (var oper in graph)


oper [](start = 25, length = 4)

nit: If you shorten operation to oper, you can just go even further to "op" option.
#Closed

Ivanidzo4ka · 2018-09-17T19:52:45Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorflowUtils.cs

+            var inputOpsLengths = new List<int>();
+            foreach (var oper in graph)
+            {
+                if (oper.NumOutputs != 1)


if (oper.NumOutputs != 1) [](start = 16, length = 25)

I think this deserve comment.

I removed this code. I think that if a node has multiple outputs it means that the output it produces is used as input to multiple nodes, but the shape and type of this output will be the same for every node that uses it as input. In this case there is no need to skip it. @zeahmed, is my assumption correct?

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

Actually, tf.Operation can produced multiple outputs with different types and shapes. In case of single output used by multiple nodes, I am not sure if `oper.NumOutputs > 1' in that case. Technically, recurrent layers can produced two outputs (hidden state, output) but I would need to see how the recurrent ops are implemented in graphs.

In reply to: 218551502 [](ancestors = 218551502,218204247)

Ivanidzo4ka · 2018-09-17T19:53:23Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorflowUtils.cs

@@ -25,7 +30,92 @@ public static void Initialize()
            ImageAnalytics.Initialize();
        }

+        private static unsafe ISchema GetModelSchema(IExceptionContext ectx, TFGraph graph)


unsafe [](start = 23, length = 6)

just curious, why it's unsafe? #Closed

Thanks for pointing it out, it doesn't need to be. In a previous iteration I was directly running the while loop over the graph that is now in the Graph class, so it was unsafe.

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

Ivanidzo4ka

🕐

Ivanidzo4ka

yaeldMS added 3 commits September 7, 2018 10:37

Add a method that returns TensorFlow model outputs as an ISchema.

0e1e2fb

Merge from master.

0476b47

Update after merge with master

f632417

yaeldekel requested review from Ivanidzo4ka, abgoswam, zeahmed, Zruty0 and ericstj September 7, 2018 22:51

ericstj reviewed Sep 10, 2018

View reviewed changes

zeahmed reviewed Sep 10, 2018

View reviewed changes

Ivanidzo4ka reviewed Sep 11, 2018

View reviewed changes

yaeldMS added 3 commits September 11, 2018 11:01

Address PR comments.

78b0ae4

Merge from master.

9669fbe

Add metadata with information about the operation type, and the input…

3702031

…s needed for it.

zeahmed approved these changes Sep 13, 2018

View reviewed changes

yaeldekel changed the title ~~WIP: Create API for extracting information about the nodes in a TensorFlow model~~ Create API for extracting information about the nodes in a TensorFlow model Sep 14, 2018

yaeldMS added 2 commits September 14, 2018 10:48

Add method that returns an enumerable of the information about graph …

1f6a931

…nodes, and a console app that displays it

Add the DnnAnalyzer project files.

478b020

yaeldekel commented Sep 14, 2018

View reviewed changes

Ivanidzo4ka reviewed Sep 17, 2018

View reviewed changes

Ivanidzo4ka suggested changes Sep 17, 2018

View reviewed changes

Address code review comments

8a9e2c4

Ivanidzo4ka approved these changes Sep 18, 2018

View reviewed changes

yaeldMS added 3 commits September 19, 2018 17:11

Merge with master

bc9fbc1

Make needed changes after merge with master

7ad8de9

Fix bug when there is a node with 1 dimension that is unknown

b94987d

yaeldekel merged commit a627d5b into dotnet:master Sep 20, 2018

yaeldekel deleted the shapesapi branch September 20, 2018 17:22

GalOshri mentioned this pull request Sep 26, 2018

Simple example for getting input and output nodes of TF graph #1040

Closed

ghost locked as resolved and limited conversation to collaborators Mar 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create API for extracting information about the nodes in a TensorFlow model #862

Create API for extracting information about the nodes in a TensorFlow model #862

yaeldekel commented Sep 7, 2018

ericstj Sep 10, 2018 •

edited by yaeldekel

Loading

yaeldekel Sep 11, 2018

ericstj Sep 10, 2018 •

edited by yaeldekel

Loading

zeahmed Sep 10, 2018 •

edited

Loading

yaeldekel Sep 11, 2018

zeahmed Sep 11, 2018

yaeldekel Sep 13, 2018

zeahmed Sep 10, 2018 •

edited

Loading

zeahmed Sep 10, 2018 •

edited

Loading

zeahmed Sep 10, 2018 •

edited

Loading

yaeldekel Sep 11, 2018

Ivanidzo4ka Sep 11, 2018 •

edited

Loading

Ivanidzo4ka Sep 11, 2018 •

edited

Loading

zeahmed left a comment

Ivanidzo4ka commented Sep 13, 2018 •

edited by yaeldekel

Loading

yaeldekel Sep 14, 2018

yaeldekel commented Sep 14, 2018

Ivanidzo4ka Sep 17, 2018 •

edited

Loading

yaeldekel Sep 18, 2018

Ivanidzo4ka Sep 17, 2018 •

edited

Loading

yaeldekel Sep 18, 2018

Ivanidzo4ka Sep 18, 2018

Ivanidzo4ka Sep 17, 2018 •

edited by yaeldekel

Loading

Ivanidzo4ka Sep 17, 2018 •

edited

Loading

Ivanidzo4ka Sep 17, 2018 •

edited

Loading

Ivanidzo4ka Sep 17, 2018

yaeldekel Sep 18, 2018

zeahmed Sep 19, 2018 •

edited

Loading

Ivanidzo4ka Sep 17, 2018 •

edited

Loading

yaeldekel Sep 18, 2018

Ivanidzo4ka left a comment

Ivanidzo4ka left a comment

Create API for extracting information about the nodes in a TensorFlow model #862

Create API for extracting information about the nodes in a TensorFlow model #862

Conversation

yaeldekel commented Sep 7, 2018

ericstj Sep 10, 2018 • edited by yaeldekel Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericstj Sep 10, 2018 • edited by yaeldekel Loading

Choose a reason for hiding this comment

zeahmed Sep 10, 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

zeahmed Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

zeahmed Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

zeahmed Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ivanidzo4ka Sep 11, 2018 • edited Loading

Choose a reason for hiding this comment

Ivanidzo4ka Sep 11, 2018 • edited Loading

Choose a reason for hiding this comment

zeahmed left a comment

Choose a reason for hiding this comment

Ivanidzo4ka commented Sep 13, 2018 • edited by yaeldekel Loading

Choose a reason for hiding this comment

yaeldekel commented Sep 14, 2018

Ivanidzo4ka Sep 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ivanidzo4ka Sep 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ivanidzo4ka Sep 17, 2018 • edited by yaeldekel Loading

Choose a reason for hiding this comment

Ivanidzo4ka Sep 17, 2018 • edited Loading

Choose a reason for hiding this comment

Ivanidzo4ka Sep 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeahmed Sep 19, 2018 • edited Loading

Choose a reason for hiding this comment

Ivanidzo4ka Sep 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ivanidzo4ka left a comment

Choose a reason for hiding this comment

Ivanidzo4ka left a comment

Choose a reason for hiding this comment

ericstj Sep 10, 2018 •

edited by yaeldekel

Loading

ericstj Sep 10, 2018 •

edited by yaeldekel

Loading

zeahmed Sep 10, 2018 •

edited

Loading

zeahmed Sep 10, 2018 •

edited

Loading

zeahmed Sep 10, 2018 •

edited

Loading

zeahmed Sep 10, 2018 •

edited

Loading

Ivanidzo4ka Sep 11, 2018 •

edited

Loading

Ivanidzo4ka Sep 11, 2018 •

edited

Loading

Ivanidzo4ka commented Sep 13, 2018 •

edited by yaeldekel

Loading

Ivanidzo4ka Sep 17, 2018 •

edited

Loading

Ivanidzo4ka Sep 17, 2018 •

edited

Loading

Ivanidzo4ka Sep 17, 2018 •

edited by yaeldekel

Loading

Ivanidzo4ka Sep 17, 2018 •

edited

Loading

Ivanidzo4ka Sep 17, 2018 •

edited

Loading

zeahmed Sep 19, 2018 •

edited

Loading

Ivanidzo4ka Sep 17, 2018 •

edited

Loading